Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 51 additions & 5 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,41 @@ on:
push:
branches:
- main
release:
types:
- published
tags:
- 'v*'

permissions: {}

jobs:
goreleaser-check:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This job does a full cross-platform build and the deb and rpm packaging on every PR, and again on every push to main. The Go workflow already builds the same code. The run on main repeats the check that the PR already did on the same code, so it uses about 10 more builds and the nfpm packaging each time.

Suggestion: run this job only on PRs, or only when .goreleaser.yml or go.mod changes.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the PR and main triggers used by the other CLI repos. STF-1869 explicitly asks for both. The snapshot job also exercises release archives and package contents, which the Go test workflow does not cover. Restricting it to .goreleaser.yml and go.mod would miss changes to source code and packaged files. Any reduction in these checks should be considered across the CLI repos.


🤖 Comment by Codex on behalf of Greg.

name: Check GoReleaser
if: github.ref_type != 'tag'
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
fetch-depth: 0
persist-credentials: false

- name: Set up Go
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
go-version: stable
cache: false

- name: Check GoReleaser
uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3
with:
distribution: goreleaser
version: latest
args: release --snapshot --clean --skip=publish

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check does not test the release settings that this PR changes. --snapshot skips validate and publish, and the job does not pass --release-notes. So a broken release: section, or the ignored notes file (see the changelog.disable comment), passes CI. The problem shows only at release time, and the second commit says that this job is there to prevent that.

Suggestion: add a goreleaser check step, and find some way to test the release notes path.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that the snapshot job covers builds and packaging, not the complete publishing path. I kept the shared CI job and separately tested tag-note extraction and GoReleaser's --release-notes handling, including rejection of a missing file. The GoReleaser checks verified the disabled-changelog fix.

I also ran goreleaser check. It reports valid configuration but exits unsuccessfully because the existing archives.builds and nfpms.builds properties are deprecated. Adding that CI gate would need the config cleanup too. No real publishing was tested, and the local notes checks are not a new CI regression test.


🤖 Comment by Codex on behalf of Greg.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: --skip=publish has no effect, because --snapshot already implies --skip=announce,publish,validate. The flag can make a reader think that the snapshot would publish without it.

Suggestion: release --snapshot --clean.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that the flag is redundant in snapshot mode. I kept the command identical to mmdbverify and mmdbconvert. There is no behavior change from removing it, so I left that cleanup out of this PR.


🤖 Comment by Codex on behalf of Greg.


goreleaser:
if: github.event_name == 'release' && github.event.action == 'published'
name: Release with GoReleaser
if: github.event_name == 'push' && github.ref_type == 'tag'
runs-on: ubuntu-latest
environment: release
permissions:
Expand All @@ -24,6 +52,23 @@ jobs:
fetch-depth: 0
persist-credentials: false

# On a tag push, actions/checkout replaces the annotated tag with a
# lightweight tag. Fetch the tags again to get the tag message.
- name: Fetch annotated tags
run: git fetch --force --tags

- name: Get release notes from the tag
run: |
if [ "$(git cat-file -t "refs/tags/$GITHUB_REF_NAME")" != tag ]; then
echo "Tag $GITHUB_REF_NAME is not an annotated tag. Create it with dev-bin/release.sh."
exit 1
fi
git tag -l --format='%(contents:body)' "$GITHUB_REF_NAME" > "$RUNNER_TEMP/release-notes.md"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step gets the tag body by hand. GoReleaser already has {{ .TagBody }}, which it reads from the same %(contents:body). With --release-notes-tmpl and a template that contains {{ .TagBody }}, you do not need this extraction. The check that the tag is annotated and has notes is still useful, so you can keep it. This depends on the fix for changelog.disable.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the existing file-based handoff used by mmdbverify and mmdbconvert. We still need to inspect the tag body to reject blank notes, and passing that same validated file to GoReleaser keeps the validation and release content together. A template would add another file without removing that validation, so I left this unchanged.


