Add a workflow that builds and publishes a release via GitHub Actions

[tonyarnold]

This PR proposes to add a new GitHub Actions workflow that will build a release version of Sparkle, and publish it to a draft release on GitHub.

The workflow can be triggered manually from the Actions tab.

This is an initial implementation of this workflow. Future iterations can (and should):

• Automatically publish non-prereleases to CocoaPods
• Use the contents of the change log as a draft body for the release notes
• Consolidate the manual build and release scripts in the repository so that there is no requirement for developers on the project to run any of the release tasks on their local machines - all of it should happen via Actions.

I've no way of testing this properly, so someone from the core team will probably need to take what I've done here, try it and fix any issues that arise when they are doing the first Sparkle 2.x prerelease.

I'm also not sure if there's more that we need to publish here - I can see that there are separate frameworks published for SPM in v1.x. Please feel free to comment or review and I'll add anything I've missed here.

Addresses #1883.

[zorgiepoo]

Thanks for the PR attempt. I didn't have a chance to run it but noticed a few things.

First off, to clarify my aversion for using GitHub's artifact zip tool and using ditto instead, see eg actions/upload-artifact#37 which apparently you have already noticed before :)

However if you use actions/upload-artifact on a .zip file, it's likely going to .zip the zipped file again (actions/upload-artifact#39) when uploading it which is problematic..

The Distribution target invokes to make-release-package which already builds a SPM zip package, so we don't need to replicate that in a GitHub action. It currently uses zip -rqyX -9, we ought to change it to use ditto. It also includes more things we need to distribute other than just the xcframework (CLI tools, XPC Services, ..)

We haven't made the decision to drop the non-xcframework variant so we may still need to provide it.. although, it might make sense to, because there's some redundancy here in packaging both versions. Unfortunately though the xcframework variant for SPM needs to be a .zip and it does not presently compress nearly as nicely compared to the tar.xz version (> 4x size difference).

The last hurdle (from #1751) which I'm now just realizing.. is that when make-release-package is not run in CI, it reads the latest git tag and computes/writes the SPM checksum of Sparkle-for-Swift-Package-Manager.zip, and when we make a tag and build locally via make release from the Makefile, release-move-tag.sh is invoked which makes a final commit with the spm checksum of the zip included in the Package.swift, and then moves the latest tag (which we created prior) forward to that commit.

[tonyarnold]

Yeah, I hadn't accounted for the double-zip bug in actions/upload-artifact - it doesn't make any difference when you're downloading directly, but there's got to be a better way to handle this. Perhaps we can avoid artefacts entirely in this instance.

We haven't made the decision to drop the non-xcframework variant so we may still need to provide it

That's fine - no problem at all. I'll add the framework variant back.

the xcframework variant for SPM needs to be a .zip and it does not presently compress nearly as nicely compared to the tar.xz version

Not a problem - providing the frameworks compressed as both zip and a gzipped tape archive are fine 👌🏻 Maybe we could shift it over the bzip2 or lzma and potentially save even more space?

it reads the latest git tag and computes/writes the SPM checksum

Yeah, this will be tricky. Ideally, the CI user has no ability to change the repo it is building from, but this is definitely a problem. I'll give it some thought, but the obvious (but perhaps not great) answer is to let the workflow calculate that checksum and update the package manifest in the repo directly (ie, follow through on the non-CI script and then push the changes back).

I wonder how other projects are handling this?

[tonyarnold]

Ok, I've updated the workflow to not use artefacts at all, and to publish both the .tar.xz and the .zip.

It will now also push the moved tag back up to the server.

[zorgiepoo]

make release actually outputs the build output in some temporary directory you can't determine ahead of time, so you may just need to run xcodebuild specifying the build directory and run release-move-tag.sh yourself.

Do you know what Xcode version is being used? In the pull request CI workflow we specify it by specifying the DEVELOPER_DIR (I think at this moment we want Xcode 12.5 on Big Sur which is the latest stable).

If I understand right, we will need to commit/push a tag we want first before running this workflow, so there will be a small window where the tag pushed is wrong. Is there any way to pass the tag we want as a parameter so we can just push the tag remotely once..? Otherwise this may be an acceptable consequence. (let's start with tagging 2.0.0-beta.1 by the way.. Note before we made a mistake of using a tag like 2.0.0beta1 which didn't follow semver with the missing dash so let's not repeat that mistake)

In make-release-package.sh we still likely need to change zip -rqyX -9 to ditto as I mentioned in an earlier comment. In this script we actually extract the non-spm tar archive to a temp directory, run codesign --verify --deep on all binaries to make sure they were archived/generated correctly. We may want/need to do the same thing with the spm archive we create. GitHub Actions for example had a prior bug where they were generating corrupt archives (actions/runner-images#2619) and this can catch that sort of issue some of the time..

make-release-package.sh currently uses a dummy git tag when ran in CI. It will need to use the actual tag now.. See the comments in #1751 (comment) about unit tests failing. Because we specify fetch-depth as 0 now (and we didn't used to back then), we may be able to retrieve the latest tag now in CI; we just need to simply update the code to omit the $CI check.

Not a problem - providing the frameworks compressed as both zip and a gzipped tape archive are fine 👌🏻 Maybe we could shift it over the bzip2 or lzma and potentially save even more space?

We used to use bzip2 I think, Kornel has experimented around with better compression.. SPM binary packages enforces us to use zip though as far as I know.

[zorgiepoo]

Thanks for writing this by the way, this can potentially save us lot of work, if it works!

I've no way of testing this properly, so someone from the core team will probably need to take what I've done here, try it and fix any issues that arise when they are doing the first Sparkle 2.x prerelease.

@kornelski feel free to give Tony access to test this and iron out issues. It may save us work and we should create a beta 1 tag anyway =)

[zorgiepoo]

There is also cocoa pods but I think we should just do that push manually, and (..maybe..) ignore it for beta versions. The important bit is that $CURRENT_PROJECT_VERSION needs to match the version in the pod spec file because that is what make-release-package.sh checks. For beta releases we could change $CURRENT_PROJECT_VERSION to include a beta number and then it would sync to the pods file and Package.swift file.. although I don't know if it's strictly necessary, it might be..

[tonyarnold]

I feel like after we've automated the releases via GH Actions, there are probably a number of other things we can rationalise and improve.

Is there any way to pass the tag we want as a parameter so we can just push the tag remotely once..?

Yes, we could make it a manual workflow that you execute directly and provide a string as the tag.

Initially, someone would then need to go in and fill in the release notes, etc - but all of that could be automated. Reading the changelog into the release is feasible, so long as it's kept up-to-date pretty religiously.

Let's get this working as a manual workflow to start with, and then we can talk through improving it so that nobody needs to be doing any builds or uploads from their personal machines.

[tonyarnold]

make release actually outputs the build output in some temporary directory you can't determine ahead of time, so you may just need to run xcodebuild specifying the build directory and run release-move-tag.sh yourself.

It's actually fine - if you predefine the BUILDDIR environment variable before running, it will use that instead of the temporary directory.

Do you know what Xcode version is being used? In the pull request CI workflow we specify it by specifying the DEVELOPER_DIR (I think at this moment we want Xcode 12.5 on Big Sur which is the latest stable).

Whatever the default is - I believe it's Xcode 12.5.1 on GitHub Actions right now. For building releases, I feel like this is probably the right call, but yes, it should be locked so that we're not automatically updated to Xcode 13 when it becomes the default.

[zorgiepoo]

Yes, we could make it a manual workflow that you execute directly and provide a string as the tag.

Curious how you pass an input in the drafts page, but neat that this is possible.

Some things I mentioned above and creating tasks:

• Use tag in make-release-package.sh (see comment)
• ditto instead of zip in make-release-package.sh
• codesign verification of extracted binaries from SPM archive (similarly, see verification done in make-release-package.sh on *tar.(xz|bz2))
• give Tony access to run / make a beta tag
• User agent string and XPC Service version validation

[tonyarnold]

Curious how you pass an input in the drafts page, but neat that this is possible

It's not happening on the drafts page. I believe the process is that you go to the "Actions" tab, and then select the "Create Draft Release" workflow - there should be a button in the detail of that page that allows you to trigger the workflow, and that is where the input field for this value will be.

The workflow will then build all of the binary bits, and make a very basic draft release that you can then visit to tag and publish.

Long story short, the tag isn't created at all as part of this workflow - it won't be created until you publish the draft release that the workflow creates. This gives you the opportunity to attach release notes and double-check that the binaries are OK before making it final.

It does make me wonder if maybe we should be using input value to populate the CURRENT_MARKETING_VERSION and use that in every place that we currently look up the tag on disk?

[zorgiepoo]

It's not happening on the drafts page. I believe the process is that you go to the "Actions" tab, and then select the "Create Draft Release" workflow - there should be a button in the detail of that page that allows you to trigger the workflow, and that is where the input field for this value will be.

The workflow will then build all of the binary bits, and make a very basic draft release that you can then visit to tag and publish.

Long story short, the tag isn't created at all as part of this workflow - it won't be created until you publish the draft release that the workflow creates. This gives you the opportunity to attach release notes and double-check that the binaries are OK before making it final.

This sounds good and reasonable.

It does make me wonder if maybe we should be using input value to populate the CURRENT_MARKETING_VERSION and use that in every place that we currently look up the tag on disk?

I think we will need to pass an input for the CURRENT_TAG_NAME or something. The CURRENT_PROJECT_VERSION needs to be equal to it, or a CURRENT_TAG_NAME needs to have a prefix "$CURRENT_PROJECT_VERSION-" in case of prerelease identifiers (CURRENT_PROJECT_VERSION which is used for bundle versions shouldn't include prerelease identifier info)

We would then need to read the current tag name we pass, instead of trying to fetch the latest tag on disk as you say, to:

• Update git-version-info.sh so short version string of various bundles is set to the new tag name (git hash won't be needed)
• Updating version and tag in Package.swift
• Updating version in podspec file (and fixing the pre-validation check about version == CURRENT_PROJECT_VERSION)
• Updating the Sparkle*.tar.(xz|bz2) filename.

(or pass in the project version and the optional prerelease identifier as two different input fields and we update the current project version..)

The draft template should probably output the tag name we used somewhere so we can copy and paste, or prefill the tag field if possible.

[tonyarnold]

What we could consider doing is allowing the input of a value into the workflow that can be in SemVer format, ie: 2.0.0-beta.1, and use a script to strip off the prerelease information from that when it is set in the bundle. So the tag would be correct (2.0.0-beta.1), and the CFBundleVersion/CFBundleShortVersionString would be 2.0.0.

This does raise questions in my mind, though… I've always used a monotonically increasing integer for the CFBundleVersion value in my app's plists, but the docs are very clear about only using a strict non-prerelease version of SemVer (as you pointed out).

How do we resolve this? It's fine to mark a build as pre-release on GitHub, but betas and the like should be clearly marked as thus in their metadata/Info.plist. How has this been handled for Sparkle in the past? I'd like to be consistent with the project's standards, if they're not broken.

[zorgiepoo]

Sparkle has a post install script (set-git-version-info.sh) that modifies the short version string on the framework and various bundles to be the current project version ($CURRENT_PROJECT_VERSION) appended by the tag/hash information (after the first dash from git describe --tags --match '[12].*'). If a tag was just created, this means the short version string will be exactly that tag name, I believe. Because the tag isn't created as part of the workflow you're advising, you would thus need to pass the tag name as input to this script.

In my opinion the CFBundleVersion and CURRENT_PROJECT_VERSION should not include pre-release info, leave it to the short version string which will show up when you get info in Finder. For 2.x's entire lifespan so far, the CFBundleVersion has basically never changed, but the CFBundleShortVersionString has meaningfully been updated on every commit.

The apple docs may say you shouldn't include prerelease info in the (localizable..) short version string, but I would really not worry about that too much, and it's less important for a framework bundle versus an app (versus an App Store app on App Store Connect) IMO.

What we could consider doing is allowing the input of a value into the workflow that can be in SemVer format, ie: 2.0.0-beta.1, and use a script to strip off the prerelease information from that when it is set in the bundle.

This could be a reasonable approach for updating the CURRENT_PROJECT_VERSION in the project config. Or we could more manually require the CURRENT_PROJECT_VERSION to be updated beforehand when we need to update the CHANGELOG manually.

[tonyarnold]

For my own projects, I use the result of the following as my CURRENT_PROJECT_VERSION:

git rev-list --count HEAD | sed -e 's/^[ \t]*//'

This produces an integer that increases on every commit to the current branch. If we run this against Sparkle's 2.x branch right now, it produces a build of 3794.

Thoughts?

[zorgiepoo]

That seems less valuable than the git describe invocation we use for the short version string which appends a hash and I don't think it's solving a significant problem. We do want a user to Get Info on a Sparkle framework bundle and be able to tell from the short version what build it is, and I'd rather not add more post Info.plist alterations.

I think we need to maintain the current display version and base project version. So my first thought would be in our ConfigCommon.xcconfig file doing:

CURRENT_PROJECT_VERSION = 2.0.0 // used by dylib version, set-git-version-info script, CFBundleVersion, CFBundleShortVersionString (but later modified by git version info script to include prerelease tag / git hash), C preprocessor
CURRENT_PROJECT_DISPLAY_VERSION_SUFFIX = "-beta.1" // if it's empty, there's no pre-release identifier
CURRENT_PROJECT_DISPLAY_VERSION = $(CURRENT_PROJECT_VERSION)$(CURRENT_PROJECT_DISPLAY_VERSION_SUFFIX) // used for .tar distribution filenames, SPM, cocoapods

I'll think more about this.

[tonyarnold]

Could I inject an argument that the CURRENT_PROJECT_VERSION should increase over time with each build? I know it's not that big a deal for a framework to have incorrect details on disk, but it sets a bad example for the wider community who do need to increase this value over time. They'll potentially look at examples like this and follow them.

Ultimately, I'll follow whatever guidance you give here, but I felt like I needed to inject this.

Also, the project should probably switch to using CURRENT_PROJECT_VERSION (for CFBundleVersion) and MARKETING_VERSION (for CFBundleShortVersionString ) consistently as they are used within Xcode's build settings.

[zorgiepoo]

I think I'm ok with something like this:

// It doesn't matter what value this initially is, as long as we increment it monotonically and it starts at a value > 2
CURRENT_PROJECT_VERSION = 174

// leave empty if there's no prerelease, otherwise it is in format of eg: "-beta.1"
MARKETING_VERSION_PRERELEASE_SUFFIX = "-beta.1"

// Changes to set-get-version-info.sh will be required to properly format the appended short version string now that we include the prerelease suffix here
MARKETING_VERSION = $(SPARKLE_VERSION_MAJOR).$(SPARKLE_VERSION_MINOR).$(SPARKLE_VERSION_PATCH)$(MARKETING_VERSION_PRERELEASE_SUFFIX)

I don't care to update a monotonically increasing bundle version on every commit, only on releases that we tag; this is a proper approach that apps take during development. So basically this is an edge case for beta releases we may publish soon. I also don't want to add more to our post install scripts (I'd more like to remove them but alas we need the set-git-version info one at least to ensure the XPC Services / frameworks are aligned on a commit level). If we publish a release locally (I want to keep this workflow working as a fallback), we can manually update the project version; we'd have to update the marketing version and CHANGELOG alongside anyway.

It's not necessary to depend on git to increment the counter (can read current value and write value + 1). However, if it's easier that way, then I don't see why not. The set git version info script will still be needed as well as the counter/short git hash it outputs, which is a more reliable indicator to see if things are aligned or what build a user has. There's a chance making this change will break the SPUXPCValidateServiceIfBundleExists() function and set-get-version-info.sh formatting for pre-releases, so those likely will need to be updated. This workflow will still need to pass the tag name we'll want to create to set-get-version-info.sh - it currently assumes we create the tag beforehand.

edit: updated again; we should indeed include pre-release suffix in marketing version macro

[tonyarnold]

I don't care to update a monotonically increasing bundle version on every commit, only on releases that we tag; this is a proper approach that apps take during development.

You're absolutely right: for simple projects, the count of the rev list is fine, but it's not reliable - especially if there is ever an intent to go back and release patch updates to older releases, or if you're branching all over the place.

It sounds to me that sticking with a manual versioning approach works better for how this project currently runs? That's fine - we can just leave all of that out of the GitHub Actions workflow, and version details will be the responsibility of the developer, not CI.

I've updated the Actions workflow to extract the marketing version to use as the draft tag, and the name of the draft release on Github, so that it all needs to be setup in the repo beforehand. The workflow won't ask for the tag details, as those should already be available in the repo.

The set git version info script will still be needed as well as the counter/short git hash it outputs, which is a more reliable indicator to see if things are aligned or what build a user has.

I've updated the script to extract the commit hash properly, even with a prerelease suffix present. Was there more in SPUXPCValidateServiceIfBundleExists() that needed to be done?

Sorry, I know this is dragging on - I'm keen to get it right.

[zorgiepoo]

You're absolutely right: for simple projects, the count of the rev list is fine, but it's not reliable - especially if there is ever an intent to go back and release patch updates to older releases, or if you're branching all over the place.

It sounds to me that sticking with a manual versioning approach works better for how this project currently runs? That's fine - we can just leave all of that out of the GitHub Actions workflow, and version details will be the responsibility of the developer, not CI.

I think this sounds fine and reasonable.

I've updated the script to extract the commit hash properly, even with a prerelease suffix present. Was there more in SPUXPCValidateServiceIfBundleExists() that needed to be done?

Let me review other parts of the PR first and then I can address the logic there (based on what we end up doing aside from this)..

[tonyarnold]

Ok, I've applied the requested patches, and I think I've covered the issues you've raised.

I apologise for taking so long, and for the shoddy work in the middle there - I've got three children under the age of 10 homeschooling during a lockdown, so things are pretty chaotic.

Thanks for being patient.

[tonyarnold]

I've also rebased onto the latest code from the 2.x branch so the PR should apply cleanly.

[zorgiepoo]

Just had two small comments, one of them about the ditto extraction, otherwise looks good to me (barring from testing the github workflow). There's no rush on this on our end.

[zorgiepoo]

Thank you. Changes look good to me although I have not tried the workflow (and it seems like it would be more productive to have changes merged first to try it out). There are also some good project versioning changes here with having the CURRENT_PROJECT_VERSION being an increasing number, and MARKETING_VERSION as what we want to market, as well as making updating the Package.swift and cocoa pods spec files consistent.

I'll give a chance @kornelski to take a look, otherwise I'll just end up merging this. I won't get to trying the workflow myself until maybe the weekend although you are free to try; I'll invite you to the project. We will first need a PR that bumps the marketing version to 2.0.0-beta.1 (and current project version too consequently) though.

[thebluepotato]

From a quick read-through of the changes, everything seems ok (and even improved!) on the SPM front

[zorgiepoo]

Thanks for taking a second look! 🎉.