CI: Split Up Windows Signing Job

[derrod]

Description

Splits up the windows signing workflow into "package" and "updater" steps which are responsible for the singing/packaging and creation of updater files/delta updates respectively.

Motivation and Context

• Speed up signing/installer creation step (12 mins -> 6 mins)
• Add Windows artifacts to automatically created draft release
• Let delta generation job run as long as it needs without blocking release creation (probably more than 15 minutes if we throw ina CEF update for good luck)

How Has This Been Tested?

Partially tested on my fork, will require follow-up test once finalised.

Types of changes

• Tweak (non-breaking change to improve existing functionality)

Checklist:

• My code has been run through clang-format.
• I have read the contributing document.
• My code is not on the master branch.
• The code has been tested.
• All commit messages are properly formatted and commits squashed where appropriate.
• I have included updates to all appropriate documentation.

[PatTheMav]

Wouldn't it make more sense to split this up into 2 separate actions (see also https://github.com/actions/cache which maintains separate save and restore actions in subdirectories).

That way you can use bouf/package and bouf/sign separately and don't have add unnecessary inputs.

Also allows us to keep each usage scenario self-contained in a bespoke action for each without having to increase complexity of a single action.

[derrod]

There's still a decent bit of overlap and I didn't want to create two separate places to keep up to date as far as third-party actions and hashes goes. There are also two steps that could be removed entirely now given that we have decided not to sign manifests on CI for the foreseeable future.

Edit: Another one could be removed if we stop double-zipping our Windows builds on CI. With the new artifacts v4 system that is no longer necessary for speed.

[PatTheMav]

There's still a decent bit of overlap and I didn't want to create two separate places to keep up to date as far as third-party actions and hashes goes. There are also two steps that could be removed entirely now given that we have decided not to sign manifests on CI for the foreseeable future.

Edit: Another one could be removed if we stop double-zipping our Windows builds on CI. With the new artifacts v4 system that is no longer necessary for speed.

Right, I'll hold off on further review until those refinements have landed.

[PatTheMav]

One thing that should definitely be changed is the name of the action however - every other action in the repo has a name that clearly states its intent/job (e.g. "build-obs", "run-clang-format", "steam-upload"), so this one sticks out.

I'd prefer if bouf itself had gotten a more descriptive name that would clearly communicate what it does, but I guess that ship has sailed already.. 😅

[derrod]

Updated now to properly support the dual-signature feature we want to use for game capture in 30.2 while we transition to the new cert. Also cleaned up a few things and tested it to make sure the draft release is created with the installer and ZIPs properly added and hashed.

One thing that should definitely be changed is the name of the action however - every other action in the repo has a name that clearly states its intent/job (e.g. "build-obs", "run-clang-format", "steam-upload"), so this one sticks out.

By that, do you meant the name of the folder (actions/bouf) should be changed?

[PatTheMav]

Updated now to properly support the dual-signature feature we want to use for game capture in 30.2 while we transition to the new cert. Also cleaned up a few things and tested it to make sure the draft release is created with the installer and ZIPs properly added and hashed.

One thing that should definitely be changed is the name of the action however - every other action in the repo has a name that clearly states its intent/job (e.g. "build-obs", "run-clang-format", "steam-upload"), so this one sticks out.

By that, do you meant the name of the folder (actions/bouf) should be changed?

Yep, something like actions/windows-bundler or something would be more canonical - what does bout actually stand for?

[RytoEX]

Commit message nits:

• sigining -> signing
• windows -> Windows

Otherwise, seems fine.

[derrod]

Yep, something like actions/windows-bundler or something would be more canonical - what does bout actually stand for?

It stands for Building OBS Updates Fast(er).

[PatTheMav]

Is the package step (of the workflow and the action) ever run without a consecutive update step as which makes "merging" two separate workflows and actions into the same file necessary.

[derrod]

Is the package step (of the workflow and the action) ever run without a consecutive update step as which makes "merging" two separate workflows and actions into the same file necessary.