🤖 Comment by Codex on behalf of Greg.

if ! grep -q '[^[:space:]]' "$RUNNER_TEMP/release-notes.md"; then
echo "Tag $GITHUB_REF_NAME has no release notes. Create it with dev-bin/release.sh."
exit 1
fi

- name: Set up Go
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
Expand All @@ -35,6 +80,7 @@ jobs:
with:
distribution: goreleaser
version: latest
args: release --clean
args: release --clean --release-notes ${{ runner.temp }}/release-notes.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GoReleaser selects the release tag itself (the highest version tag that points at HEAD). The notes come from $GITHUB_REF_NAME. These two tags can be different.

Example: the v2.0.1 run fails, and the releaser tags the same commit as v2.0.2. If someone re-runs the v2.0.1 run, or if both runs are in the queue, GoReleaser releases v2.0.2 with the v2.0.1 notes.

Suggestion: set GORELEASER_CURRENT_TAG: ${{ github.ref_name }} in the step env.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4ab751e. The GoReleaser step now sets GORELEASER_CURRENT_TAG to github.ref_name, with the same change in the companion workflows. A smoke test put two version tags on one commit and confirmed that selecting the lower tag retained that version and its supplied notes.


🤖 Comment by Codex on behalf of Greg.

env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GORELEASER_CURRENT_TAG: ${{ github.ref_name }}
5 changes: 1 addition & 4 deletions .goreleaser.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ checksum:
snapshot:
version_template: "{{ .Tag }}-next"

changelog:
disable: true

nfpms:
- id: 'mmdbinspect'
builds:
Expand All @@ -68,4 +65,4 @@ nfpms:
dst: '/usr/share/doc/mmdbinspect/README.md'

release:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The release flow now depends on GoReleaser creating a draft release and publishing it at the end. But neither replace_existing_draft nor use_existing_draft is set. If an asset upload fails after the draft exists, "Re-run job" does not find that draft, because GetReleaseByTag skips drafts. GoReleaser then creates a second draft and publishes it. The first draft stays on the Releases page until someone deletes it.

Suggestion: set replace_existing_draft: true (or use_existing_draft: true) in this release: section.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left the draft settings consistent with mmdbverify and mmdbconvert. This PR does not add automatic recovery after a partial upload. Choosing whether to replace or reuse a draft changes how existing release assets are handled, so I would treat that as a shared release-recovery change. The possible leftover draft remains a limitation.


🤖 Comment by Codex on behalf of Greg.

target_commitish: "{{ .FullCommit }}"
name_template: "{{.Version}}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changelog: disable: true (line 45, outside this diff) is still set. With it, GoReleaser ignores the --release-notes file that the workflow passes at .github/workflows/release.yml:83. Every GitHub release then gets an empty body. Immutable releases lock the release after it is published, so you cannot add the notes later.

I tested this with GoReleaser 2.17.1. With changelog.disable: true, the log shows "skipped generating changelog" and GoReleaser never reads the file. When I enabled the changelog, GoReleaser loaded the file. The GoReleaser docs also say this.

Suggestion: remove the changelog.disable setting.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4ab751e. Removed changelog.disable here and in the affected companion repos. With GoReleaser 2.18.2, a smoke test loaded the supplied notes file and failed when that file was missing. No release was published during testing.


🤖 Comment by Codex on behalf of Greg.

15 changes: 9 additions & 6 deletions README.dev.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# Prereqs

