refactor(gitcli): resolve object ids before refs, read HEAD in one place, walk last commits in a loop

A full object id now resolves as an object id first; refs are tried only
when no such object exists, so a branch named like a commit cannot redirect
a link. readHead replaces three HEAD readers, refs/heads/ has one constant,
and fillLastCommits is a plain loop bounded by the exec semaphore. Trash
documents why the pool forgets the process after the rename. GetTree caps a
listing at 5,000 entries.

commit: d97ab5787d0803da7edaa14d4cf91f9781d45206
author: brice · 12d ago · 2026-10-10 11:14 UTC
parent: 147824d
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +122 -83

M internal/adapters/gitcli/read.go +45 -63


@@ -8,11 +8,12 @@ import (
8	8		"errors"
9	9		"fmt"
10	10		"io"
11		-	"os/exec"
	11	+	"io/fs"
	12	+	"os"
	13	+	"path/filepath"
12	14		"slices"
13	15		"strconv"
14	16		"strings"
15		-	"sync"
16	17		"time"
17	18
18	19		"bnema.dev/ermite/internal/domain"

@@ -26,9 +27,6 @@ const (
26	27		tagsRefPrefix = "refs/tags/"
27	28		// maxRefLine bounds a line of for-each-ref.
28	29		maxRefLine = 16 * 1024
29		-	// lastCommitWorkers is how many `git log -1` run at once for the last
30		-	// commits of a directory.
31		-	lastCommitWorkers = 8
32	30		)
33	31
34	32		// notFound reports whether err means a missing object.

@@ -50,7 +48,7 @@ func (g *Git) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.Ref,
50	48		var raws []raw
51	49		args := readArgs(dir, "for-each-ref",
52	50		"--format=%(refname)%00%(objectname)%00%(objecttype)%00%(objectname)%00%(objecttype)",
53		-	"refs/heads/", "refs/tags/")
	51	+	headsRefPrefix, tagsRefPrefix)
