installer icon indicating copy to clipboard operation
installer copied to clipboard

Updated msi windows installers to use wix v5

Open jmjaffe37 opened this issue 1 year ago • 22 comments

In moving to wix v5, some of the code could be simplified a bit (mainly: light.exe and candle.exe commands could be combined into one). For end users to use this wix installer creation tool, they will just need to run: dotnet tool install --global wix --version 5.0.0

To install wix 5.0.0 (or they can install another version of wix v4 or wix v5 if they so choose).

jmjaffe37 avatar Apr 10 '24 17:04 jmjaffe37

Notes on my testing: I ensured that both OpenJDK and Microsoft MSI installers are able to be created for all supported versions. I downloaded the resulting MSIs, installed it, checked that the java --version string was correct, and then uninstalled it (with the installer) and made sure that it was gone.

Link to successful run on my fork (although I believe that this PR will also do the same checks): https://github.com/jmjaffe37/openjdk-installer/actions/runs/8624015173

jmjaffe37 avatar Apr 10 '24 17:04 jmjaffe37

Converted to draft due to ongoing release. Note: currently passing all PR checks

jmjaffe37 avatar Apr 10 '24 17:04 jmjaffe37

Note: I had to change the xml propery ID ALLUSERS in Main.wxs.template to ALLUSERSSETUP to avoid a duplicate symbol error that I was getting locally. Here is a link to a run I did on github actions to recreate this error that I previously had locally: https://github.com/jmjaffe37/openjdk-installer/actions/runs/8635538967/job/23673480230#step:7:61

jmjaffe37 avatar Apr 10 '24 17:04 jmjaffe37

Just did a test run using wix 5.0.0 and it seems to have built properly: https://github.com/jmjaffe37/openjdk-installer/actions/runs/8635971450

I also did an internal test run to create Microsoft OpenJDK MSIs and the resulting MSIs passed the test outlined above (install, correct java version, uninstall as expected)

jmjaffe37 avatar Apr 10 '24 20:04 jmjaffe37

EDIT: I figured out how to test different languages. Below shows what I did:

I have confirmed that all available translations (uncommented lines in /wix/Lang/LanguageList.config) are available and work as expected. Method used to test (run on powershell): msiexec.exe /i 'C:\<path_to_msi>\OpenJDK17-jdk_x64_windows_hotspot-17.0.10.0.7.msi' ProductLanguage="<LANGUAGE_CODE>"

This is true for MSIs created with wixV4 and wixV5

jmjaffe37 avatar Apr 10 '24 20:04 jmjaffe37

I think some useful testing will be to upgrade from older installations of Eclipse Temurin and MSFT builds of OpenJDK respectively.

karianna avatar Apr 10 '24 20:04 karianna

Is anyone able to help me test if the installer translations into other languages/cultures is still behaving as expected? I am unsure on how to do that at the moment 😅

@douph1 - Are you able to check for French?

For the rest I recommend adding a note to the installer channel in Slack and giving folks some instructions on how to self serve this PR and test translations.

karianna avatar Apr 10 '24 20:04 karianna

Is anyone able to help me test if the installer translations into other languages/cultures is still behaving as expected? I am unsure on how to do that at the moment 😅

@douph1 - Are you able to check for French?

For the rest I recommend adding a note to the installer channel in Slack and giving folks some instructions on how to self serve this PR and test translations.

@karianna I actually figured out how to do it after posting that comment 😅 I updated it to instead explain how I tested it (I made that update before I saw your reply)

jmjaffe37 avatar Apr 10 '24 20:04 jmjaffe37

Another note: wix v5 was officially release (taken out of beta) on april 5th according to this blog post on their website: https://www.firegiant.com/blog/2024/4/5/wix-v5.0.0-has-been-released/

Since I did my testing on both wix v4.0.5 and wix v5.0.0 (the first non-beta version), would we like me to change the default wix version to 5.0.0?

