-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
[release/8.0] wasm-tools: Fix workload manifest MSI ProductVersion generation #91023
Conversation
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsBackport of #91015 to release/8.0 /cc @joeloff Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
Tagging subscribers to this area: @directhex Issue DetailsBackport of #91015 to release/8.0 /cc @joeloff Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
@joeloff can you please fill out the template and get a code review sign-off? This looks infra, so I am assuming it's tell mode, in which case it's automatically approved (unless you tell me otherwise). Note: I can merge it for you to bypass the merge restriction. No need to click on "Update branch", or that will restart the CI (unless you really really need to rebase your PR). But I'll also wait for the |
I ported this because the same fixes are going into servicing for September and need this fixed in 8.0 before GA and before we ship that in VS. Main is 9.0 and not shipping anywhere. Not really an infra change - it's fixing a versioning bug that has functional impact on the CLI and VS. Can you point to the template? |
Ok thanks for confirming. In that case, we need an M2 approval. @jeffschwMSFT is this in your area?
Yes, it's in the main description textbox of this PR. It was automatically generated. |
The 6.0 and 7.0 equivalent PRs have been merged. Awaiting confirmation to merge this one too. |
Backport of #91015 to release/8.0
/cc @joeloff
Customer Impact
Manifest installers for .NET optional workloads rely on performing major upgrades in .NET 6 and 7. This requires that the MSI version increments for new builds. The MSI version was generated using the assembly version. MSI versions should be generated using the major/minor/patch/buildnumber, e.g. 7.0.11.12345. Because the assembly version was being used, the patch value was always set to 0. Due to the bit masking and shift operations used to construct the MSI version, the 3rd part of the version number wrapped around, generating a smaller version for a newer release. MSI ProductVersion is only 32-bits in size (8-bit major, 8-bit minor and 16-bit build number).
While 8.0 is using SxS manifests, having MSIs with lower version can result in manifest validation errors when inserting to VS. It will also be confusing to ship MSIs with lower version in newer releases. If we ever decide to flip back to upgradable manifest installers, the versioning issues will block the CLI from correctly installing updates.
Testing
Verification has to be manual using Orca to inspect the MSI product version along with the component manifest for the generated workload that flows to VS.
Risk
Low, the fix has been validated for .NET 6 and 7 as part of the September servicing release.