- You must have the [GitHub CLI tool (gh)](https://cli.github.com/) installed,
in your path, and logged into an account that can make GitHub releases on the
repo.
in your path, and logged into an account that can push to the repo.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script no longer uses gh, but dev-bin/release.sh:15-25 still requires gh auth status and gh repo view, and this line still makes gh a prerequisite. A releaser with working git push credentials but no gh login cannot release. Also, this line says the gh login must be able to push to the repo, but the push uses git credentials, not gh. The git ls-remote origin check in the script already verifies access.

Suggestion: remove the gh checks from the script and the gh prerequisite from this file.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that gh authentication does not prove Git push access. git ls-remote only proves read access, either. I retained the existing preflight checks to match mmdbverify and the other CLI release scripts. Removing the gh prerequisite and correcting the credential guidance would be a useful shared cleanup, but I have left it outside this release-flow fix.


🤖 Comment by Codex on behalf of Greg.

- Your environment also must have `bash`, `git` and `sed` available.

# Releasing
Expand All @@ -18,9 +17,13 @@
- Run `dev-bin/release.sh`.
- You might need to initialize/update submodules to successfully run tests,
eg. `git submodule update --init --recursive`.
- The script pushes the branch. Then it pushes an annotated tag, for example
`v2.0.1`. The tag message contains the release notes from `CHANGELOG.md`.
- The tag push starts the Release workflow. The authorized releasers receive an
email to review the pending deployment. If you are an authorized releaser,
approve the deployment. If you are not, wait for an authorized releaser to do
so.
- After the approval, GoReleaser creates the GitHub release with the notes from
the tag. It uploads the binaries and packages, then publishes the release.
- Verify the release on the GitHub Releases page.
- If everything goes well, the authorized releasers will receive an email to
review the pending deployment. If you are an authorized releaser, you will
need to approve the release deployment run. If you are not, you will have to
wait for an authorized releaser to do so.
- Make a PR and get it merged.
13 changes: 12 additions & 1 deletion dev-bin/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ date="${BASH_REMATCH[3]}"
notes="$(echo "${BASH_REMATCH[4]}" | sed -n -E '/^## [0-9]+\.[0-9]+\.[0-9]+(-[^ ]+)?/,$!p')"
tag="v$version"

if ! grep -q '[^[:space:]]' <<<"$notes"; then
echo "Release notes for $tag are empty. Add notes to CHANGELOG.md." >&2
exit 1
fi

if [[ "$date" != "$(date +"%Y-%m-%d")" ]]; then
echo "$date is not today!"
exit 1
Expand All @@ -94,4 +99,10 @@ fi

git push

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script pushes the branch before it creates the tag, and it does not handle a tag that already exists. A failed run can leave partial state, and then the next run fails.

Example: the first run creates v2.0.1 locally, and then git push origin v2.0.1 fails because of a network error or a ruleset. On the second run, the branch push succeeds again, and then git tag -a fails with "tag already exists". The releaser must delete the local tag first.

Suggestion: create the tag before any push. Or push both in one command: git push --atomic origin HEAD "refs/tags/$tag".

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retry limitation is real. I kept the branch-then-tag sequence used by mmdbverify and mmdbconvert for this PR. Creating the tag earlier or using an atomic push would still leave a local tag after a failed push, so retry handling needs a separate decision about existing local and remote tags. This change does not make retries automatic.


🤖 Comment by Codex on behalf of Greg.


gh release create --target "$(git branch --show-current)" -t "$version" -n "$notes" "$tag"
# The tag message is the version, a blank line, and the release notes.
# "whitespace" keeps Markdown headings and adds a newline before a signature.
git tag -a --cleanup=whitespace -m "$version

$notes" "$tag"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script does not make sure that $notes has content before it pushes the tag. The workflow rejects a tag without notes (.github/workflows/release.yml:67), but only after the tag is public. For example, if the CHANGELOG.md section for the version is empty, the script pushes the tag, and then the workflow fails with "has no release notes". The releaser must then delete the remote tag by hand and run the script again.

Suggestion: add the same non-whitespace check here, before any git push.

🤖 Comment by Claude Opus 5.5.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4ab751e. The script now rejects notes without a non-whitespace character before either push. I tested empty and whitespace-only changelog sections against a local bare remote. Both failed before changing the remote.


🤖 Comment by Codex on behalf of Greg.


git push origin "refs/tags/$tag"
Loading