Windows Installer: Upgrade to WiX Toolset 5 #929
Conversation
jkarasti
force-pushed
the
602-upgrade-wix-toolset-v2
branch
2 times, most recently
from
ce8afbe
to
c191fba
Compare
.github/workflows/ci.yml
Outdated
| with: | ||
| dotnet-version: "8.x" | ||
| - name: Install WiX Toolset | ||
| run: dotnet tool install --global wix --version 5.0.1 |
There was a problem hiding this comment.
Any clue if we can specify the major version (5) in our CI, so that we can catch regressions later on? Same goes for the wixext plugin below.
There was a problem hiding this comment.
We could set the version in an environment variable would that be enough?
env:
wix_version: 5.0.1
- name: Install WiX Toolset
run: dotnet tool install --global wix --version $wix_version
- name: Add WiX UI extension
run: wix extension add --global WixToolset.UI.wixext/$wix_versionThere was a problem hiding this comment.
It's not the duplication that worries me, but whether a future 5.1 Wix release breaks something. I'll give it a shot locally.
There was a problem hiding this comment.
Oops, yes, I started looking into the wrong thing. We could set the WiX version with a matrix stradegy. Something like:
strategy:
matrix:
wix_version: [5.0.0, 5.0.1]
- name: Install WiX Toolset
run: dotnet tool install --global wix --version ${{ matrix.wix_version }}
- name: Add WiX UI extension
run: wix extension add --global WixToolset.UI.wixext/${{ matrix.wix_version }}There was a problem hiding this comment.
Actually, I was thinking of something like the following:
dotnet tool install --global wix --version 5.*
wix extension add --global WixToolset.UI.wixext/5
I just realized though that there's an open issue for the latter, and it won't fetch the latest 5.x.y release of WixToolset.UI.wixext (see wixtoolset/issues#8033)
Anyway, we can pin the extension version for now, but my suggestion is to always install the latest WiX version using dotnet tool install, if you agree.
There was a problem hiding this comment.
Initially I hardcoded the version to avoid a situation where a developer installs WiX Toolset to release a new version of Dangerzone and then accidentally builds the windows installer with a newer version of WiX which breaks in unexpected ways. E.g See the whole Scope debacle.
We could drop the version numbers in CI to catch any regressions, but keep them in the build instructions to prevent accidentally upgrading WiX while doing a release. Or maybe even have two jobs in CI. One installs WiX without a version number to test for regressions and the other uses whichever version is used to build the current released installer and gets bumped when the installer is upgraded to a newer version of WiX,
There was a problem hiding this comment.
And on that note, should I also remove the dotnet version? The setup-dotnet action picks whatever is the newest version installed in the runner if its not defined. I picked 8 since that's the version you'll get offered when downloading .NET SDK. On second thought, using the latest 8.x LTS release is probably the sensible thing to do.
There was a problem hiding this comment.
Or maybe even have two jobs in CI. One installs WiX without a version number to test for regressions and the other uses whichever version is used to build the current released installer and gets bumped when the installer is upgraded to a newer version of WiX
Excellent, that's the best of both worlds. Our CI is undergoing a major change, so no need to do this in this PR. I'll create a separate issue to track it. For now, we can keep it as is.
|
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
|
|
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
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 |
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. |
Just to note: Setting the value 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 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 |
jkarasti
force-pushed
the
602-upgrade-wix-toolset-v2
branch
from
c191fba
to
8489bc0
Compare
|
Pushed a couple of changes: Removed the
|
jkarasti
force-pushed
the
602-upgrade-wix-toolset-v2
branch
from
8489bc0
to
ad7dd22
Compare
WixCop.exe is a built in formatting tool that comes with WiX toolset v3. This fixes `wix convert` command not beins able to run
…oring It's not needed anymore.
Also rename `root_el` to `wix_el`. WiX version 5 uses the same namespace.
It's the new default name for it
jkarasti
force-pushed
the
602-upgrade-wix-toolset-v2
branch
2 times, most recently
from
86179d4
to
223fb0f
Compare
install/windows/build-wxs.py
Outdated
| # Workaround for an issue after upgrading from WiX Toolset 3 to 5 where the older | ||
| # version of Dangerzone is not uninstalled during the upgrade | ||
| # | ||
| # Work around the issue by adding some extra functionality to the "Next" button on the welcome screen | ||
| # of the installer. When the user clicks it to proceed with the installation this: | ||
| # 1. Flips the install scope to "perUser" which is the default in WiX 3 | ||
| # 2. Finds the older installation | ||
| # 3. And finally flips the scope back to "perMachine" which is the default in WiX 4 and newer | ||
| # | ||
| # Adapted from this stack overflow answer: https://stackoverflow.com/a/35064434 | ||
| # | ||
| # TODO: Revert this once we are reasonably certain there are no affected Dangerzone Installations? | ||
| ET.SubElement( | ||
| ui_el, | ||
| "Publish", | ||
| Dialog="WelcomeDlg", | ||
| Control="Next", | ||
| Property="ALLUSERS", | ||
| Value="{}", | ||
| ) | ||
| ET.SubElement( | ||
| ui_el, | ||
| "Publish", | ||
| Dialog="WelcomeDlg", | ||
| Control="Next", | ||
| Event="DoAction", | ||
| Value="FindRelatedProducts", | ||
| ) | ||
| ET.SubElement( | ||
| ui_el, | ||
| "Publish", | ||
| Dialog="WelcomeDlg", | ||
| Control="Next", | ||
| Property="ALLUSERS", | ||
| Value="1", | ||
| ) | ||
|
|
There was a problem hiding this comment.
I'm really excited to test this hack out. Thanks a lot!
There was a problem hiding this comment.
I tested it some more and, unfortunately, it get's us about 99% of the way there. With this workaround, the installer can find and uninstall the older version of Dangerzone and the subsequent upgrades work as well. But after reverting the hack the duplicate version issue returns, even if the older installer was built with WiX v5 (!).
- The Keywords and Description items move under a new SummaryInformation element. - Shuffle things around so that elements previously under the product element are now under the Package element. - Rename SummaryCodepage in SummaryInformation to Codepage and remove a duplicate Manufacturer item. - Remove InstallerVersion and let WiX set it to default value. (500 a.k.a Windows 7)
jkarasti
force-pushed
the
602-upgrade-wix-toolset-v2
branch
from
223fb0f
to
5c9f5fe
Compare
|
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 |
|
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 |
|
Out of curiosity, I see that we can trigger the uninstall dialog via: (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. |
jkarasti
force-pushed
the
602-upgrade-wix-toolset-v2
branch
from
5c9f5fe
to
769271a
Compare
Since running `wix msi validate` with it set to `yes` causes an error.
Due to limitations of the xml.etree.ElementTree library, add the items in the root element as a dictionary
This is a new default and makes authoring slightly simpler without any functional changes.
- Rename variables to be more clear about what they do: - reorganise code - simplify a few checks
…pRef` With this, all the files are organised into Components, each of which points to a Directory defined in the StandardDirectory element. This simplifies the Feature element considerable as only thing it needs to include everything in the built msi is a reference to `ApplicationComponents`
- rename for clarity - remove unnecessary checks
…maryInformation elements directly
Also reduce duplication slightly by definig `build_dir`, `cx_freeze_dir` and `dist_dir`
- WiX Toolset v3 used to validate the msi package by default. In v5 that has moved to a new command, so add a new validation step to the script. - Also emove the step that uses `insignia.exe` to sign the Dangerzone.msi with the digital signatures from its external cab archives. In WiX Toolset v4 and newer, insignia is replaced with a new command `wix msi inscribe`, but we tell wix to embed the cabinets into the .msi (That's what`EmbedCab="yes"` in the Media / MediaTemplate element does) so singning them separately is not necessary. [0] [0] https://wixtoolset.org/docs/tools/signing/
…ot uninstalled by an msi built with WiX Toolset v5 Fix the issue by adding some extra functionality to the "Next" button on the welcome screen of the installer. When the user clicks it to proceed with the installation this: 1. Flips the install scope to "perUser" which is the default in WiX v3 2. Finds the older installation 3. And finally flips the scope back to "perMachine" which is the default in WiX v4 and newer TODO: Revert this once we are reasonably certain there are no affected Dangerzone Installations?
jkarasti
force-pushed
the
602-upgrade-wix-toolset-v2
branch
from
769271a
to
da09566
Compare
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:
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
I removed it.
I think this could work, maybe use a WiX 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. |
This upgrades tooling used to build the Dangerzone.msi installer to WiX Toolset 5.
The Major change in this, apart from what running
wix converton the old generated WiX authoring, is that the target directory for each component gets set in theDirectoryattribute of the component instead of a separateDirectoryelement one level above in the XML hierarchy. All components get grouped under aComponentGroupthat gets referenced in theFeaturetree, 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 validateon the built msi also prints a warning about this.What to set the value of
InstallerVersionto?In this its set to
200to make 64 bit installer work, but it could also be removed or set to the default500, 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 apparentlyset to
perUserby mistake and in WiX 4 its set toperMachineOne workaround for the uninstallation issue would be to set
Scope="perUser"attribute in thePackageelement. 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