Windows Installer: Upgrade to WiX Toolset 5
This upgrades tooling used to build the Dangerzone.msi installer to WiX Toolset 5.
The Major change in this, apart from what running wix convert on the old generated WiX authoring, is that the target directory for each component gets set in the Directory attribute of the component instead of a separate Directory element one level above in the XML hierarchy. All components get grouped under a ComponentGroup that gets referenced in the Feature tree, which makes things much simpler aswell.
There are still a couple of open questions:
Should AllowSameVersionUpgrades="yes" be removed?
According to the documentation setting it useful if the installer uses a fourth version number. But Dangerzone doesn't use one so it should be save to remove it. Running wix msi validate on the built msi also prints a warning about this.
What to set the value of InstallerVersion to?
In this its set to 200 to make 64 bit installer work, but it could also be removed or set to the default 500, which would cap the lowest version of windows Dangerzone supports to Windows 7.
Should Dangerzone also set a desktop icon?
This is also a possibility, but opinions vary is this is good practice or not. Also this should come with an option to not install the icon, but that would require developing some custom dialogs for the installer.
Customise the installer theming more?
This should probably be its own issue. The installer, while limited, allows for a bit customization, mainly by addind a banner image that would replace the red cd icon shown during install currently. The installer already uses a custom image shown at the start of the installation . For more, see:
https://wixtoolset.org/docs/tools/wixext/wixui/#replacing-the-default-bitmaps https://stackoverflow.com/questions/48713061/how-do-you-change-the-red-cd-icon-on-wix
[!CAUTION] One unfortunate side effect this has is that installing the newer installer does not uninstall older versions of Dangerzone. The root cause for this is a changed default for the
Scopeattribute in thePackageelement. In WiX 3 it was apparently set toperUserby mistake and in WiX 4 its set toperMachine
One workaround for the uninstallation issue would be to set Scope="perUser" attribute in the Package element. But MSIs apparently have poor support for per user packages, though I havent figured out how exactly just yet.
Another solution could be to bite the bullet this once and guide users on Windows into uninstalling the older version of Dangerzone somehow, for example a blog post or a pinned issue, or maybe within Dangerzone itself. SInce this pull request also makes Dangerzone a proper 64 bit installation, the new version should install itself alongside the old one just fine without breaking anything. Though it'll show up twice in installed programs and the start menu, leaving it up to the user to discover older version is still installed and uninstall it. Not exactly the best user experience.
closes #602
Amazing work here @jkarasti. Not only did you manage to update our scripts to use WiX 5, but you broke down the changes in 20 (!) short and sweet commits. The changes make sense, and I'm testing them in a Windows box now.
Now, to your questions, here's my understanding:
Should AllowSameVersionUpgrades="yes" be removed?
My understanding is that previously, we specified * as the product code:
https://github.com/freedomofpress/dangerzone/blob/791444cd5dd983f87696c4d16313f2d3a6af515d/install/windows/build-wxs.py#L167
With this PR, we don't specify something, which means that the default behavior will trigger:
By default, a new ProductCode property will be generated with every build enabling major upgrades.
(from https://wixtoolset.org/docs/schema/wxs/package/#attributes)
So, if we remove AllowSameVersionUpgrades="yes", this means that we cannot simply rebuild the Dangerzone MSI, if Wix announces a CVE for instance (this has happened in the past btw). We will need to also bump the patch version of Dangerzone. This is not bad per se, but it's a limitation us devs need to be aware of.
What to set the value of InstallerVersion to?
Setting it to 200 - or even removing it, since it's the default - is fine. After all, Docker Desktop (well, WSL2) works from Windows 10 onwards, so we're capped by this requirement.
Should Dangerzone also set a desktop icon?
Not pressing, looks like a future improvement to me.
Customise the installer theming more?
Another future improvement, but thanks for the context. Once we merge the PR, we should refer to your links in a separate issue, as you have correctly mentioned.
FYI, I tested your branch and it works great. I can verify that the previous Dangerzone installation is not removed, which is a major bummer. I'll dig more around to figure out if we can do something about that.
I tested with Scope="perUser" and I get the following error:
This is similar to this SO post, in which they had to do some shenanigans to elevate their privileges on installation time.
I wonder, is there a way to detect that a similar package exists, and ask users to remove it from "Programs and Features", like we do in the MajorUpgrade element?
I tested with
Scope="perUser"and I get the following error:
This is similar to this SO post, in which they had to do some shenanigans to elevate their privileges on installation time.
I wonder, is there a way to detect that a similar package exists, and ask users to remove it from "Programs and Features", like we do in the
MajorUpgradeelement?
I found this. maybe we could add some custom action to detect and block the installation, I haven't looked into it much yet. Also note that this is for WiX 3 since the HowTos for WiX 4 and up haven't been written yet, so some things might have changed.
Should
AllowSameVersionUpgrades="yes"be removed?My understanding is that previously, we specified
*as the product code:https://github.com/freedomofpress/dangerzone/blob/791444cd5dd983f87696c4d16313f2d3a6af515d/install/windows/build-wxs.py#L167
With this PR, we don't specify something, which means that the default behavior will trigger:
By default, a new ProductCode property will be generated with every build enabling major upgrades.
(from https://wixtoolset.org/docs/schema/wxs/package/#attributes)
Just to note: Setting the value of Id to * means WiX will generate a GUID automatically, so this is doesn't introduce a change in behaviour. See docs for WiX 3 here and here
So, if we remove
AllowSameVersionUpgrades="yes", this means that we cannot simply rebuild the Dangerzone MSI, if Wix announces a CVE for instance (this has happened in the past btw). We will need to also bump the patch version of Dangerzone. This is not bad per se, but it's a limitation us devs need to be aware of.
Ah, I didn't consider needing to rebuild the installer with a same version number. The main motivation for removing it is to make validation pass without warnings. Looks like making AllowSameVersionUpgrades="yes" the default was considered for WiX 4, but doing it would set off the warning in every .msi built with WiX 4 and disabling it would disable many other checks in ICE61 aswell. See this episode of Rob Menschings excellent series on WiX Toolset 4. (This was a great resource with this upgrade. btw.)
So it comes down to being fully compliant with spec and potentionally needing to release a patch release exclusively for windows every now and then. Or keeping things as is and having the ability to patch the windows installer without a version bump.
Also. WiX Toolset 5 introduces a brand new feature: Files. I'm working on a new version of this PR that integrates the Files feature in the .wxs build script. In a nutshell, the whole file harversting, component, and directory generation dance in build-wxs.py (and ~3000 lines of code in the generated Dangerzone.wxs) could potentially be replaced with a sinle line:
<Files Include="exe.win-amd64-3.12\**" />
Pushed a couple of changes:
Removed the InstallerVersion.
Let's trust the WiX developers keep the default at a sensibly old version of Windows.
Updated the build instructions.
Better be safe than sorry, so mention opening a new terminal before installing the WiX UI extension. Also made a couple of Smaller tweaks. I've still kept the harcoded version in place. Maybe there should be an extra step in the release instructions to check for newer version of WiX and either bump if it's a minor or a patch version, or plan the upgrade if there's a new major version with breaking changes.
I havent touched the AllowSameVersionUpgrades attribute yet.
Hey @jkarasti. Just wanted to let you know that we have wrapped Dangerzone 0.8.0. If there's something I can take a look at or help you with, now is the time to let me know!
Hey @jkarasti. Just wanted to let you know that we have wrapped Dangerzone 0.8.0. If there's something I can take a look at or help you with, now is the time to let me know!
Hi. I'd say the biggest TODO on here is figuring out the better way around the changed install scope, since the one I have here currently seems to break the installer in some way.
I've been experimenting with using this trick to try detecting the version of currently installed Dangerzone and then stopping the install using the Condition attribute in a WiX Launch element. I've almost got it working, but it still needs work. A bit annoyingly, since #909 got included in 0.8.0, this way now has an edge case where it wont work if the user chooses a non-default installation location.
An another way I've been trying a bit would be running a PowerShell script to parse through the windows registry for an existing install. I have a script that should be able to detect Dangerzone reliably regardless of where its installed. I should be able to include the script using a WiX Binary element and then run it using a Custom Action, but still need to figure out a way to actually do that. The issue I have with this approach is that snooping through what the user has installed on their computer doesen't feel particularly security minded. At the moment the registry keys for Dangerzone live in ´HKLM/Software/Wow6432Node/Microsoft/Windows/CurrentVersion/Uninstall/´ under some GUID. One thought I had while writing this would be to see if the GUID is the same for some version of Dangerzone installed and try detecting those instead.
Other than that, I'm fairly confident this is ready for review, except for maybe a bunch of bikeshedding over minor stuff like AllowSameVersionUpgrades, documentation and such.
Great, thanks for the progress report. I'll give this PR another spin soon and review the latest additions. As for our main blocker, sigh, I'll try to take a look as well, but it may take me a while to build the context you have. I'll keep you posted...
Just checked the diff between the state of the PR when I reviewed it and its current state. I'm fine with the changes, and the only thing remaining is indeed migrating the installation scope. Haven't tested your proposed changes on that part yet, since you mention some edge cases.
Regarding AllowSameVersionUpgrades, your explanation about being spec-compliant makes sense, so I'm fine with removing it. As for the documentation fixes, they look ok to me.
Out of curiosity, I see that we can trigger the uninstall dialog via:
MsiExec.exe /X{GUID}
(taken from this SO post, not tested)
I suppose we can trigger this command during the installation of our .MSI. Any downside to that?
Btw, if it takes too much of our energy to fix this issue, then that's fine. You mentioned already that:
Another solution could be to bite the bullet this once and guide users on Windows into uninstalling the older version of Dangerzone somehow, for example a blog post or a pinned issue, or maybe within Dangerzone itself. SInce this pull request also makes Dangerzone a proper 64 bit installation, the new version should install itself alongside the old one just fine without breaking anything. Though it'll show up twice in installed programs and the start menu, leaving it up to the user to discover older version is still installed and uninstall it. Not exactly the best user experience.
I would be fine with adding a download link that says "Read this if you have installed Dangerzone version v0.8.0 or older", and explain to the user that they need to uninstall their previous Dangerzone version first. It's not the best thing ever, but we're reaching Wix v3 deprecation soon, so that's the hand we're dealt with.
Just checked the diff between the state of the PR when I reviewed it and its current state. I'm fine with the changes, and the only thing remaining is indeed migrating the installation scope. Haven't tested your proposed changes on that part yet, since you mention some edge cases.
I would recommend testing the installer in a vm or a Windows Sandbox As it turns out, if you're clever enough, you can tell WiX to build an installer which will launch if and only if it's not installed... that's apparently true when uninstalling it aswell. Fun times...
Which brings us to:
Btw, if it takes too much of our energy to fix this issue, then that's fine. You mentioned already that:
Another solution could be to bite the bullet this once and guide users on Windows into uninstalling the older version of Dangerzone somehow, for example a blog post or a pinned issue, or maybe within Dangerzone itself. SInce this pull request also makes Dangerzone a proper 64 bit installation, the new version should install itself alongside the old one just fine without breaking anything. Though it'll show up twice in installed programs and the start menu, leaving it up to the user to discover older version is still installed and uninstall it. Not exactly the best user experience.
I would be fine with adding a download link that says "Read this if you have installed Dangerzone version v0.8.0 or older", and explain to the user that they need to uninstall their previous Dangerzone version first. It's not the best thing ever, but we're reaching Wix v3 deprecation soon, so that's the hand we're dealt with.
I'm leaning towards agreeing as of now, maybe it would be enough to mention uninstalling the older version of Dangerzone in the release notes and/or blog post with some degree of urgency and maybe add an announcement banner on the website like was done in here. Though I do have a couple of ideas for a workaround, for example, adding a registry key, something like NewInstaller=1 and then using that as a marker for showing an error message when launching the installer. But this could be a follow up.
Regarding
AllowSameVersionUpgrades, your explanation about being spec-compliant makes sense, so I'm fine with removing it. As for the documentation fixes, they look ok to me.
I removed it.
Out of curiosity, I see that we can trigger the uninstall dialog via:
MsiExec.exe /X{GUID}(taken from this SO post, not tested)
I suppose we can trigger this command during the installation of our .MSI. Any downside to that?
I think this could work, maybe use a WiX Custom Action Element to run it, though we'd need to know which GUIDs to look for first. I suspect it's whatever WiX assingns to the ProductCode property in the Package element.
I pushed a couple small tweaks for the build docs in a separate fixup commit, please check I didn't make it too complicated.
And I also dropped the hardcoded versions with the msi build in CI. Thinking about it, having two separate actions for it might be a bit overkill, especially since there shouldn't be any meaningful difference between them before WiX Toolset v6 is released (rc.1 has a release date in February). I think the idea is releasing more frequent major versions of WiX with less breaking changes going foward, so upgrading to WiX v6 should be (fingers crossed) be much less painfull aswell.
Tested the installer and it works, with the known caveat. We have to discuss internally whether it's fine to proceed as is and notify users in our installation page, or whether we need to pursue a proper fix. I'll let you know next week what's the case.
Note that I see in the code the workaround you were experimenting with. In my case, it didn't uninstall the 0.8.0 Dangerzone installation, and I've been using the default installation path. Just letting you know in case it's useful.
Trying to avoid leaving the Scope issue as is, I took an another deep dive to WiX Toolset Launch and Custom Action elements. Unfortunately there really doesen't seem to be a "works like a charm" solution here, at least I haven't found it yet, but I did manage to come up with a somewhat better fix which won't result in a somehow broken installer in jkarasti:602-upgrade-wix-toolset-fix-regsearch. Here I'm using a registry search to look for Istallation of Dangerzone 0.8.0 and stop the installation from starting with a WiX Launch Element. It's not quite perfect. For one, it relies on the ProductId of 0.8.0 installation being {03C2D2B2-9955-4AED-831F-DA4E67FC0FDB} though I think that should always be the case. And second, most likely thanks to the different install scope, you can install 0.8.0 after installing Dangerzone using the new installer, but cannot uninstall the new installation before uninstalling 0.8.0 with this fix included. Note that this is still very much experimental, so test it in a VM.
I also tried using MsiExec.exe to trigger an uninstall during the install but that didn't work at all. Or it did insofar as trying to run MsiExec.exe during the install, but I completely forgot you can't run two msi at the same time. But from that SO answer, it could maybe be possible if we converted the installer into a WiX Bundle which can run msi installers in a sequence. Though that would mean redesigning the installer from scratch so, unless shipping a WiX Bundle has some other amazing muat-have benefits, I don't think going that route is worth it.
Also, I've moved the fix that I had in this branch into jkarasti:602-upgrade-wix-toolset-fix-old since it appears to break the install scope so I don't think we should include it.
Just tested jkarasti:602-upgrade-wix-toolset-fix-regsearch and it detected the Dangerzone 0.8.0 installation. However, it doesn't detect previous installations, such as 0.6.0. Our users may skip 0.8.0 for whatever reasons, so we have to expect such a scenario :-/
I think it's fine to merge it as is, and think of some instructions that we want to provide to users, instead of just letting them download the .msi.
Just tested jkarasti:602-upgrade-wix-toolset-fix-regsearch and it detected the Dangerzone 0.8.0 installation. However, it doesn't detect previous installations, such as 0.6.0. Our users may skip 0.8.0 for whatever reasons, so we have to expect such a scenario :-/
That wors as expected. My initial plan was to figure out the GUIDs for some amount of older versions of Dangerzone and add checks for them aswell, but I've come up with a better fix. Instead of looking at the registy, the installer now checks if dangerzone.exe exists in C:\Program Files (x86)\Dangerzone. This should cover every version except for 0.8.0 with a non-default install folder, so I've also kept the registry search from jkarasti:602-upgrade-wix-toolset-fix-regsearch. The installer should now stop with an error message prompting to uninstall if either of the two are true and continue otherwise.
That's great @jkarasti. I haven't managed to take a look into it yet, but assuming it will work, here's a suggested user flow.
-
In our website, keep the download button for the Windows MSI as is.
-
If users have not installed Dangerzone before, they will not be affected.
-
Write a descriptive error message, for the users who have Dangerzone installed. Something like:
We detected that a previous Dangerzone version is installed in your system. Dangerzone versions prior to 0.9.0 must be uninstalled from "Programs and Features", before installing a new version. Read more about this change in:
(@jkarasti that's on us, don't fret about it)
-
Add a section in our dangerzone.rocks website or our GitHub wiki page where we can explain with screenshots how to uninstall a previous Dangerzone version, and install the new one. We can also link to this PR for more context.
Hopefully this will be a smooth transition as any.
Thanks again for the contribution @jkarasti. I just tested it in a Windows machine and everything works. Awesome job :star_struck:
