Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release improvements #640

Draft
wants to merge 93 commits into
base: main
Choose a base branch
from

Conversation

rgoldberg
Copy link
Contributor

@rgoldberg rgoldberg commented Nov 16, 2024

@phatblat

I've incorporated your fixes from https://github.com/mas-cli/mas/tree/638/release-fixes into the branch for this PR, and the 1.8.8-beta.1 test run doesn't need to be release, so we should be able to delete the following branches:

I've rebased this PR's branch on updated main, so if you want to look at or test my release process, please force checkout this PR's branch.

Release Workflow

We shouldn't start our automated release workflow after the release has been published (on: release: types: published), because the release then will have already been made available to the public.

If anything fails or is suboptimal (like poorly worded generated release notes) in the workflow, we'd need to either delete, modify, or supersede the botched release, all of which are suboptimal.

It's probably best to split the release process into stages:

Stage 1: Generate Draft

Somehow start a process that creates a GitHub Release draft, and that, while still a draft, performs all the other release steps except for asking the homebrew-core maintainers to approve an mas formula bump PR, or for labelling / approving the bump PR for the mas-cli/tap/mas formula:

We should support only one way to start the stage. Start options include:

  • When a tag matching a version string regex (v\d+\.\d+\.\d+(?:-(?:alpha|beta|rc)\.\d+)) is pushed to the main branch of the mas-cli/mas repo.
  • When a draft release is made manually via the GitHub web or cli interface

This stage would:

  • Validate inputs (like version tag)
  • Checkout the version tag
  • Bootstrap
  • Build
  • Package the *.pkg(s)
  • Create a GitHub Release draft which would:
    • Be a prerelease if the version tag contains a -
    • Have autogenerated release notes
  • Upload the *.pkg(s) to the draft
  • Create a mas-cli/tap/mas formula bump PR (possibly submitted as a draft PR)
  • Submit a formula bump draft PR (but not a non-draft PR) to homebrew-core, as the latter would implicitly ask the homebrew-core maintainers to approve the bump

Stage 2: Manual Draft Inspection / Modification

This step could be skipped, but I advise against it. If skipped, stages 1 & 3 could be combined into one continuous script.

The releaser inspects everything created in stage 1, including the GitHub Releases draft (including the generated release notes) & related external PRs. The releaser may then either:

  • Publish the draft as a release as is
  • Modify the draft, then publish the draft as a release
  • Delete the draft

Stage 3: Publish Release / Delete Draft

If the releaser deletes the GitHub Release draft from stage 2, then an on: release: types: deleted workflow will close the related external PRs.

If the releaser publishes the draft, then an on: release: types: published workflow will switch the homebrew-core mas & mas-cli/tap/mas formula bump PRs from drafts to normal submissions, and will apply labels / approve the latter PR.

Dry Run

Need we support explicit dry runs?

If we create a GitHub Release draft as per above, we can always delete the draft, and we can always close the generated draft PRs without them having been approved or merged.

Tag Type

Should we use lightweight or annotated tags for versions?

Briefly noted in #655.

Legacy Version Patch Branches

If we ever release patches for legacy mas versions, they should be done in a legacy version patch branch.

Given that we aren't patching legacy versions right now, the first revamp of the new release system doesn't need to support it, but we might want to think through the workflow to support it in the future.

See #655 for details about legacy version patch branches & other tag-related proposals.

Details

This is my WIP of improvements for the release process, plus some unrelated cleanup.

I haven't tested it, as I'm worried about accidentally releasing a new version.

I've incorporated all your changes from your 638/release-fixes, so that can be deleted.

Can you let me know if my changes are good, should be modified, or should be discarded?

The general principle of this PR is simplicity:

  • Remove extraneous files from mas-cli/mas repo:
    • Makefile
    • .github/event.json
    • Many bash scripts
    • Homebrew formulae: store each only in its respective tap repo, e.g., homebrew/homebrew-core or mas-cli/tap
      • Bump homebrew-core & mas-cli/tap/mas formulae in taps instead of locally generating / modifying them.
        • Do not create a manual pull request for the custom tap.
    • Generate files only when absolutely necessary, but git ignore them, e.g., Sources/mas/Package.swift
  • Do not commit or branch in mas-cli/mas on release.
  • Store values in as few places as possible, e.g.:
    • Store version in mas-cli/mas repo only in version tag, not in any files or in any branch names.
  • Reuse existing functionality (like brew bump-formula-pr) instead of reinventing the wheel.
  • Standardize:
    • Ensure the same tools are used everywhere, e.g.:
      • ensure /bin/bash is used everywhere instead of some places using bash from the $PATH.
    • Remove dependencies whose functionality can be performed by other things, e.g.:
      • sd: script that used it was deleted, but if script had been kept, should use bash scripting instead
      • mise: use brew bundle instead
  • Scope code, e.g.,:
    • Formulae should be bumped only during a release, so do not perform bumps in scripts called by release workflow; instead, embed bumps in the workflow itself, otherwise people might incorrectly run bump scripts outside of the workflow.
      • If, under extraordinary circumstances, someone must bump the formulae outside of the release process, they should know enough about the release process to know that they should just copy the bump commands from the workflow.
  • Limit pathways, prevent bad values, & make config more explicit, e.g.:
    • bash scripts should accept only arguments, not environment variables.
    • Release workflows & bash scripts should:
      • Require that the release tag is a properly formatted version tag.
      • Checkout the release version tag, then use that version tag as the source for the version & for the git revision.
      • Not accept arguments, environment variables, or anything else to specify version, tag, branch, revision, etc.
  • Replace script/version --write & script/version_bump with script/generate_version_info_for_swift
  • When working locally, versions used (e.g. in Sources/mas/Package.swift) for revisions that aren't directly tagged with a version tag are now specified by the full git describe --tags output, which includes the current revision & the number of commits since the tag.
  • Merge all release.yml jobs into one.
  • bash script cleanup:
    • cd into project root at beginning of every bash script.
    • Use -u in all bash shebangs.
    • Lowercase all bash non-environment variables.
    • Quote as appropriate.
    • Include braces in all variable uses.
    • Use printf instead of echo.
    • Write to stderr where appropriate.
    • Inline single-use variables.
    • Improve script outputs.
    • Simplify & standardize scripts.
  • Fix informational URLs throughout repo:
    • Replace redirecting links.
    • Remove defunct links.
    • http -> http.
  • Ensure all files have one & only one trailing newline, and no leading newlines.
  • Minor Swift cleanups.
  • Improve documentation & samples.
  • Improve & simplify .editorconfig.
  • Upgrade danger-swift from 3.18.0 to 3.20.2.