54	52		err = g.pipe(ctx, args, func(r *bufio.Reader) error {
55	53		lr := newLineReader(r, maxRefLine)
56	54		for {

@@ -126,7 +124,7 @@ func (g *Git) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.Ref,
126	124		}
127	125
128	126		head := domain.Ref{Name: "HEAD", Kind: domain.RefKindHead}
129		-	target, err := g.headTarget(ctx, dir)
	127	+	target, err := readHead(dir)
130	128		if err != nil {
131	129		return nil, fmt.Errorf("read HEAD of %q: %w", repo, err)
132	130		}

@@ -138,17 +136,23 @@ func (g *Git) ListRefs(ctx context.Context, repo domain.RepoName) ([]domain.Ref,
138	136		return append([]domain.Ref{head}, refs...), nil
139	137		}
140	138
141		-	// headTarget returns the ref HEAD points to, or "" if HEAD is detached.
142		-	func (g *Git) headTarget(ctx context.Context, dir string) (string, error) {
143		-	out, err := g.run(ctx, readArgs(dir, "symbolic-ref", "--quiet", "HEAD")...)
	139	+	// readHead returns the ref HEAD of the bare repository in dir points to, or
	140	+	// "" if HEAD is detached. It reads the HEAD file (the repositories use the
	141	+	// files ref backend), which is what `git symbolic-ref HEAD` would answer
	142	+	// without starting a process. A missing file is domain.ErrNotFound.
	143	+	func readHead(dir string) (string, error) {
	144	+	raw, err := os.ReadFile(filepath.Join(dir, "HEAD"))
144	145		if err != nil {
145		-	var exit *exec.ExitError
146		-	if errors.As(err, &exit) && exit.ExitCode() == 1 {
147		-	return "", nil
	146	+	if errors.Is(err, fs.ErrNotExist) {
	147	+	return "", domain.ErrNotFound
148	148		}
149		-	return "", err
	149	+	return "", fmt.Errorf("read HEAD: %w", err)
150	150		}
151		-	return strings.TrimSpace(out), nil
	151	+	ref, ok := strings.CutPrefix(strings.TrimSpace(string(raw)), "ref: ")
	152	+	if !ok {
	153	+	return "", nil // a commit id: detached
	154	+	}
	155	+	return ref, nil
152	156		}
153	157
154	158		// revCandidate is one way to read a revision as a commit.

@@ -157,27 +161,31 @@ type revCandidate struct {
157	161		ref string // full ref name it stands for, "" for ids and HEAD
158	162		}
159	163
160		-	// candidates lists, in order, how a revision is tried: the branch, then the
161		-	// tag, then (for "HEAD", full ref names, ids and abbreviations) as written.
162		-	// Other forms Git understands, such as "main~2" or "FETCH_HEAD", are not
163		-	// revisions of ermite.
	164	+	// candidates lists, in order, how a revision is tried. A full object id is an
	165	+	// object id first: a branch or a tag that happens to be named like a commit
	166	+	// must not redirect a link to that commit; refs are tried only when no such
	167	+	// object exists. Otherwise it is the branch, then the tag, then (for "HEAD",
	168	+	// full ref names and abbreviations) as written. Other forms Git understands,
	169	+	// such as "main~2" or "FETCH_HEAD", are not revisions of ermite.
164	170		func candidates(rev domain.Revision) []revCandidate {
165	171		s := rev.String()
166	172		switch {
167	173		case rev.IsHead() \|\| s == "HEAD":
168	174		return []revCandidate{{spec: "HEAD^{commit}"}}
169	175		}
170		-	out := []revCandidate{
	176	+	refs := []revCandidate{
171	177		{spec: headsRefPrefix + s + "^{commit}", ref: headsRefPrefix + s},
172	178		{spec: tagsRefPrefix + s + "^{commit}", ref: tagsRefPrefix + s},
173	179		}
174	180		switch {
	181	+	case rev.IsObjectID():
	182	+	return append([]revCandidate{{spec: s + "^{commit}"}}, refs...)
175	183		case strings.HasPrefix(s, "refs/"):
176		-	out = append(out, revCandidate{spec: s + "^{commit}", ref: s})
177		-	case rev.IsObjectID() \|\| rev.IsAbbrevID():
178		-	out = append(out, revCandidate{spec: s + "^{commit}"})
	184	+	return append(refs, revCandidate{spec: s + "^{commit}", ref: s})
	185	+	case rev.IsAbbrevID():
	186	+	return append(refs, revCandidate{spec: s + "^{commit}"})
179	187		}
180		-	return out
	188	+	return refs
181	189		}
182	190
183	191		// resolve resolves rev on an open cat-file connection.

@@ -357,6 +365,9 @@ func (g *Git) GetTree(ctx context.Context, repo domain.RepoName, rev domain.Revi
357	365		return strings.Compare(a.Name, b.Name)
358	366		})
359	367
	368	+	if len(tree.Entries) > domain.MaxTreeEntries {
	369	+	tree.Entries, tree.Truncated = tree.Entries[:domain.MaxTreeEntries], true
	370	+	}
360	371		if includeLast {
361	372		if err := g.fillLastCommits(ctx, dir, &tree); err != nil {
362	373		return domain.Tree{}, fmt.Errorf("last commits of %q in %q: %w", path, repo, err)

@@ -398,54 +409,25 @@ func parseTreeEntry(rec string, parent domain.TreePath) (domain.TreeEntry, error
398	409		}
399	410
400	411		// fillLastCommits sets the last commit of the first MaxLastCommitEntries
401		-	// entries with `git log -1 <commit> -- <path>`, a few at a time. Git answers
402		-	// each from the commit-graph and its changed-path filters when they exist.
403		-	// Entries beyond the bound are left without one and the tree says so.
	412	+	// entries with `git log -1 <commit> -- <path>`, one after the other. Git
	413	+	// answers each from the commit-graph and its changed-path filters when they
	414	+	// exist. Every command takes a slot of the exec semaphore, so a large
	415	+	// directory cannot use more than its share of the processes the server may
	416	+	// run. Entries beyond the bound are left without one and the tree says so.
404	417		func (g Git) fillLastCommits(ctx context.Context, dir string, tree domain.Tree) error {
405	418		n := len(tree.Entries)
406	419		if n > domain.MaxLastCommitEntries {
407	420		n = domain.MaxLastCommitEntries
408	421		tree.LastCommitsOmitted = true
409	422		}
410		-	ctx, cancel := context.WithCancel(ctx)
411		-	defer cancel()
412		-
413		-	var (
414		-	wg sync.WaitGroup
415		-	mu sync.Mutex
416		-	firstErr error
417		-	jobs = make(chan int)
418		-	)
419		-	for range min(lastCommitWorkers, n) {
420		-	wg.Go(func() {
421		-	for i := range jobs {
422		-	c, err := g.lastCommit(ctx, dir, tree.CommitID, tree.Entries[i].Path)
423		-	mu.Lock()
424		-	if err != nil && firstErr == nil {
425		-	firstErr = err
426		-	cancel()
427		-	}
428		-	if err == nil {
429		-	tree.Entries[i].LastCommit = &c
430		-	}
431		-	mu.Unlock()
432		-	}
433		-	})
434		-	}
435		-	feed:
436	423		for i := range n {
437		-	select {
438		-	case jobs <- i:
439		-	case <-ctx.Done():
440		-	break feed
	424	+	c, err := g.lastCommit(ctx, dir, tree.CommitID, tree.Entries[i].Path)
	425	+	if err != nil {
	426	+	return err
441	427		}
	428	+	tree.Entries[i].LastCommit = &c
442	429		}
443		-	close(jobs)
444		-	wg.Wait()
445		-	if firstErr != nil {
446		-	return firstErr
447		-	}
448		-	return ctx.Err()
	430	+	return nil
449	431		}
450	432
451	433		// lastCommit returns the newest commit reachable from head that changed path;

 	"errors"
 	"fmt"
 	"io"
-	"os/exec"
+	"io/fs"
+	"os"
+	"path/filepath"
 	"slices"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 	"bnema.dev/ermite/internal/domain"
 	tagsRefPrefix  = "refs/tags/"
 	// maxRefLine bounds a line of for-each-ref.
 	maxRefLine = 16 * 1024
-	// lastCommitWorkers is how many `git log -1` run at once for the last
-	// commits of a directory.
-	lastCommitWorkers = 8
 )
 // notFound reports whether err means a missing object.
 	var raws []raw
 	args := readArgs(dir, "for-each-ref",
 		"--format=%(refname)%00%(objectname)%00%(objecttype)%00%(*objectname)%00%(*objecttype)",
-		"refs/heads/", "refs/tags/")
+		headsRefPrefix, tagsRefPrefix)
 	err = g.pipe(ctx, args, func(r *bufio.Reader) error {
 		lr := newLineReader(r, maxRefLine)
 		for {
 	}
 	head := domain.Ref{Name: "HEAD", Kind: domain.RefKindHead}
-	target, err := g.headTarget(ctx, dir)
+	target, err := readHead(dir)
 	if err != nil {
 		return nil, fmt.Errorf("read HEAD of %q: %w", repo, err)
 	}
 	return append([]domain.Ref{head}, refs...), nil
 }
-// headTarget returns the ref HEAD points to, or "" if HEAD is detached.
+// readHead returns the ref HEAD of the bare repository in dir points to, or
-func (g *Git) headTarget(ctx context.Context, dir string) (string, error) {
+// "" if HEAD is detached. It reads the HEAD file (the repositories use the
-	out, err := g.run(ctx, readArgs(dir, "symbolic-ref", "--quiet", "HEAD")...)
+// files ref backend), which is what `git symbolic-ref HEAD` would answer
+// without starting a process. A missing file is domain.ErrNotFound.
+func readHead(dir string) (string, error) {
+	raw, err := os.ReadFile(filepath.Join(dir, "HEAD"))
 	if err != nil {
-		var exit *exec.ExitError
+		if errors.Is(err, fs.ErrNotExist) {
-		if errors.As(err, &exit) && exit.ExitCode() == 1 {
+			return "", domain.ErrNotFound
-			return "", nil
 		}
-		return "", err
+		return "", fmt.Errorf("read HEAD: %w", err)
 	}
-	return strings.TrimSpace(out), nil
+	ref, ok := strings.CutPrefix(strings.TrimSpace(string(raw)), "ref: ")
+	if !ok {
+		return "", nil // a commit id: detached
+	}
+	return ref, nil
 }
 // revCandidate is one way to read a revision as a commit.
 	ref  string // full ref name it stands for, "" for ids and HEAD
 }
-// candidates lists, in order, how a revision is tried: the branch, then the
+// candidates lists, in order, how a revision is tried. A full object id is an
-// tag, then (for "HEAD", full ref names, ids and abbreviations) as written.
+// object id first: a branch or a tag that happens to be named like a commit
-// Other forms Git understands, such as "main~2" or "FETCH_HEAD", are not
+// must not redirect a link to that commit; refs are tried only when no such
-// revisions of ermite.
+// object exists. Otherwise it is the branch, then the tag, then (for "HEAD",
+// full ref names and abbreviations) as written. Other forms Git understands,
+// such as "main~2" or "FETCH_HEAD", are not revisions of ermite.
 func candidates(rev domain.Revision) []revCandidate {
 	s := rev.String()
 	switch {
 	case rev.IsHead() || s == "HEAD":
 		return []revCandidate{{spec: "HEAD^{commit}"}}
 	}
-	out := []revCandidate{
+	refs := []revCandidate{
 		{spec: headsRefPrefix + s + "^{commit}", ref: headsRefPrefix + s},
 		{spec: tagsRefPrefix + s + "^{commit}", ref: tagsRefPrefix + s},
 	}
 	switch {
+	case rev.IsObjectID():
+		return append([]revCandidate{{spec: s + "^{commit}"}}, refs...)
 	case strings.HasPrefix(s, "refs/"):
-		out = append(out, revCandidate{spec: s + "^{commit}", ref: s})
+		return append(refs, revCandidate{spec: s + "^{commit}", ref: s})
-	case rev.IsObjectID() || rev.IsAbbrevID():
+	case rev.IsAbbrevID():
-		out = append(out, revCandidate{spec: s + "^{commit}"})
+		return append(refs, revCandidate{spec: s + "^{commit}"})
 	}
-	return out
+	return refs
 }
 // resolve resolves rev on an open cat-file connection.
 		return strings.Compare(a.Name, b.Name)
 	})
+	if len(tree.Entries) > domain.MaxTreeEntries {
+		tree.Entries, tree.Truncated = tree.Entries[:domain.MaxTreeEntries], true
+	}
 	if includeLast {
 		if err := g.fillLastCommits(ctx, dir, &tree); err != nil {
 			return domain.Tree{}, fmt.Errorf("last commits of %q in %q: %w", path, repo, err)
 }
 // fillLastCommits sets the last commit of the first MaxLastCommitEntries
-// entries with `git log -1 <commit> -- <path>`, a few at a time. Git answers
+// entries with `git log -1 <commit> -- <path>`, one after the other. Git
-// each from the commit-graph and its changed-path filters when they exist.
+// answers each from the commit-graph and its changed-path filters when they
-// Entries beyond the bound are left without one and the tree says so.
+// exist. Every command takes a slot of the exec semaphore, so a large
+// directory cannot use more than its share of the processes the server may
+// run. Entries beyond the bound are left without one and the tree says so.
 func (g *Git) fillLastCommits(ctx context.Context, dir string, tree *domain.Tree) error {
 	n := len(tree.Entries)
 	if n > domain.MaxLastCommitEntries {
 		n = domain.MaxLastCommitEntries
 		tree.LastCommitsOmitted = true
 	}
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
-	var (
-		wg       sync.WaitGroup
-		mu       sync.Mutex
-		firstErr error
-		jobs     = make(chan int)
-	)
-	for range min(lastCommitWorkers, n) {
-		wg.Go(func() {
-			for i := range jobs {
-				c, err := g.lastCommit(ctx, dir, tree.CommitID, tree.Entries[i].Path)
-				mu.Lock()
-				if err != nil && firstErr == nil {
-					firstErr = err
-					cancel()
-				}
-				if err == nil {
-					tree.Entries[i].LastCommit = &c
-				}
-				mu.Unlock()
-			}
-		})
-	}
-feed:
 	for i := range n {
-		select {
+		c, err := g.lastCommit(ctx, dir, tree.CommitID, tree.Entries[i].Path)
-		case jobs <- i:
+		if err != nil {
-		case <-ctx.Done():
+			return err
-			break feed
 		}
+		tree.Entries[i].LastCommit = &c
 	}
-	close(jobs)
+	return nil
-	wg.Wait()
-	if firstErr != nil {
-		return firstErr
-	}
-	return ctx.Err()
 }
 // lastCommit returns the newest commit reachable from head that changed path;

M internal/adapters/gitcli/read_test.go +61 -0


@@ -5,6 +5,7 @@ package gitcli_test
5	5		import (
6	6		"context"
7	7		"errors"
	8	+	"fmt"
8	9		"io"
9	10		"strings"
10	11		"testing"

@@ -654,3 +655,63 @@ func TestHostileRevisionAndPathStayLiteral(t *testing.T) {
654	655		var pe *domain.InvalidNameError
655	656		assert.False(t, errors.As(err, &pe))
656	657		}
	658	+
	659	+	// A full object id is an object id before it is a ref: a branch or a tag
	660	+	// named like a commit must not redirect a link to that commit.
	661	+	func TestResolveRevisionObjectIDBeatsRefs(t *testing.T) {
	662	+	ctx := context.Background()
	663	+	g, data := newReader(t, nil)
	664	+	h := newHistory(t, data)
	665	+
	666	+	// Git itself warns about these refs and prefers the object id.
	667	+	h.git("-c", "advice.objectNameWarning=false", "branch", h.c1, h.c3)
	668	+	h.git("-c", "advice.objectNameWarning=false", "tag", h.c2, h.c3)
	669	+
	670	+	got, err := g.ResolveRevision(ctx, h.name, rev(t, h.c1))
	671	+	require.NoError(t, err)
	672	+	assert.Equal(t, h.c1, got.CommitID.String(), "the commit, not the branch of the same name")
	673	+	assert.Empty(t, got.Ref)
	674	+
	675	+	got, err = g.ResolveRevision(ctx, h.name, rev(t, h.c2))
	676	+	require.NoError(t, err)
	677	+	assert.Equal(t, h.c2, got.CommitID.String(), "the commit, not the tag of the same name")
	678	+
	679	+	// An abbreviation is still a name first, as before.
	680	+	h.git("branch", "abc1234", h.c3)
	681	+	if !strings.HasPrefix(h.c1, "abc1234") {
	682	+	got, err = g.ResolveRevision(ctx, h.name, rev(t, "abc1234"))
	683	+	require.NoError(t, err)
	684	+	assert.Equal(t, h.c3, got.CommitID.String())
	685	+	assert.Equal(t, "refs/heads/abc1234", got.Ref)
	686	+	}
	687	+
	688	+	// The ref is used when no object has that id.
	689	+	ghost := strings.Repeat("ab", 20)
	690	+	h.git("-c", "advice.objectNameWarning=false", "branch", ghost, h.c3)
	691	+	got, err = g.ResolveRevision(ctx, h.name, rev(t, ghost))
	692	+	require.NoError(t, err)
	693	+	assert.Equal(t, h.c3, got.CommitID.String())
	694	+	assert.Equal(t, "refs/heads/"+ghost, got.Ref)
	695	+	}
	696	+
	697	+	func TestGetTreeIsBounded(t *testing.T) {
	698	+	ctx := context.Background()
	699	+	g, data := newReader(t, nil)
	700	+	f := newFixtureRepo(t, data, "wide")
	701	+	n := domain.MaxTreeEntries + 7
	702	+	for i := range n {
	703	+	f.write(fmt.Sprintf("d/f%05d", i), "x")
	704	+	}
	705	+	f.commit("wide")
	706	+
	707	+	tree, err := g.GetTree(ctx, f.name, domain.Revision{}, tpath(t, "d"), false)
	708	+	require.NoError(t, err)
	709	+	assert.True(t, tree.Truncated)
	710	+	require.Len(t, tree.Entries, domain.MaxTreeEntries)
	711	+	assert.Equal(t, "f00000", tree.Entries[0].Name, "the first entries in order")
	712	+	assert.Equal(t, fmt.Sprintf("f%05d", domain.MaxTreeEntries-1), tree.Entries[domain.MaxTreeEntries-1].Name)
	713	+
	714	+	small, err := g.GetTree(ctx, f.name, domain.Revision{}, domain.TreePath{}, false)
	715	+	require.NoError(t, err)
	716	+	assert.False(t, small.Truncated)
	717	+	}


@@ -5,6 +5,7 @@ package gitcli_test
5	import (	5		import (
6	"context"	6		"context"
7	"errors"	7		"errors"
		8	+	"fmt"
8	"io"	9		"io"
9	"strings"	10		"strings"
10	"testing"	11		"testing"

@@ -654,3 +655,63 @@ func TestHostileRevisionAndPathStayLiteral(t *testing.T) {
654	var pe *domain.InvalidNameError	655		var pe *domain.InvalidNameError
655	assert.False(t, errors.As(err, &pe))	656		assert.False(t, errors.As(err, &pe))
656	}	657		}
		658	+
		659	+	// A full object id is an object id before it is a ref: a branch or a tag
		660	+	// named like a commit must not redirect a link to that commit.
		661	+	func TestResolveRevisionObjectIDBeatsRefs(t *testing.T) {
		662	+	ctx := context.Background()
		663	+	g, data := newReader(t, nil)
		664	+	h := newHistory(t, data)
		665	+
		666	+	// Git itself warns about these refs and prefers the object id.
		667	+	h.git("-c", "advice.objectNameWarning=false", "branch", h.c1, h.c3)
		668	+	h.git("-c", "advice.objectNameWarning=false", "tag", h.c2, h.c3)
		669	+
		670	+	got, err := g.ResolveRevision(ctx, h.name, rev(t, h.c1))
		671	+	require.NoError(t, err)
		672	+	assert.Equal(t, h.c1, got.CommitID.String(), "the commit, not the branch of the same name")
		673	+	assert.Empty(t, got.Ref)
		674	+
		675	+	got, err = g.ResolveRevision(ctx, h.name, rev(t, h.c2))
		676	+	require.NoError(t, err)
		677	+	assert.Equal(t, h.c2, got.CommitID.String(), "the commit, not the tag of the same name")
		678	+
		679	+	// An abbreviation is still a name first, as before.
		680	+	h.git("branch", "abc1234", h.c3)
		681	+	if !strings.HasPrefix(h.c1, "abc1234") {
		682	+	got, err = g.ResolveRevision(ctx, h.name, rev(t, "abc1234"))
		683	+	require.NoError(t, err)
		684	+	assert.Equal(t, h.c3, got.CommitID.String())
		685	+	assert.Equal(t, "refs/heads/abc1234", got.Ref)
		686	+	}
		687	+
		688	+	// The ref is used when no object has that id.
		689	+	ghost := strings.Repeat("ab", 20)
		690	+	h.git("-c", "advice.objectNameWarning=false", "branch", ghost, h.c3)
		691	+	got, err = g.ResolveRevision(ctx, h.name, rev(t, ghost))
		692	+	require.NoError(t, err)
		693	+	assert.Equal(t, h.c3, got.CommitID.String())
		694	+	assert.Equal(t, "refs/heads/"+ghost, got.Ref)
		695	+	}
		696	+
		697	+	func TestGetTreeIsBounded(t *testing.T) {
		698	+	ctx := context.Background()
		699	+	g, data := newReader(t, nil)
		700	+	f := newFixtureRepo(t, data, "wide")
		701	+	n := domain.MaxTreeEntries + 7
		702	+	for i := range n {
		703	+	f.write(fmt.Sprintf("d/f%05d", i), "x")
		704	+	}
		705	+	f.commit("wide")
		706	+
		707	+	tree, err := g.GetTree(ctx, f.name, domain.Revision{}, tpath(t, "d"), false)
		708	+	require.NoError(t, err)
		709	+	assert.True(t, tree.Truncated)
		710	+	require.Len(t, tree.Entries, domain.MaxTreeEntries)
		711	+	assert.Equal(t, "f00000", tree.Entries[0].Name, "the first entries in order")
		712	+	assert.Equal(t, fmt.Sprintf("f%05d", domain.MaxTreeEntries-1), tree.Entries[domain.MaxTreeEntries-1].Name)
		713	+
		714	+	small, err := g.GetTree(ctx, f.name, domain.Revision{}, domain.TreePath{}, false)
		715	+	require.NoError(t, err)
		716	+	assert.False(t, small.Truncated)
		717	+	}

M internal/adapters/gitcli/repos.go +16 -20


@@ -166,7 +166,13 @@ func (g *Git) Trash(ctx context.Context, name domain.RepoName) error {
166	166		return fmt.Errorf("trash repository %q: %w", name, err)
167	167		}
168	168		// A cat-file process of the old directory must not serve a repository
169		-	// created later under the same name.
	169	+	// created later under the same name. This runs after the rename, not
	170	+	// before: a rename that fails leaves the repository where it was, with
	171	+	// its process still serving it, and a process dropped first would only
	172	+	// be started again by the next read. Reads between the rename and the
	173	+	// forget find no directory (repoDir fails first), and the pool is keyed
	174	+	// by directory, so the stale process cannot answer for a new repository
	175	+	// before it is dropped here.
170	176		g.cat.forget(src)
171	177		if err := syncDir(g.reposDir()); err != nil {
172	178		return fmt.Errorf("trash repository %q: %w", name, err)

@@ -232,25 +238,21 @@ func (g *Git) repoDir(name domain.RepoName) (string, error) {
232	238		return dir, nil
233	239		}
234	240
235		-	const headsPrefix = "refs/heads/"
236		-
237		-	// DefaultBranch reads HEAD with `git symbolic-ref` and checks the branch with
238		-	// `git for-each-ref`.
	241	+	// DefaultBranch reads HEAD and checks the branch with `git for-each-ref`.
239	242		func (g *Git) DefaultBranch(ctx context.Context, name domain.RepoName) (string, bool, error) {
240	243		dir, err := g.repoDir(name)
241	244		if err != nil {
242	245		return "", false, err
243	246		}
244		-	out, err := g.run(ctx, "-C", dir, "symbolic-ref", "--quiet", "HEAD")
	247	+	ref, err := readHead(dir)
245	248		if err != nil {
246	249		return "", false, fmt.Errorf("read HEAD of %q: %w", name, err)
247	250		}
248		-	ref := strings.TrimSpace(out)
249		-	branch, ok := strings.CutPrefix(ref, headsPrefix)
	251	+	branch, ok := strings.CutPrefix(ref, headsRefPrefix)
250	252		if !ok {
251	253		return "", false, fmt.Errorf("HEAD of %q points outside refs/heads: %q", name, ref)
252	254		}
253		-	out, err = g.run(ctx, "-C", dir, "for-each-ref", "--count=1", "--format=%(refname)", ref)
	255	+	out, err := g.run(ctx, "-C", dir, "for-each-ref", "--count=1", "--format=%(refname)", ref)
254	256		if err != nil {
255	257		return "", false, fmt.Errorf("look up %s in %q: %w", ref, name, err)
256	258		}

@@ -267,7 +269,7 @@ func (g *Git) HasBranch(ctx context.Context, name domain.RepoName, branch string
267	269		if err != nil {
268	270		return false, err
269	271		}
270		-	ref := headsPrefix + branch
	272	+	ref := headsRefPrefix + branch
271	273		if branch == "" \|\| strings.HasPrefix(branch, "-") {
272	274		return false, nil
273	275		}

@@ -286,23 +288,17 @@ func (g *Git) HasBranch(ctx context.Context, name domain.RepoName, branch string
286	288		}
287	289
288	290		// HeadBranch reads the HEAD file of the bare repository (files ref backend).
	291	+	// It is empty when HEAD is detached.
289	292		func (g *Git) HeadBranch(_ context.Context, name domain.RepoName) (string, error) {
290	293		dir, err := g.repoDir(name)
291	294		if err != nil {
292	295		return "", err
293	296		}
294		-	raw, err := os.ReadFile(filepath.Join(dir, "HEAD"))
	297	+	ref, err := readHead(dir)
295	298		if err != nil {
296		-	if errors.Is(err, fs.ErrNotExist) {
297		-	return "", domain.ErrNotFound
298		-	}
299	299		return "", fmt.Errorf("read HEAD of %q: %w", name, err)
300	300		}
301		-	ref, ok := strings.CutPrefix(strings.TrimSpace(string(raw)), "ref: ")
302		-	if !ok {
303		-	return "", nil // detached
304		-	}
305		-	return strings.TrimPrefix(ref, headsPrefix), nil
	301	+	return strings.TrimPrefix(ref, headsRefPrefix), nil
306	302		}
307	303
308	304		// SetDefaultBranch runs `git symbolic-ref HEAD refs/heads/<branch>`.

@@ -317,7 +313,7 @@ func (g *Git) SetDefaultBranch(ctx context.Context, name domain.RepoName, branch
317	313		if _, err := g.run(ctx, "check-ref-format", "--branch", branch); err != nil {
318	314		return fmt.Errorf("invalid branch name %q: %w", branch, err)
319	315		}
320		-	if _, err := g.run(ctx, "-C", dir, "symbolic-ref", "HEAD", headsPrefix+branch); err != nil {
	316	+	if _, err := g.run(ctx, "-C", dir, "symbolic-ref", "HEAD", headsRefPrefix+branch); err != nil {
321	317		return fmt.Errorf("set HEAD of %q: %w", name, err)
322	318		}
323	319		return nil

 			return fmt.Errorf("trash repository %q: %w", name, err)
 		}
 		// A cat-file process of the old directory must not serve a repository
-		// created later under the same name.
+		// created later under the same name. This runs after the rename, not
+		// before: a rename that fails leaves the repository where it was, with
+		// its process still serving it, and a process dropped first would only
+		// be started again by the next read. Reads between the rename and the
+		// forget find no directory (repoDir fails first), and the pool is keyed
+		// by directory, so the stale process cannot answer for a new repository
+		// before it is dropped here.
 		g.cat.forget(src)
 		if err := syncDir(g.reposDir()); err != nil {
 			return fmt.Errorf("trash repository %q: %w", name, err)
 	return dir, nil
 }
-const headsPrefix = "refs/heads/"
+// DefaultBranch reads HEAD and checks the branch with `git for-each-ref`.
-// DefaultBranch reads HEAD with `git symbolic-ref` and checks the branch with
-// `git for-each-ref`.
 func (g *Git) DefaultBranch(ctx context.Context, name domain.RepoName) (string, bool, error) {
 	dir, err := g.repoDir(name)
 	if err != nil {
 		return "", false, err
 	}
-	out, err := g.run(ctx, "-C", dir, "symbolic-ref", "--quiet", "HEAD")
+	ref, err := readHead(dir)
 	if err != nil {
 		return "", false, fmt.Errorf("read HEAD of %q: %w", name, err)
 	}
-	ref := strings.TrimSpace(out)
+	branch, ok := strings.CutPrefix(ref, headsRefPrefix)
-	branch, ok := strings.CutPrefix(ref, headsPrefix)
 	if !ok {
 		return "", false, fmt.Errorf("HEAD of %q points outside refs/heads: %q", name, ref)
 	}
-	out, err = g.run(ctx, "-C", dir, "for-each-ref", "--count=1", "--format=%(refname)", ref)
+	out, err := g.run(ctx, "-C", dir, "for-each-ref", "--count=1", "--format=%(refname)", ref)
 	if err != nil {
 		return "", false, fmt.Errorf("look up %s in %q: %w", ref, name, err)
 	}
 	if err != nil {
 		return false, err
 	}
-	ref := headsPrefix + branch
+	ref := headsRefPrefix + branch
 	if branch == "" || strings.HasPrefix(branch, "-") {
 		return false, nil
 	}
 }
 // HeadBranch reads the HEAD file of the bare repository (files ref backend).
+// It is empty when HEAD is detached.
 func (g *Git) HeadBranch(_ context.Context, name domain.RepoName) (string, error) {
 	dir, err := g.repoDir(name)
 	if err != nil {
 		return "", err
 	}
-	raw, err := os.ReadFile(filepath.Join(dir, "HEAD"))
+	ref, err := readHead(dir)
 	if err != nil {
-		if errors.Is(err, fs.ErrNotExist) {
-			return "", domain.ErrNotFound
-		}
 		return "", fmt.Errorf("read HEAD of %q: %w", name, err)
 	}
-	ref, ok := strings.CutPrefix(strings.TrimSpace(string(raw)), "ref: ")
+	return strings.TrimPrefix(ref, headsRefPrefix), nil
-	if !ok {
-		return "", nil // detached
-	}
-	return strings.TrimPrefix(ref, headsPrefix), nil
 }
 // SetDefaultBranch runs `git symbolic-ref HEAD refs/heads/<branch>`.
 	if _, err := g.run(ctx, "check-ref-format", "--branch", branch); err != nil {
 		return fmt.Errorf("invalid branch name %q: %w", branch, err)
 	}
-	if _, err := g.run(ctx, "-C", dir, "symbolic-ref", "HEAD", headsPrefix+branch); err != nil {
+	if _, err := g.run(ctx, "-C", dir, "symbolic-ref", "HEAD", headsRefPrefix+branch); err != nil {
 		return fmt.Errorf("set HEAD of %q: %w", name, err)
 	}
 	return nil