Fix x86 installer bug: it gets installed as an x64 component and has the same UpgradeCode as the x64 installer leading to an uninstallation of an x64 installation (and vice versa) #5812
Conversation
…of them: -Make x86 installer to be installed as a x86 component (-arch argument to candle.exe) -Make the UpgradeCode unique per platform Additionally, the architecture was appended to the package name to be able to distinguish the installations. Simplified architecture variable use to have only 1 variable for it.
bergmeister
requested review from
TravisEz13,
adityapatwardhan and
daxian-dbw
as code owners
|
@SteveL-MSFT @joeyaiello I am not sure if this bug was ever triaged but it seems quite severe to me and would cause problems in the future when trying to upgrade install the x86 version in 6.1 if this fix does not go into 6.0. On the other hand, I can understand that this is last minute (maybe building only the x86 installer using this fix would mitigate the risk?)... |
|
Should Explorer menu be fixed too? |
|
@iSazonov : The explorer context menu goes into 6.1 (I already asked this question, the answer by @SteveL-MSFT is here). @SteveL-MSFT said here that it is not clear whether the current behaviour is right or wrong and therefore we should wait for feedback. If the current behaviour is bothering you please open an issue so that it can be tracked and discussed further. |
|
@bergmeister My request was that in the previous PR we added the menu and in this PR we add a second PowerShell version - what version will be called from this menu? I suppose that if we want this menu, then in this PR you have to add the menu for both versions. |
|
@iSazonov I see, you have a fair point that I did not consider the case of installing both x64 and x86 for the context menu. |
|
I think the number of users using both x86 and x64 are very minimal. Giving that where we are with 6.0.0, I think we should document this (Known Issues @joeyaiello) and fix it for 6.1.0. |
SteveL-MSFT
added
the
Documentation Needed in this repo
|
I think we need to take this for GA to ensure that we can create servicing packages correctly without creating a complicated servicing wrapper. |
|
I talked to @TravisEz13 offline, agree that servicing would be an issue so we should take this for GA. |
SteveL-MSFT
removed
the
Documentation Needed in this repo
|
filed: #5821 |
|
OK. I will prepare now a separate PR that targets the release branch to check for merge problems beforehand. |
| <!-- Generate Your Own GUID for both ID and UpgradeCode attributes. --> | ||
| <!-- Note: UpgradeCode GUID MUST REMAIN SAME THROUGHOUT ALL VERSIONS --> | ||
| <!-- Otherwise, updates won't occur --> | ||
| <Product Id="$(var.ProductGuid)" Name="$(var.ProductSemanticVersionWithName)" Language="1033" Version="$(var.ProductVersion)" Manufacturer="Microsoft Corporation" UpgradeCode="{f7ba3e58-0be8-443b-ac91-f99dd1e7bd3b}"> |
There was a problem hiding this comment.
Can you add an comment that <Product Id is the product code?
https://stackoverflow.com/questions/10330534/in-wix-where-is-the-productcode-specified
http://wixtoolset.org/documentation/manual/v3/xsd/wix/product.html
There was a problem hiding this comment.
@TravisEz13 : I created a separate PR for 6.0.0 here. The difference is that it is based on the 6.0.0. branch where I cherry picked the fixing commit (an auto-merge of whitespace was needed) so that local testing becomes easier. I have addressed your comment about adding the product code related comment.
This PR shall now as WIP and for 6.1.0.
For the record: the mentioned context menu issue applies only to 6.1 and is not related to this PR
|
@bergmeister I will take care of any merge conflicts with the release branch. Just get it working on this branch. I've finished my review. It looks good otherwise. |
|
OK. Whatever you prefer then. |
|
Can you get the comment added in the next few hours? |
|
@bergmeister Thanks |
|
No worries, my pleasure. I am glad I found it by coincidence (I guess the bug must have been in there since day one). I think the next time we have to ensure that issues get triaged better because if the impact was understood better from the reported issue then it would not have been so much last minute. |
|
I did some analysis and it showed that the bug about the upgrade code being constant and the |
… same UpgradeCode as the x64 installer leading to an uninstallation of an x64 installation (and vice versa) (PowerShell#5812) Fix bug PowerShell#5597: x86/x64 installer are uninstalling each other when installing either of them: -Make x86 installer to be installed as an x86 component (-arch argument to candle.exe, which sets the `sys.BUILDARCH` variable) -Make the UpgradeCode unique per platform -Replace `var.ProductTargetArchitecture` variable with sys.BUILDARCH use to have only 1 variable for the architecture -Additionally, the architecture was appended to the package name to be able to distinguish the installations. # Conflicts: # assets/Product.wxs
… same UpgradeCode as the x64 installer leading to an uninstallation of an x64 installation (and vice versa) (#5812) Fix bug #5597: x86/x64 installer are uninstalling each other when installing either of them: -Make x86 installer to be installed as an x86 component (-arch argument to candle.exe, which sets the `sys.BUILDARCH` variable) -Make the UpgradeCode unique per platform -Replace `var.ProductTargetArchitecture` variable with sys.BUILDARCH use to have only 1 variable for the architecture -Additionally, the architecture was appended to the package name to be able to distinguish the installations. # Conflicts: # assets/Product.wxs
4 participants
PR Summary
Fix bug #5597: x86/x64 installer are uninstalling each other when installing either of them:
-Make x86 installer to be installed as a x86 component (-arch argument to candle.exe, which sets the
sys.BUILDARCHvariable)-Make the UpgradeCode unique per platform
Replace
var.ProductTargetArchitecturevariable with sys.BUILDARCH use to have only 1 variable for the architectureAdditionally, the architecture was appended to the package name to be able to distinguish the installations.
Windows installer PRs usually got reviewed by @SteveL-MSFT in the past.
PR Checklist
Note: Please mark anything not applicable to this PR
NA.[feature]if the change is significant or affectes feature testsWIP:to the beginning of the title and remove the prefix when the PR is ready.