They're always run consecutively, they're just split up so that the creation of the draft release is not blocked by the creation of the updater files, which can take a long time (~28 minutes). Since the action in both cases is almost identical save for two steps and a command line parameter I didn't want to split it up and have to maintain two files that are mostly the same.

[PatTheMav]

Is the package step (of the workflow and the action) ever run without a consecutive update step as which makes "merging" two separate workflows and actions into the same file necessary.

They're always run consecutively, they're just split up so that the creation of the draft release is not blocked by the creation of the updater files, which can take a long time (~28 minutes). Since the action in both cases is almost identical save for two steps and a command line parameter I didn't want to split it up and have to maintain two files that are mostly the same.

I'd argue that the latter is preferable (see also the code formatting actions) because it puts logical representation of workflow actions/steps before implementation (as the implementation can and should change over time). If something about the packaging action changes, it then requires ensuring that both packaging and signing will work because both are intermingled in the same "thing".

Also if updater creation takes that long, I'd just throw it into the publish workflow which is run after a release has been created (that's also when we publish to Flathub, etc.) so it should not even run as part of a push IMO.

[derrod]

Previously we decided that we wanted to have the updater files ready for testing and deployment before we publish a github release, so that's a question for @RytoEX if we want to go back on that. We could then also move the Sparkle job into the publish workflow and increase the number of deltas there so we cover more than one version as well (Windows currently goes back ~2 years) and just let it take however long it takes.

[RytoEX]

Previously we decided that we wanted to have the updater files ready for testing and deployment before we publish a github release, so that's a question for @RytoEX if we want to go back on that. We could then also move the Sparkle job into the publish workflow and increase the number of deltas there so we cover more than one version as well (Windows currently goes back ~2 years) and just let it take however long it takes.

I think I'm okay with experimenting with this. It would mean adjusting our overall release workflow a bit (GitHub would end up being first, with everything else following after), but I'm not strictly opposed. @PatTheMav ?

[PatTheMav]

Previously we decided that we wanted to have the updater files ready for testing and deployment before we publish a github release, so that's a question for @RytoEX if we want to go back on that. We could then also move the Sparkle job into the publish workflow and increase the number of deltas there so we cover more than one version as well (Windows currently goes back ~2 years) and just let it take however long it takes.

I think I'm okay with experimenting with this. It would mean adjusting our overall release workflow a bit (GitHub would end up being first, with everything else following after), but I'm not strictly opposed. @PatTheMav ?

Putting the Sparkle job in the release workflow seems beneficial to me as well.

[derrod]

With that in mind I would then propose the following approach:

• Rename the bouf action to windows-signing to have a more descriptive name
  • It will remain part of the existing reusable workflow so it can be pinned to a specific commit hash that is validated by the Google Cloud authentication mechanism
• Make patch generation a separate action called windows-patches
  • Part of release and dispatch workflows (so it can be manually rerun if necessary)
• (In a separate PR) Move Sparkle action to the release workflow and increase the number of deltas

This will also make way for the following changes:

• The signing action can be optimised to only download the previous release from GCS, and then sync back its output automatically
• The patch generation action can be updated to use a static read-only access key/secret to fetch builds via the S3-compatible GCS API
  • This will allow it to no longer require approvals as it will not have access to sensitive APIs
• We could move the patch generation workflow to Linux runners as it will no longer require any Windows-specific features, which may help speed it up a bit more

[derrod]

Implemented the changes outlined above, tested it on my fork as well and seems to work fine.

[PatTheMav]

Looks good to me. How much work would it be to add arm64 Windows builds to this?

Mainly I'm interested in how much more work would it be to make the architecture a variable now vs later when we actually produce arm64 builds (so does YAGNI apply or not).

[derrod]

To be honest I've not really thought about how we'll handle multi-architecture Windows at all yet. Will require some further thought about how we want to implement that in the Windows updater setup, e.g. go the Sparkle route and have different feeds entirely, or do we want something unified (no need to duplicate non-binary files), etc.

[RytoEX]

Looks fine other than my question about rclone/pandoc versions.

[RytoEX]

Seems fine.