jmjaffe37 avatar Apr 10 '24 22:04 jmjaffe37

Another note: wix v5 was officially release (taken out of beta) on april 5th according to this blog post on their website: https://www.firegiant.com/blog/2024/4/5/wix-v5.0.0-has-been-released/

Since I did my testing on both wix v4.0.5 and wix v5.0.0 (the first non-beta version), would we like me to change the default wix version to 5.0.0?

Yes please

karianna avatar Apr 10 '24 22:04 karianna

@jmjaffe37 great work getting this sorted! I'll take a more thorough review this week. One note though is that it kinda sucks that we've lost the <?xml version="1.0" encoding="utf-8"?> definition at the top of the files as it means that GitHub no longer highlights the XML files for us. Does v4 not support this at the top?

gdams avatar Apr 22 '24 16:04 gdams

@jmjaffe37 great work getting this sorted! I'll take a more thorough review this week. One note though is that it kinda sucks that we've lost the <?xml version="1.0" encoding="utf-8"?> definition at the top of the files as it means that GitHub no longer highlights the XML files for us. Does v4 not support this at the top?

I can try a run with this added back. I believe the inclusion of this line is optional for wix v4 and v5

jmjaffe37 avatar Apr 22 '24 16:04 jmjaffe37

I can try a run with this added back. I believe the inclusion of this line is optional for wix v4 and v5

There's certainly no harm to add it back if it's optional :)

gdams avatar Apr 22 '24 17:04 gdams

@jmjaffe37 great work getting this sorted! I'll take a more thorough review this week. One note though is that it kinda sucks that we've lost the <?xml version="1.0" encoding="utf-8"?> definition at the top of the files as it means that GitHub no longer highlights the XML files for us. Does v4 not support this at the top?

xml tags have been added back, gh test run suggests it works fine :)

jmjaffe37 avatar Apr 22 '24 18:04 jmjaffe37

My initial testing shows that the default install dir is wrong. See the new installer on the left vs the old installer on the right

Screenshot 2024-04-24 at 10 00 33

Doing some research I think this may be linked to InstallPrivileges="elevated" which can be defined at the <Package /> definition although we don't want to break perUser installs so we may need some logic here

E.g:

<Package InstallerVersion="200" Compressed="yes" InstallScope="perMachine" InstallPrivileges="elevated" />

gdams avatar Apr 24 '24 08:04 gdams

My initial testing shows that the default install dir is wrong. See the new installer on the left vs the old installer on the right

Screenshot 2024-04-24 at 10 00 33 Doing some research I think this may be linked to `InstallPrivileges="elevated"` which can be defined at the `` definition although we don't want to break perUser installs so we may need some logic here

E.g:

<Package InstallerVersion="200" Compressed="yes" InstallScope="perMachine" InstallPrivileges="elevated" />

I can look into this some more, but it looks like the InstallPriveleges attribute is no longer part of the package element in wix v4 or v5. See this page for a list of available attributes: https://wixtoolset.org/docs/schema/wxs/package/#attributes

jmjaffe37 avatar Apr 24 '24 16:04 jmjaffe37

My initial testing shows that the default install dir is wrong. See the new installer on the left vs the old installer on the right

Screenshot 2024-04-24 at 10 00 33 Doing some research I think this may be linked to `InstallPrivileges="elevated"` which can be defined at the `` definition although we don't want to break perUser installs so we may need some logic here

E.g:

<Package InstallerVersion="200" Compressed="yes" InstallScope="perMachine" InstallPrivileges="elevated" />

I got the default installdir to change towhat you wanted, but I had to change the install scope to perMachine as you suggested. This means that the the ALLUSERS variable will be initialized to 1 by default, which is different than what we have done historically (2). Is this the solution that we want? See screenshot below of the installer post installScope change: image

jmjaffe37 avatar Apr 24 '24 16:04 jmjaffe37

/thaw

gdams avatar Apr 30 '24 14:04 gdams

