-
Notifications
You must be signed in to change notification settings - Fork 10
fix(release): refuse to tag unless HEAD is at the remote default tip #553
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # Releasing modules | ||
|
|
||
| This repository is a multi-module Go repository. Each module is released with | ||
| its own tag of the form `<module>/vX.Y.Z` (for example `store/v0.0.29`). | ||
|
|
||
| Releases are cut with the helper in [`x/release`](../x/release): | ||
|
|
||
| ```sh | ||
| # from the repository root | ||
| go run ./x/release bump <module> | ||
| ``` | ||
|
|
||
| By default this performs a patch release and propagates the bump to internal | ||
| downstream modules. Useful flags: | ||
|
|
||
| - `--release <patch|minor|major>` — choose the bump level for `<module>` | ||
| (downstreams are always propagated as a patch). | ||
| - `--dry` — log every step without changing anything. | ||
| - `--skip-git` — preview only the `go.mod` edits, skipping git operations. | ||
| - `--no-propagate` — release only `<module>`; do not bump downstreams. | ||
|
|
||
| ## How a release is applied | ||
|
|
||
| For the released module the tool: | ||
|
|
||
| 1. Creates an annotated tag on the current `HEAD` and pushes the tag. | ||
| 2. Bumps the module version in each downstream `go.mod`. | ||
| 3. Commits those edits as `chore: bump <module>/vX.Y.Z`. | ||
|
|
||
| > [!IMPORTANT] | ||
| > The tag is created on `HEAD`, and the `chore: bump` commit is committed | ||
| > **locally** — the tool pushes tags, not the branch. Land that commit on the | ||
| > default branch via a PR. | ||
|
|
||
| ## HEAD must match the remote default branch | ||
|
|
||
| Before creating any tag, the tool fetches and verifies that `HEAD` is exactly | ||
| the tip of the remote default branch (`origin/HEAD`, falling back to `main`). | ||
| If it is not, the release is refused: | ||
|
|
||
| ``` | ||
| refusing to tag: HEAD (<sha>) is not at the tip of origin/main (<sha>); ... | ||
| ``` | ||
|
|
||
| This guard exists because the tool tags whatever `HEAD` points at. If it is run | ||
| while `HEAD` sits on a local-only `chore: bump` commit (or any commit not yet on | ||
| the default branch), the tag lands on a commit that does not contain changes | ||
| merged afterwards. That is exactly how `store/v0.0.28` was pinned to an older | ||
| `chore: bump store/v0.0.27` commit and shipped without a keychain fix that had | ||
| already merged via PRs. | ||
|
|
||
| Always release from an up-to-date checkout of the default branch: | ||
|
|
||
| ```sh | ||
| git checkout main | ||
| git pull --ff-only | ||
| go run ./x/release bump <module> | ||
| ``` | ||
|
|
||
| The `--dry` and `--skip-git` modes skip this check, since they do not create | ||
| tags. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| // Copyright 2026 Docker, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| "context" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // Not parallel: the git helpers shell out in the process working directory, so | ||
| // the test switches cwd, which is process-global. | ||
| func Test_verifyReleaseRef(t *testing.T) { | ||
| repo := newGitRepoWithRemote(t) | ||
|
|
||
| t.Run("passes when HEAD is at remote default tip", func(t *testing.T) { | ||
| assert.NoError(t, verifyReleaseRef(repo.ctxAt(t))) | ||
| }) | ||
|
|
||
| t.Run("fails on a local-only commit", func(t *testing.T) { | ||
| repo.commit(t, "chore: bump local/v0.0.1") | ||
| err := verifyReleaseRef(repo.ctxAt(t)) | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), "refusing to tag") | ||
| }) | ||
|
|
||
| t.Run("passes again once the commit is pushed", func(t *testing.T) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Subtest 3 passes trivially when run in isolation — no coverage of the recovery path
Consider restructuring so subtest 3 explicitly creates the local-only commit itself before asserting failure, then pushes and asserts success: t.Run("passes again once the commit is pushed", func(t *testing.T) {
// put HEAD ahead of remote
repo.addCommit(t, "temp commit")
require.Error(t, verifyReleaseRef(ctx, repo.dir)) // must fail first
repo.run(t, "push", "origin", "HEAD:main")
require.NoError(t, verifyReleaseRef(ctx, repo.dir)) // then pass
}) |
||
| repo.run(t, "push", "origin", "HEAD:main") | ||
| assert.NoError(t, verifyReleaseRef(repo.ctxAt(t))) | ||
| }) | ||
| } | ||
|
|
||
| type gitRepo struct { | ||
| dir string | ||
| } | ||
|
|
||
| // newGitRepoWithRemote creates a working clone whose origin is a local bare | ||
| // repo, with origin/HEAD pointing at main, and HEAD at the remote tip. | ||
| func newGitRepoWithRemote(t *testing.T) *gitRepo { | ||
| t.Helper() | ||
| root := t.TempDir() | ||
| bare := filepath.Join(root, "origin.git") | ||
| work := filepath.Join(root, "work") | ||
|
|
||
| runGitIn(t, root, "init", "--bare", "--initial-branch=main", bare) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] The A portable alternative that works on all Git versions: // Instead of: runGitIn(t, root, "init", "--bare", "--initial-branch=main", bare)
runGitIn(t, root, "init", "--bare", bare)
runGitIn(t, bare, "symbolic-ref", "HEAD", "refs/heads/main")This also aligns with the |
||
|
|
||
| r := &gitRepo{dir: work} | ||
| runGitIn(t, root, "clone", bare, work) | ||
| r.config(t) | ||
| require.NoError(t, os.WriteFile(filepath.Join(work, "README.md"), []byte("seed\n"), 0o644)) | ||
| r.run(t, "add", "README.md") | ||
| r.run(t, "commit", "-m", "seed") | ||
| r.run(t, "push", "-u", "origin", "main") | ||
| // Make origin/HEAD resolvable so remoteDefaultBranch finds it. | ||
| r.run(t, "remote", "set-head", "origin", "main") | ||
| return r | ||
| } | ||
|
|
||
| func (r *gitRepo) config(t *testing.T) { | ||
| t.Helper() | ||
| r.run(t, "config", "user.email", "test@example.com") | ||
| r.run(t, "config", "user.name", "test") | ||
| r.run(t, "config", "commit.gpgsign", "false") | ||
| r.run(t, "config", "tag.gpgsign", "false") | ||
| } | ||
|
|
||
| func (r *gitRepo) commit(t *testing.T, msg string) { | ||
| t.Helper() | ||
| require.NoError(t, os.WriteFile(filepath.Join(r.dir, "file.txt"), []byte(msg), 0o644)) | ||
| r.run(t, "add", "file.txt") | ||
| r.run(t, "commit", "-m", msg) | ||
| } | ||
|
|
||
| // ctxAt returns a context after switching the process into the repo dir, so the | ||
| // git helpers (which shell out in the cwd) operate on this repo. | ||
| func (r *gitRepo) ctxAt(t *testing.T) context.Context { | ||
| t.Helper() | ||
| cwd, err := os.Getwd() | ||
| require.NoError(t, err) | ||
| require.NoError(t, os.Chdir(r.dir)) | ||
| t.Cleanup(func() { _ = os.Chdir(cwd) }) | ||
| return t.Context() | ||
| } | ||
|
|
||
| func (r *gitRepo) run(t *testing.T, args ...string) { | ||
| t.Helper() | ||
| runGitIn(t, r.dir, args...) | ||
| } | ||
|
|
||
| func runGitIn(t *testing.T, dir string, args ...string) { | ||
| t.Helper() | ||
| cmd := exec.Command("git", args...) | ||
| cmd.Dir = dir | ||
| out, err := cmd.CombinedOutput() | ||
| require.NoError(t, err, "git %v: %s", args, out) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MEDIUM] Silent fallback to
"main"produces confusing errors when the real default branch differsWhen
git symbolic-ref --short refs/remotes/origin/HEADfails (unconfiguredorigin/HEAD, which is common on repos cloned without--remoteor before runninggit remote set-head origin -a), the function silently returns"main". If the repository's actual default branch is"master","develop", or anything else, the subsequentgit fetch origin mainwill fail with a confusing exit-status error like'couldn't find remote ref main'— giving the operator no indication that the root cause is an unconfiguredorigin/HEAD.Consider logging a diagnostic message, e.g.:
or attempting
git remote show originas a fallback to detect the true default branch.