Improvements that are being worked on:

  • Instead of automatically running the release workflow when a release is published, manually run the release workflow, which will:
    • Ensure that the given release tag argument is a correctly formatted version tag.
    • Create a draft release.
    • Stop progressing if any step fails.
      • Hopefully will revert prior steps if any step fails.
    • Specify dry run via argument to release.yml.
      • Perform local steps regardless of dry or wet run.
      • Perform dry/wet run external steps depending on setting.
      • If dry run, skip external steps that don't support dry runs.
    • Skip steps that should be skipped for prereleases (like bumping home-brew-core formula).
    • If all succeeds in a wet run, the draft release will be changed to a public release.
    • Automatically approve / label with pr-pull custom tap bump?
  • Why does release.yml output release event info only when name != ''?
  • mas-cli/tap/mas / GitHub Release artifacts:
    • Include version (& platform? & macOS version?) in mas.pkg filename.
    • Provide 1.8.6 bottle for macOS 10.11 & 10.12, because 1.8.7+ requires macOS 10.13+.
    • Manually generate bottles for macOS 10.13 & 10.14, because brew bump-formula-pr probably won't do that.
    • Remove bottles from custom tap that are available from homebrew-core?
    • Instead of using a single universal executable for all supported macOS versions, generate bottles for each supported platform & version of macOS?

Future tasks include:

Resolve #638, #636, #635, #634, #628

@rgoldberg rgoldberg force-pushed the 638-release-improvements branch 6 times, most recently from 03de1d2 to 8306c6e Compare November 17, 2024 00:13
@rgoldberg rgoldberg force-pushed the 638-release-improvements branch 7 times, most recently from 0faa0f8 to ef6bd6e Compare November 18, 2024 06:56
@rgoldberg
Copy link
Contributor Author

@phatblat I've incorporated your fixes from the 638/release-fixes branch in this repo into my branch for this PR, so you can restart using the branch from this PR.

If you have any other fixes that weren't already in that branch in this repo at the time when this comment was posted, please share them with me so I can incorporate them into the branch for this PR.

Thanks.

@rgoldberg rgoldberg force-pushed the 638-release-improvements branch 4 times, most recently from f06fedd to 4856cee Compare November 20, 2024 06:07
@rgoldberg rgoldberg mentioned this pull request Nov 20, 2024
@rgoldberg rgoldberg added this to the 1.8.8 milestone Nov 21, 2024
@rgoldberg rgoldberg force-pushed the 638-release-improvements branch 2 times, most recently from 9016365 to c169e88 Compare November 21, 2024 03:59
…sistent with the shebangs that are used throughout.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
…sion values.

It only seems to be used for testing with `act`. `event.json` should either be generated or be manually created; in either case, it should reside somewhere that is ignored by git, as the only source of version info in git should be version tags.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
…ify & speedup workflow.

Move all environment variables to the top level.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
…into `release.yml`.

Run `git rev-parse …` instead of `git rev-list -n 1 …` for mas-cli/tap/mas.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
Requires corresponding commits to `homebrew-core/mas` & `mas-cli/tap/mas` formulae.

Rename `dstroot` variable as `distributions_root` in `script/package`.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
Should only release revisions that have version tags on them, so should only need a single commit.

`ref:` should default to the reference for the event, which should be `github.event.release.tag_name`, so shouldn't need to explicitly set it.

If any of these changes cause problems, will back them out.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
`script/package` calls
`script/build --universal` instead of requiring the latter to be called before the former.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
…` target has been merged into `mas` target.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
@rgoldberg rgoldberg self-assigned this Nov 21, 2024
@rgoldberg rgoldberg added the 🧽 chore Administrative task: documentation, build, test, release, git, etc. label Nov 21, 2024
… tag & current revision for local (i.e. non-release builds), since releases should all be for a version tagged revision.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
Remove useless `--strict` from `swiftlint` in `script/format`.

Partial mas-cli#638

Signed-off-by: Ross Goldberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧽 chore Administrative task: documentation, build, test, release, git, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve release process
2 participants