Note: to address a request made here: https://github.com/adoptium/installer/issues/888, I implemented a feature to allow vendors to implement custom licenses for hotspot JVMs. It also allows the vendors to choose whether or not to have the end user read and agree to this license (for example, this is optional for the GPLv2 license). Lastly, I added an explanation of this to the README.md

Successful test run with visible GPLv2 license for Eclipse Adoptium vendor: https://github.com/adoptium/installer/actions/runs/8911988475?pr=851

See the PR check for a successful test run for Eclipse Adoptium vendor MSIs without the license showing (default behavior for all vendors with the GPLv2 license unless specified)

Edit: looks like it was decided to remove this logic. The updates have3 been made to remove it properly.

jmjaffe37 avatar Apr 30 '24 22:04 jmjaffe37

raised https://github.com/adoptium/infrastructure/pull/3545 to install wix v5.0.0 on the CI machines

gdams avatar May 02 '24 12:05 gdams

Ok, I believe that this PR is ready to be merged now!

CC: @gdams, @karianna, @douph1

jmjaffe37 avatar May 02 '24 16:05 jmjaffe37

Probably worth communicating this major change to the whole community and landing all of the related PRs in sync. Then we should ask for volunteers to test various scenarios (fresh install, upgrade from previously installed version with old WIX installer, downgrade etc).

karianna avatar May 04 '24 01:05 karianna

Hi @jmjaffe37 great work. I have seen you have made best effort to maintain dual capability of installer to work perUser or perMachine.

I have added perUser installer capability in https://github.com/adoptium/installer/pull/107

Some time later, Adoptium team have decided that perUser is not the way to go and won't be supported anymore: See https://adoptium.net/installation/windows/

The installer is designed for use on a per-machine basis, not per-user basis, so you can have only one installation of the MSI on a machine for all users.

If ALLUSERS was here it was only because cleanup was not done by the dev who disable perUser capability..

douph1 avatar Jun 03 '24 08:06 douph1

Hi @jmjaffe37 great work. I have seen you have made best effort to maintain dual capability of installer to work perUser or perMachine.

I have added perUser installer capability in #107

Some time later, Adoptium team have decided that perUser is not the way to go and won't be supported anymore: See https://adoptium.net/installation/windows/

The installer is designed for use on a per-machine basis, not per-user basis, so you can have only one installation of the MSI on a machine for all users.

If ALLUSERS was here it was only because cleanup was not done by the dev who disable perUser capability..

Thanks for letting me know @douph1! This raises a few questions for me:

  1. Does this mean that additional work would be necessary to support a perUser installation? From my understanding, my update takes care of the necessary prerequisites to support this.

  2. Do we prefer to turn off the perUser option?

  3. I noticed that the linked page states that only windows x64 is supported, but Microsoft has been using this code to generate installers for x86 and aarm64 without issue. In fact, recent updates have specifically debugged aarm64 installer creation. Is there somewhere I can propose new wording?

Thanks!

CC: @d3r3kk, @gdams, @karianna

jmjaffe37 avatar Jun 03 '24 17:06 jmjaffe37

@jmjaffe37 For that last point (docs) - you can send a PR into the adoptium.net repo.

karianna avatar Jun 03 '24 22:06 karianna

  1. I think TSC don't want to support "perUser", I don't understand why, as it worked by the past. If changing we probably need a little more testing for install/upgrade/uninstall and maybe cohabitation with x64. I have no time for this.

    1. I have no opinion, nor interest, but thanks for asking :)
  2. Adoptium seems to not build x86 MSI installer after JDK 19. JDK 20/21/22 installer for x86 is missing. I don't know why but it is maybe the meaning of "Note: Windows installer packages are supported only on Windows x64 systems." If someone has clue on this we can fix the docs of the website.

Adoptium don't build/test/publish arm64 (AArch64) MSI installer. So this is not documented on the Website, but you can add some wording on wix/README.md

douph1 avatar Jun 21 '24 11:06 douph1