diff --git a/AGENTS.md b/AGENTS.md index 8e2ba42..1b58a77 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,6 +7,8 @@ ## Git And Tests +- All changes must land through pull requests; do not push changes directly to + the default branch. - Product code that invokes Git should stay behind `internal/gitexec`. - Tests must not depend on the user's global Git identity, real Braid cache, or network remotes. diff --git a/internal/command/add_test.go b/internal/command/add_test.go index ec07196..59c30b9 100644 --- a/internal/command/add_test.go +++ b/internal/command/add_test.go @@ -432,7 +432,7 @@ func TestAddCommandBlocksUnresolvedGitOperationBeforeScopedStatus(t *testing.T) assertContains(t, stderr, "unresolved git operation state is present: MERGE_HEAD") } -func TestAddCommandNoCacheTags(t *testing.T) { +func TestAddCommandTags(t *testing.T) { tests := []struct { name string tag string @@ -463,23 +463,47 @@ func TestAddCommandNoCacheTags(t *testing.T) { } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - upstream := testutil.InitRepo(t) - revision := test.make(t, upstream, test.tag) - repo := initDownstream(t) - localPath := "vendor/" + test.name - runCommandOK(t, repo, []string{"--no-cache", "add", upstream, localPath, "--tag", test.tag}) + for _, mode := range []string{"no-cache", "global-cache"} { + t.Run(test.name+"/"+mode, func(t *testing.T) { + upstream := testutil.InitRepo(t) + revision := test.make(t, upstream, test.tag) + repo := initDownstream(t) + localPath := "vendor/" + test.name + args := []string{"--no-cache", "add", upstream, localPath, "--tag", test.tag} + if mode == "global-cache" { + args = []string{"--global-cache-dir", t.TempDir(), "add", upstream, localPath, "--tag", test.tag} + } + runCommandOK(t, repo, args) - assertFile(t, repo, localPath+"/README.md", test.name+"\n") - m := loadMirror(t, repo, localPath) - if m.Tag != test.tag || m.Branch != "" || m.Revision != revision { - t.Fatalf("mirror = %#v, want tag %q at %s", m, test.tag, revision) - } - assertClean(t, repo) - }) + assertFile(t, repo, localPath+"/README.md", test.name+"\n") + m := loadMirror(t, repo, localPath) + if m.Tag != test.tag || m.Branch != "" || m.Revision != revision { + t.Fatalf("mirror = %#v, want tag %q at %s", m, test.tag, revision) + } + git := gitexec.New(repo, false, nil) + assertRefMissing(t, context.Background(), git, "refs/tags/"+test.tag) + assertRefMissing(t, context.Background(), git, m.LocalRef()) + assertClean(t, repo) + }) + } } } +func TestAddTagMirrorPreservesSameNamedDownstreamTag(t *testing.T) { + upstream := testutil.InitRepo(t) + testutil.WriteFile(t, upstream, "README.md", "upstream\n") + testutil.CommitAll(t, upstream, "upstream") + testutil.Git(t, upstream, "tag", "v1") + + repo := initDownstream(t) + testutil.Git(t, repo, "tag", "-a", "v1", "-m", "downstream tag") + downstreamTag := strings.TrimSpace(testutil.Git(t, repo, "rev-parse", "refs/tags/v1").Stdout) + + runCommandOK(t, repo, []string{"--no-cache", "add", upstream, "vendor/tagged", "--tag", "v1"}) + + assertRefObjectID(t, context.Background(), gitexec.New(repo, false, nil), "refs/tags/v1", downstreamTag) +} + func TestAddCommandMissingUpstreamPathCleansUpTemporaryRemote(t *testing.T) { upstream := testutil.InitRepo(t) testutil.WriteFile(t, upstream, "README.md", "hello\n") diff --git a/internal/command/cache.go b/internal/command/cache.go index 6916677..025ca55 100644 --- a/internal/command/cache.go +++ b/internal/command/cache.go @@ -242,7 +242,7 @@ func fetchMirror(ctx context.Context, git fetchGit, cache CacheConfig, m mirror. args = append(args, "+refs/heads/"+m.Branch+":refs/remotes/"+m.Remote()+"/"+m.Branch) } if m.Tag != "" { - args = append(args, "+refs/tags/"+m.Tag+":refs/tags/"+m.Tag) + args = append(args, "+refs/tags/"+m.Tag+":"+m.LocalRef()) } args = append(args, "+refs/braid/*:refs/braid/*") return git.Fetch(ctx, args...) @@ -251,7 +251,7 @@ func fetchMirror(ctx context.Context, git fetchGit, cache CacheConfig, m mirror. return err } if m.Tag != "" { - return git.Fetch(ctx, "-n", m.Remote(), "+refs/tags/"+m.Tag+":refs/tags/"+m.Tag) + return git.Fetch(ctx, "-n", m.Remote(), "+refs/tags/"+m.Tag+":"+m.LocalRef()) } return nil }, diff --git a/internal/command/cache_test.go b/internal/command/cache_test.go index f21d32c..d881e1a 100644 --- a/internal/command/cache_test.go +++ b/internal/command/cache_test.go @@ -296,6 +296,31 @@ func TestFetchMirrorFromRepositoryLocalCacheFetchesBraidRefs(t *testing.T) { } } +func TestFetchTagMirrorFromRepositoryLocalCacheUsesRemoteTrackingRef(t *testing.T) { + ctx := context.Background() + upstream := testutil.InitRepo(t) + testutil.WriteFile(t, upstream, "README.md", "base\n") + revision := testutil.CommitAll(t, upstream, "base") + testutil.Git(t, upstream, "tag", "v1") + repo := testutil.InitRepo(t) + cache := repositoryLocalCacheForTest(t, ctx, repo) + m := mirror.Mirror{Path: "vendor/basic", URL: upstream, Tag: "v1", Revision: revision} + + if err := (MirrorObjectCache{Config: cache}).Hydrate(ctx, m); err != nil { + t.Fatalf("Hydrate returned error: %v", err) + } + git := gitexec.New(repo, false, nil) + if err := setupOne(ctx, git, m, true, cache); err != nil { + t.Fatalf("setupOne returned error: %v", err) + } + if err := fetchMirror(ctx, git, cache, m, progressReporter{}); err != nil { + t.Fatalf("fetchMirror returned error: %v", err) + } + + assertRefCommit(t, ctx, git, m.LocalRef(), revision) + assertRefMissing(t, ctx, git, "refs/tags/v1") +} + func TestAcquireCacheLockTimesOut(t *testing.T) { lockPath := filepath.Join(t.TempDir(), "mirror.git.lock") if err := os.Mkdir(lockPath, 0o700); err != nil { @@ -325,17 +350,6 @@ func repositoryLocalCacheForTest(t *testing.T, ctx context.Context, repo string) return cache } -func assertRefCommit(t *testing.T, ctx context.Context, git gitexec.Git, ref, want string) { - t.Helper() - got, err := git.RevParse(ctx, ref+"^{commit}") - if err != nil { - t.Fatalf("rev-parse %s: %v", ref, err) - } - if got != want { - t.Fatalf("%s = %s, want %s", ref, got, want) - } -} - func assertPathIsDir(t *testing.T, path string) { t.Helper() info, err := os.Stat(path) diff --git a/internal/command/sync_test.go b/internal/command/sync_test.go index 28e872d..6a61773 100644 --- a/internal/command/sync_test.go +++ b/internal/command/sync_test.go @@ -1106,6 +1106,28 @@ func TestSyncCommandKeepRetainsTemporaryRemote(t *testing.T) { assertGitRemote(t, repo, "main_braid_vendor_basic") } +func TestSyncCommandKeepRetainsDistinctTagTrackingRefs(t *testing.T) { + ctx := context.Background() + upstreamA := testutil.InitRepo(t) + testutil.WriteFile(t, upstreamA, "README.md", "a\n") + revisionA := testutil.CommitAll(t, upstreamA, "a") + testutil.Git(t, upstreamA, "tag", "v1") + upstreamB := testutil.InitRepo(t) + testutil.WriteFile(t, upstreamB, "README.md", "b\n") + revisionB := testutil.CommitAll(t, upstreamB, "b") + testutil.Git(t, upstreamB, "tag", "v1") + repo := initDownstream(t) + runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--tag", "v1"}) + runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b", "--tag", "v1"}) + + runCommandOK(t, repo, []string{"sync", "--pull-only", "--keep", "vendor/a", "vendor/b"}) + + git := gitexec.New(repo, false, nil) + assertRefCommit(t, ctx, git, "refs/remotes/v1_braid_vendor_a/tags/v1", revisionA) + assertRefCommit(t, ctx, git, "refs/remotes/v1_braid_vendor_b/tags/v1", revisionB) + assertRefMissing(t, ctx, git, "refs/tags/v1") +} + func assertNoGitRemote(t *testing.T, repo, remote string) { t.Helper() remotes := strings.Fields(testutil.Git(t, repo, "remote").Stdout) diff --git a/internal/command/test_support_test.go b/internal/command/test_support_test.go index 038e63a..ba2d948 100644 --- a/internal/command/test_support_test.go +++ b/internal/command/test_support_test.go @@ -24,6 +24,35 @@ func initDownstream(t *testing.T) string { return repo } +func assertRefCommit(t *testing.T, ctx context.Context, git gitexec.Git, ref, want string) { + t.Helper() + got, err := git.RevParse(ctx, ref+"^{commit}") + if err != nil { + t.Fatalf("rev-parse %s: %v", ref, err) + } + if got != want { + t.Fatalf("%s = %s, want %s", ref, got, want) + } +} + +func assertRefObjectID(t *testing.T, ctx context.Context, git gitexec.Git, ref, want string) { + t.Helper() + got, err := git.RevParse(ctx, ref) + if err != nil { + t.Fatalf("rev-parse %s: %v", ref, err) + } + if got != want { + t.Fatalf("%s = %s, want %s", ref, got, want) + } +} + +func assertRefMissing(t *testing.T, ctx context.Context, git gitexec.Git, ref string) { + t.Helper() + if result, err := git.RunOK(ctx, "rev-parse", "--verify", "--quiet", ref); err == nil { + t.Fatalf("ref %s exists unexpectedly at %s", ref, strings.TrimSpace(result.Stdout)) + } +} + func runCommandOK(t *testing.T, repo string, args []string) string { t.Helper() return runCommandOKInDir(t, repo, repo, args) diff --git a/internal/command/update_test.go b/internal/command/update_test.go index e206ef5..4068b6e 100644 --- a/internal/command/update_test.go +++ b/internal/command/update_test.go @@ -1042,6 +1042,8 @@ func TestUpdateCommandNoCacheTags(t *testing.T) { testutil.Git(t, upstream, "tag", test.tag) } repo := initDownstream(t) + testutil.Git(t, repo, "tag", "-a", test.tag, "-m", "downstream tag") + downstreamTag := strings.TrimSpace(testutil.Git(t, repo, "rev-parse", "refs/tags/"+test.tag).Stdout) localPath := "vendor/" + test.name runCommandOK(t, repo, []string{"--no-cache", "add", upstream, localPath, "--tag", test.tag}) @@ -1051,6 +1053,7 @@ func TestUpdateCommandNoCacheTags(t *testing.T) { if got := loadMirror(t, repo, localPath).Revision; got != revision { t.Fatalf("revision = %q, want %q", got, revision) } + assertRefObjectID(t, context.Background(), gitexec.New(repo, false, nil), "refs/tags/"+test.tag, downstreamTag) }) } } diff --git a/internal/mirror/mirror.go b/internal/mirror/mirror.go index 10c7027..771a9da 100644 --- a/internal/mirror/mirror.go +++ b/internal/mirror/mirror.go @@ -74,7 +74,7 @@ func (m Mirror) LocalRef() string { case m.Branch != "": return m.Remote() + "/" + m.Branch case m.Tag != "": - return "tags/" + m.Tag + return "refs/remotes/" + m.Remote() + "/tags/" + m.Tag default: return m.Revision } diff --git a/internal/mirror/mirror_test.go b/internal/mirror/mirror_test.go index 6fc0c7d..06e3bc3 100644 --- a/internal/mirror/mirror_test.go +++ b/internal/mirror/mirror_test.go @@ -64,7 +64,7 @@ func TestMirrorRefsAndRemote(t *testing.T) { if got, want := tag.Remote(), "v1_braid_mytool"; got != want { t.Fatalf("Remote = %q, want %q", got, want) } - if got, want := tag.LocalRef(), "tags/v1"; got != want { + if got, want := tag.LocalRef(), "refs/remotes/v1_braid_mytool/tags/v1"; got != want { t.Fatalf("LocalRef = %q, want %q", got, want) } if got, err := tag.RemoteRef(); err != nil || got != "+refs/tags/v1" {