Skip to content

[release/8.0] Chain runtime .Msi's instead of .Exe's in hosting bundle #56459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Jul 8, 2024
Merged

[release/8.0] Chain runtime .Msi's instead of .Exe's in hosting bundle #56459

merged 7 commits into from
Jul 8, 2024

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Jun 25, 2024 •
edited
Loading

@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jun 25, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 8.0.x milestone Jun 25, 2024
@wtgodbe wtgodbe marked this pull request as ready for review June 27, 2024 00:32
@wtgodbe wtgodbe requested a review from a team as a code owner June 27, 2024 00:32
@wtgodbe wtgodbe marked this pull request as draft June 27, 2024 00:33
@wtgodbe wtgodbe requested a review from joeloff July 2, 2024 20:57
@wtgodbe wtgodbe marked this pull request as ready for review July 2, 2024 20:57
joeloff
joeloff reviewed Jul 3, 2024
View reviewed changes
src/Installers/Windows/WindowsHostingBundle/Product.targets
<TargetPlatform>x64</TargetPlatform>
<BundleNameProperty>DotNetRedistLtsInstallerx64</BundleNameProperty>
<Version>$(MicrosoftNETCoreAppRuntimeVersion)</Version>
</RuntimeInstallers>
<RuntimeInstallers Include="$(DepsPath)dotnet-runtime-$(MicrosoftNETCoreAppRuntimeVersion)-win-x86.exe">
<RuntimeInstallers Include="$(DepsPath)dotnet-runtime-$(MicrosoftNETCoreAppRuntimeVersion)-win-x86.msi">
Copy link
Member

@joeloff joeloff Jul 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need two additional MSIs for runtime, similar to what we do in desktop. Both the host and hostfxr MSIs need to be included.

The test here would be that on a clean machine, you should be able to install the hosting bundle, then run dotnet --info

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also realized we don't need the ProductSearch anymore, removed those too

joeloff
joeloff reviewed Jul 3, 2024
View reviewed changes
src/Installers/Windows/WindowsHostingBundle/SharedFramework.wxs Outdated
Permanent="yes"
DetectCondition="SharedFxRedistProductVersion_arm64 = v$(var.SharedFxInstallerProductVersionarm64)">
</ExePackage>
Permanent="yes">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we need to remove the Permanent attribute too. When set, the MSI is marked as a system component so it won't remove it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@wtgodbe
Copy link
Member Author

wtgodbe commented Jul 3, 2024 •
edited
Loading

New build https://dev.azure.com/dnceng/internal/_build/results?buildId=2488228&view=results

joeloff
joeloff approved these changes Jul 3, 2024
View reviewed changes
Copy link
Member

@joeloff joeloff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an internal build of this so I can look at a copy of the install?

@wtgodbe
Copy link
Member Author

wtgodbe commented Jul 3, 2024

Do you have an internal build of this so I can look at a copy of the install?

https://dev.azure.com/dnceng/internal/_build/results?buildId=2488228&view=results is almost finished

@wtgodbe
Copy link
Member Author

wtgodbe commented Jul 4, 2024

/backport to release/6.0

@wtgodbe
Copy link
Member Author

wtgodbe commented Jul 4, 2024

/backport to main

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 4, 2024

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/9799873926

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 4, 2024

Started backporting to main: https://github.com/dotnet/aspnetcore/actions/runs/9799875112

@github-actions github-actions bot mentioned this pull request Jul 4, 2024
Merged
11 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 4, 2024

@wtgodbe backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Chain runtime .msi's instead of .exe's in Hosting Bundle
.git/rebase-apply/patch:28: trailing whitespace.
            </MsiPackage>            
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	src/Installers/Windows/WindowsHostingBundle/DotNetCore.wxs
M	src/Installers/Windows/WindowsHostingBundle/Product.targets
M	src/Installers/Windows/WindowsHostingBundle/SharedFramework.wxs
M	src/Installers/Windows/WindowsHostingBundle/WindowsHostingBundle.wixproj
Falling back to patching base and 3-way merge...
Auto-merging src/Installers/Windows/WindowsHostingBundle/WindowsHostingBundle.wixproj
CONFLICT (content): Merge conflict in src/Installers/Windows/WindowsHostingBundle/WindowsHostingBundle.wixproj
Auto-merging src/Installers/Windows/WindowsHostingBundle/SharedFramework.wxs
CONFLICT (content): Merge conflict in src/Installers/Windows/WindowsHostingBundle/SharedFramework.wxs
Auto-merging src/Installers/Windows/WindowsHostingBundle/Product.targets
CONFLICT (content): Merge conflict in src/Installers/Windows/WindowsHostingBundle/Product.targets
Auto-merging src/Installers/Windows/WindowsHostingBundle/DotNetCore.wxs
CONFLICT (content): Merge conflict in src/Installers/Windows/WindowsHostingBundle/DotNetCore.wxs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Chain runtime .msi's instead of .exe's in Hosting Bundle
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 4, 2024

@wtgodbe an error occurred while backporting to release/6.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@wtgodbe wtgodbe mentioned this pull request Jul 4, 2024
Merged
10 tasks
@mkArtakMSFT mkArtakMSFT added the Servicing-approved Shiproom has approved the issue label Jul 8, 2024
@wtgodbe wtgodbe merged commit 54df535 into release/8.0 Jul 8, 2024
25 checks passed
@wtgodbe wtgodbe deleted the wtgodbe/BundleMSIs branch July 8, 2024 20:19
@dotnet-policy-service dotnet-policy-service bot modified the milestones: 8.0.x, 8.0.8 Jul 8, 2024
@Launster Launster mentioned this pull request Nov 27, 2024
Closed
1 task
@jberezanski jberezanski mentioned this pull request May 30, 2025
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joeloff joeloff joeloff approved these changes

Assignees
No one assigned
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Servicing-approved Shiproom has approved the issue
Projects
None yet
Milestone
8.0.8
Development

Successfully merging this pull request may close these issues.
3 participants
@wtgodbe @joeloff @mkArtakMSFT