Add Microsoft Installer (MSI) package to releases

[briantist]

This PR modifies the GitHub workflow for releasing to run an additional job that builds an MSI package, and adds it to the release assets (only for AMD64).

The existing build is already taking care of a Windows binary, and there's chocolatey installs available, but MSI packages offer a number of advantages:

• Standard packing format that natively supports install/remove/repair
• Works interactively or unattended
• Works with many software management platforms in enterprises
• Works with Group Policy

The MSI is built with the Wix toolset in Wine and does not require a Windows runner or environment to build, and this uses a pre-built container that is set up with wix.

The existing build and release steps using go-releaser are not changed; the MSI part runs in a separate dependent job, and downloads the .exe from the release produced by the original job, and uploads the resulting .msi as an additional release asset. Here's an example as run on my fork:

• https://github.com/briantist/saml2aws/releases/tag/v2.34.0
• and the full run that produced that: https://github.com/briantist/saml2aws/actions/runs/1792307741

Additionally, I've added a workflow_dispatch trigger that takes a tag input. When triggered this way, you can run the workflow manually against an existing previous release, to add the MSI build to it after the fact:

During that run, it will not re-run the release job (it would fail because go-releaser gets errors trying to upload the assets that already exist), and instead will only run the MSI portion. A run like that can be seen here:
https://github.com/briantist/saml2aws/actions/runs/1792301340

And again this will not change the tag of the existing release, it will use the .exe already uploaded to it to produce the MSI.

So if this PR is accepted, that is a way to go back and add MSIs to a few past releases if desired.

---

Happy to answer any questions or concerns, thanks!

[briantist]

@wolfeidau any chance you might be able to take a look this week? Thank you!

[briantist]

@wolfeidau sorry to ping directly again, but I haven't seen anyone else reviewing or merging PRs, is there anyone who could do so? Thanks for your work on this project!

[briantist]

@wolfeidau or anyone on this project, would really appreciate a look at this 🙏

[briantist]

@Versent @lincheney @mapkon is there anything we can do to get some eyes 👀 on this?

[briantist]

Hi @wolfeidau ! Noticed some new activity on this repo, rebased my branch. Would really appreciate a review!

[briantist]

Hi @wolfeidau @Versent checking in again to see if we could get anyone to take a look at this.

There are no code changes in this PR, it doesn't negatively affect the existing releases (even if it breaks), and it'd be a real win for managed Windows environments, admins, and end users.

[briantist]

Trying to give this another bump.. @wolfeidau @Versent what could I do to get this looked at?

[briantist]

@wolfeidau @Versent what could I do here to get someone's attention? I'd love to discuss this and open up some kind of conversation at the very least. Please let me know! 🙏

I believe this is a useful change, and I've gone out of my way to make it unobtrusive to any existing process in place. It would help for issues like #820 .

[briantist]

would still love to have someone look at this... or chat about it.. @wolfeidau @Versent is there anyone who could take a look?

[briantist]

Hi @mapkon @wolfeidau @lincheney @Versent , I've just updated the PR to keep it up to date with master, would anyone take a look at this?

If the intention is is not to consider this PR, it would be nice to know that as well.. thanks

[mapkon]

Hi @mapkon @wolfeidau @lincheney @Versent , I've just updated the PR to keep it up to date with master, would anyone take a look at this?

If the intention is is not to consider this PR, it would be nice to know that as well.. thanks

@briantist Thank you for the message. I will take a look at the earliest. In the long run, we are formulating a much more sustainable maintenance plan for the project, which will involve a contribution path for the wider community. We hope that will hopefully lead to better cadences when it comes to merging and fixing issues.

Please expect a discussion post soon.

[mapkon]

@briantist The changes in the #962 will need to be merged with yours

[briantist]

@briantist The changes in the #962 will need to be merged with yours

Thank you, I will take a look next week and incorporate those changes. Much appreciated!

[briantist]

@mapkon I've resolved the conflicts and incorporated the changes from #962 .

I've re-tested on my fork:

• Here's a "new" release run (note: the annotations about node.js and set-output were resolved in b1ea919): https://github.com/briantist/saml2aws/actions/runs/4347276063
• Here's a run using a workflow dispatch trigger to add the MSI to an existing release: https://github.com/briantist/saml2aws/actions/runs/4347349959
• Here's the release in question: https://github.com/briantist/saml2aws/releases/tag/v99.0.2

The v99.0.2 release did have the MSI in it already from the "new" release run, and then I deleted just the MSI from the assets to simulate an older/existing release, and ran the workflow dispatch against it to show adding an MSI (back) to it.

Please take a look when you can and let me know if you have any questions or concerns. Thanks!

[mapkon]

@mapkon I've resolved the conflicts and incorporated the changes from #962 .

I've re-tested on my fork:

• Here's a "new" release run (note: the annotations about node.js and set-output were resolved in b1ea919): https://github.com/briantist/saml2aws/actions/runs/4347276063
• Here's a run using a workflow dispatch trigger to add the MSI to an existing release: https://github.com/briantist/saml2aws/actions/runs/4347349959
• Here's the release in question: https://github.com/briantist/saml2aws/releases/tag/v99.0.2

The v99.0.2 release did have the MSI in it already from the "new" release run, and then I deleted just the MSI from the assets to simulate an older/existing release, and ran the workflow dispatch against it to show adding an MSI (back) to it.

Please take a look when you can and let me know if you have any questions or concerns. Thanks!

Thank you! Apologies but I am seeking internal guidance on how to proceed on this given that I am new to this project. I will revert before weeks end

[briantist]

Thank you! Apologies but I am seeking internal guidance on how to proceed on this given that I am new to this project. I will revert before weeks end

@mapkon no problem, thanks for letting me know.
I'm very happy to answer any additional questions or walk through this.

Changes to workflows or the release process can be really tricky to evaluate by PR alone, so if there's additional testing/demonstration I can do on my fork that would be helpful, let me know.

[mapkon]

@briantist @gliptak I have moved the discussion here: #987

Please tag anyone you think can add benefit and thoughts on this. Thank you

[codecov-commenter]

Codecov Report

Merging #766 (aebff57) into master (f5bc286) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master     #766   +/-   ##
=======================================
  Coverage   27.88%   27.88%           
=======================================
  Files          52       52           
  Lines        7379     7379           
=======================================
  Hits         2058     2058           
  Misses       5069     5069           
  Partials      252      252           

Flag	Coverage Δ
unittests	27.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[mapkon]

Decisions made here: #987

I will defer this to the next release to stagger the testing.