fix(ermitegit): keep a dangling ref out of ErrNotFound and trim names like git
- commit
- dedf508f11ea8c4e745bbbbc9fbf650af0fa9312
- author
- brice · · 2026-10-11 00:39 UTC
- parent
- c38360a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +50 -2
M internal/adapters/ermitegit/convert.go +13 -0
| @@ -53,6 +53,12 @@ func listRefs(r *git.Repo) ([]domain.Ref, error) { | |||
| 53 | 53 | } | |
| 54 | 54 | case errors.Is(err, git.ErrWrongType): | |
| 55 | 55 | // A tag of a tree or a blob has no commit. | |
| 56 | + | case errors.Is(err, git.ErrNotFound): | |
| 57 | + | // The ref exists and its object does not: the repository is | |
| 58 | + | // damaged, which is not a missing repository or revision, so | |
| 59 | + | // the error must not match domain.ErrNotFound (gitcli fails | |
| 60 | + | // the same way). %v hides git.ErrNotFound from mapErr. | |
| 61 | + | return nil, fmt.Errorf("ref %s points to a missing object: %v", ref.Name, err) | |
| 56 | 62 | default: | |
| 57 | 63 | return nil, fmt.Errorf("follow %s: %w", ref.Name, err) | |
| 58 | 64 | } | |
| @@ -388,6 +394,9 @@ func convertCommit(c *git.Commit) (domain.Commit, error) { | |||
| 388 | 394 | } | |
| 389 | 395 | end := [5]int{} | |
| 390 | 396 | for i, f := range [...][]byte{c.Author.Name, c.Author.Email, c.Committer.Name, c.Committer.Email, c.Message} { | |
| 397 | + | if i == 0 || i == 2 { | |
| 398 | + | f = trimNameRight(f) | |
| 399 | + | } | |
| 391 | 400 | buf = append(buf, f...) | |
| 392 | 401 | end[i] = len(buf) | |
| 393 | 402 | } | |
| @@ -419,6 +428,10 @@ func convertCommit(c *git.Commit) (domain.Commit, error) { | |||
| 419 | 428 | return out, nil | |
| 420 | 429 | } | |
| 421 | 430 | ||
| 431 | + | // trimNameRight drops the trailing whitespace of a name the way git does (the | |
| 432 | + | // library trims spaces only). | |
| 433 | + | func trimNameRight(b []byte) []byte { return bytes.TrimRight(b, " \t\r\n") } | |
| 434 | + | ||
| 422 | 435 | func isUTF8Label(b []byte) bool { | |
| 423 | 436 | return bytes.EqualFold(b, []byte("utf-8")) || bytes.EqualFold(b, []byte("utf8")) | |
| 424 | 437 | } | |
| @@ -53,6 +53,12 @@ func listRefs(r *git.Repo) ([]domain.Ref, error) { | |||||
| 53 | } | 53 | } | ||
| 54 | case errors.Is(err, git.ErrWrongType): | 54 | case errors.Is(err, git.ErrWrongType): | ||
| 55 | // A tag of a tree or a blob has no commit. | 55 | // A tag of a tree or a blob has no commit. | ||
| 56 | + | case errors.Is(err, git.ErrNotFound): | |||
| 57 | + | // The ref exists and its object does not: the repository is | |||
| 58 | + | // damaged, which is not a missing repository or revision, so | |||
| 59 | + | // the error must not match domain.ErrNotFound (gitcli fails | |||
| 60 | + | // the same way). %v hides git.ErrNotFound from mapErr. | |||
| 61 | + | return nil, fmt.Errorf("ref %s points to a missing object: %v", ref.Name, err) | |||
| 56 | default: | 62 | default: | ||
| 57 | return nil, fmt.Errorf("follow %s: %w", ref.Name, err) | 63 | return nil, fmt.Errorf("follow %s: %w", ref.Name, err) | ||
| 58 | } | 64 | } | ||
| @@ -388,6 +394,9 @@ func convertCommit(c *git.Commit) (domain.Commit, error) { | |||||
| 388 | } | 394 | } | ||
| 389 | end := [5]int{} | 395 | end := [5]int{} | ||
| 390 | for i, f := range [...][]byte{c.Author.Name, c.Author.Email, c.Committer.Name, c.Committer.Email, c.Message} { | 396 | for i, f := range [...][]byte{c.Author.Name, c.Author.Email, c.Committer.Name, c.Committer.Email, c.Message} { | ||
| 397 | + | if i == 0 || i == 2 { | |||
| 398 | + | f = trimNameRight(f) | |||
| 399 | + | } | |||
| 391 | buf = append(buf, f...) | 400 | buf = append(buf, f...) | ||
| 392 | end[i] = len(buf) | 401 | end[i] = len(buf) | ||
| 393 | } | 402 | } | ||
| @@ -419,6 +428,10 @@ func convertCommit(c *git.Commit) (domain.Commit, error) { | |||||
| 419 | return out, nil | 428 | return out, nil | ||
| 420 | } | 429 | } | ||
| 421 | 430 | ||||
| 431 | + | // trimNameRight drops the trailing whitespace of a name the way git does (the | |||
| 432 | + | // library trims spaces only). | |||
| 433 | + | func trimNameRight(b []byte) []byte { return bytes.TrimRight(b, " \t\r\n") } | |||
| 434 | + | ||||
| 422 | func isUTF8Label(b []byte) bool { | 435 | func isUTF8Label(b []byte) bool { | ||
| 423 | return bytes.EqualFold(b, []byte("utf-8")) || bytes.EqualFold(b, []byte("utf8")) | 436 | return bytes.EqualFold(b, []byte("utf-8")) || bytes.EqualFold(b, []byte("utf8")) | ||
| 424 | } | 437 | } | ||
M internal/readertest/compare.go +2 -2
| @@ -55,7 +55,7 @@ func errClass(err error) string { | |||
| 55 | 55 | case errors.Is(err, context.Canceled): | |
| 56 | 56 | return "canceled" | |
| 57 | 57 | } | |
| 58 | - | return "other: " + err.Error() | |
| 58 | + | return "other" // the messages of two readers differ | |
| 59 | 59 | } | |
| 60 | 60 | ||
| 61 | 61 | // Queries are the questions put to both readers about one repository. | |
| @@ -116,7 +116,7 @@ func Derive(t testing.TB, dir string, b Budget) Queries { | |||
| 116 | 116 | var commits []string | |
| 117 | 117 | { | |
| 118 | 118 | // An empty repository has no commit: rev-list --all prints nothing. | |
| 119 | - | cmd := exec.Command("git", "--git-dir="+dir, "rev-list", "--all") | |
| 119 | + | cmd := exec.Command("git", "--git-dir="+dir, "rev-list", "--all", "--missing=allow-any") | |
| 120 | 120 | cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + dir, "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null"} | |
| 121 | 121 | out, err := cmd.Output() | |
| 122 | 122 | require.NoError(t, err) | |
| @@ -55,7 +55,7 @@ func errClass(err error) string { | |||||
| 55 | case errors.Is(err, context.Canceled): | 55 | case errors.Is(err, context.Canceled): | ||
| 56 | return "canceled" | 56 | return "canceled" | ||
| 57 | } | 57 | } | ||
| 58 | - | return "other: " + err.Error() | 58 | + | return "other" // the messages of two readers differ |
| 59 | } | 59 | } | ||
| 60 | 60 | ||||
| 61 | // Queries are the questions put to both readers about one repository. | 61 | // Queries are the questions put to both readers about one repository. | ||
| @@ -116,7 +116,7 @@ func Derive(t testing.TB, dir string, b Budget) Queries { | |||||
| 116 | var commits []string | 116 | var commits []string | ||
| 117 | { | 117 | { | ||
| 118 | // An empty repository has no commit: rev-list --all prints nothing. | 118 | // An empty repository has no commit: rev-list --all prints nothing. | ||
| 119 | - | cmd := exec.Command("git", "--git-dir="+dir, "rev-list", "--all") | 119 | + | cmd := exec.Command("git", "--git-dir="+dir, "rev-list", "--all", "--missing=allow-any") |
| 120 | cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + dir, "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null"} | 120 | cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + dir, "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null"} | ||
| 121 | out, err := cmd.Output() | 121 | out, err := cmd.Output() | ||
| 122 | require.NoError(t, err) | 122 | require.NoError(t, err) | ||
M internal/readertest/fixture.go +12 -0
| @@ -70,6 +70,18 @@ func (f *Fixture) Git(args ...string) string { | |||
| 70 | 70 | return strings.TrimSpace(string(out)) | |
| 71 | 71 | } | |
| 72 | 72 | ||
| 73 | + | // WriteObject stores a raw object of the given type (commit, tag...) built by | |
| 74 | + | // hand, for what git itself refuses to write, and returns its id. | |
| 75 | + | func (f *Fixture) WriteObject(typ, content string) string { | |
| 76 | + | f.T.Helper() | |
| 77 | + | cmd := exec.Command("git", "hash-object", "-w", "-t", typ, "--stdin") | |
| 78 | + | cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + f.Work, "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null", "GIT_DIR=" + f.Bare} | |
| 79 | + | cmd.Stdin = strings.NewReader(content) | |
| 80 | + | out, err := cmd.CombinedOutput() | |
| 81 | + | require.NoError(f.T, err, "git hash-object: %s", out) | |
| 82 | + | return strings.TrimSpace(string(out)) | |
| 83 | + | } | |
| 84 | + | ||
| 73 | 85 | // Path returns the path of a file of the work tree. | |
| 74 | 86 | func (f *Fixture) Path(p string) string { return filepath.Join(f.Work, filepath.FromSlash(p)) } | |
| 75 | 87 | ||
| @@ -70,6 +70,18 @@ func (f *Fixture) Git(args ...string) string { | |||||
| 70 | return strings.TrimSpace(string(out)) | 70 | return strings.TrimSpace(string(out)) | ||
| 71 | } | 71 | } | ||
| 72 | 72 | ||||
| 73 | + | // WriteObject stores a raw object of the given type (commit, tag...) built by | |||
| 74 | + | // hand, for what git itself refuses to write, and returns its id. | |||
| 75 | + | func (f *Fixture) WriteObject(typ, content string) string { | |||
| 76 | + | f.T.Helper() | |||
| 77 | + | cmd := exec.Command("git", "hash-object", "-w", "-t", typ, "--stdin") | |||
| 78 | + | cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + f.Work, "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null", "GIT_DIR=" + f.Bare} | |||
| 79 | + | cmd.Stdin = strings.NewReader(content) | |||
| 80 | + | out, err := cmd.CombinedOutput() | |||
| 81 | + | require.NoError(f.T, err, "git hash-object: %s", out) | |||
| 82 | + | return strings.TrimSpace(string(out)) | |||
| 83 | + | } | |||
| 84 | + | ||||
| 73 | // Path returns the path of a file of the work tree. | 85 | // Path returns the path of a file of the work tree. | ||
| 74 | func (f *Fixture) Path(p string) string { return filepath.Join(f.Work, filepath.FromSlash(p)) } | 86 | func (f *Fixture) Path(p string) string { return filepath.Join(f.Work, filepath.FromSlash(p)) } | ||
| 75 | 87 | ||||
M internal/readertest/fixtures.go +23 -0
| @@ -4,10 +4,14 @@ package readertest | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "fmt" | |
| 7 | + | "os" | |
| 8 | + | "path/filepath" | |
| 7 | 9 | "strings" | |
| 8 | 10 | "testing" | |
| 9 | 11 | "time" | |
| 10 | 12 | ||
| 13 | + | "github.com/stretchr/testify/require" | |
| 14 | + | ||
| 11 | 15 | "bnema.dev/ermite/internal/domain" | |
| 12 | 16 | ) | |
| 13 | 17 | ||
| @@ -57,6 +61,7 @@ func Cases() []Case { | |||
| 57 | 61 | {"large blobs", buildLarge}, | |
| 58 | 62 | {"names and messages", buildNames}, | |
| 59 | 63 | {"refs", buildRefs}, | |
| 64 | + | {"dangling ref", buildDangling}, | |
| 60 | 65 | {"submodule", buildSubmodule}, | |
| 61 | 66 | {"wide directory", buildWide}, | |
| 62 | 67 | {"detached HEAD", func(t *testing.T, data string) *Fixture { | |
| @@ -194,6 +199,11 @@ func buildNames(t *testing.T, data string) *Fixture { | |||
| 194 | 199 | f.Git("-c", "user.name=Zoë <Ünï>", "commit", "--quiet", "--author=Zoë Ünï <zoe@example.org>", "-m", strings.Repeat("long ", 400)) | |
| 195 | 200 | f.Write("héllo wörld.txt", "x\n").Write("tab\tname.txt", "t\n").Write("-dash", "d\n") | |
| 196 | 201 | f.Commit("names") | |
| 202 | + | // Git trims the trailing whitespace of a name when it writes a commit but | |
| 203 | + | // shows one that has some as it is read: the readers must agree on it. | |
| 204 | + | head, tree := f.Git("rev-parse", "HEAD"), f.Git("rev-parse", "HEAD^{tree}") | |
| 205 | + | f.Git("update-ref", "refs/heads/main", f.WriteObject("commit", "tree "+tree+"\nparent "+head+"\n"+ | |
| 206 | + | "author Ada\t <a@b> 1767268860 +0000\ncommitter Grace \t\r <g@b> 1767268860 +0000\n\nraw names\n")) | |
| 197 | 207 | f.Maintain() | |
| 198 | 208 | return f | |
| 199 | 209 | } | |
| @@ -231,6 +241,19 @@ func buildRefs(t *testing.T, data string) *Fixture { | |||
| 231 | 241 | return f | |
| 232 | 242 | } | |
| 233 | 243 | ||
| 244 | + | // buildDangling has a loose branch and a loose tag whose objects do not exist, | |
| 245 | + | // next to sound refs. git lists them, and fails when it must follow them. | |
| 246 | + | func buildDangling(t *testing.T, data string) *Fixture { | |
| 247 | + | f := NewFixture(t, data, "dangling") | |
| 248 | + | f.Write("a.txt", "1\n") | |
| 249 | + | f.Commit("one") | |
| 250 | + | ghost := strings.Repeat("0123456789", 4) | |
| 251 | + | for _, ref := range []string{"refs/heads/ghost", "refs/tags/ghost"} { | |
| 252 | + | require.NoError(t, os.WriteFile(filepath.Join(f.Bare, filepath.FromSlash(ref)), []byte(ghost+"\n"), 0o644)) | |
| 253 | + | } | |
| 254 | + | return f | |
| 255 | + | } | |
| 256 | + | ||
| 234 | 257 | // buildSubmodule has a gitlink entry, a symlink and an executable. | |
| 235 | 258 | func buildSubmodule(t *testing.T, data string) *Fixture { | |
| 236 | 259 | f := NewFixture(t, data, "submod") | |
| @@ -4,10 +4,14 @@ package readertest | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | "os" | |||
| 8 | + | "path/filepath" | |||
| 7 | "strings" | 9 | "strings" | ||
| 8 | "testing" | 10 | "testing" | ||
| 9 | "time" | 11 | "time" | ||
| 10 | 12 | ||||
| 13 | + | "github.com/stretchr/testify/require" | |||
| 14 | + | ||||
| 11 | "bnema.dev/ermite/internal/domain" | 15 | "bnema.dev/ermite/internal/domain" | ||
| 12 | ) | 16 | ) | ||
| 13 | 17 | ||||
| @@ -57,6 +61,7 @@ func Cases() []Case { | |||||
| 57 | {"large blobs", buildLarge}, | 61 | {"large blobs", buildLarge}, | ||
| 58 | {"names and messages", buildNames}, | 62 | {"names and messages", buildNames}, | ||
| 59 | {"refs", buildRefs}, | 63 | {"refs", buildRefs}, | ||
| 64 | + | {"dangling ref", buildDangling}, | |||
| 60 | {"submodule", buildSubmodule}, | 65 | {"submodule", buildSubmodule}, | ||
| 61 | {"wide directory", buildWide}, | 66 | {"wide directory", buildWide}, | ||
| 62 | {"detached HEAD", func(t *testing.T, data string) *Fixture { | 67 | {"detached HEAD", func(t *testing.T, data string) *Fixture { | ||
| @@ -194,6 +199,11 @@ func buildNames(t *testing.T, data string) *Fixture { | |||||
| 194 | f.Git("-c", "user.name=Zoë <Ünï>", "commit", "--quiet", "--author=Zoë Ünï <zoe@example.org>", "-m", strings.Repeat("long ", 400)) | 199 | f.Git("-c", "user.name=Zoë <Ünï>", "commit", "--quiet", "--author=Zoë Ünï <zoe@example.org>", "-m", strings.Repeat("long ", 400)) | ||
| 195 | f.Write("héllo wörld.txt", "x\n").Write("tab\tname.txt", "t\n").Write("-dash", "d\n") | 200 | f.Write("héllo wörld.txt", "x\n").Write("tab\tname.txt", "t\n").Write("-dash", "d\n") | ||
| 196 | f.Commit("names") | 201 | f.Commit("names") | ||
| 202 | + | // Git trims the trailing whitespace of a name when it writes a commit but | |||
| 203 | + | // shows one that has some as it is read: the readers must agree on it. | |||
| 204 | + | head, tree := f.Git("rev-parse", "HEAD"), f.Git("rev-parse", "HEAD^{tree}") | |||
| 205 | + | f.Git("update-ref", "refs/heads/main", f.WriteObject("commit", "tree "+tree+"\nparent "+head+"\n"+ | |||
| 206 | + | "author Ada\t <a@b> 1767268860 +0000\ncommitter Grace \t\r <g@b> 1767268860 +0000\n\nraw names\n")) | |||
| 197 | f.Maintain() | 207 | f.Maintain() | ||
| 198 | return f | 208 | return f | ||
| 199 | } | 209 | } | ||
| @@ -231,6 +241,19 @@ func buildRefs(t *testing.T, data string) *Fixture { | |||||
| 231 | return f | 241 | return f | ||
| 232 | } | 242 | } | ||
| 233 | 243 | ||||
| 244 | + | // buildDangling has a loose branch and a loose tag whose objects do not exist, | |||
| 245 | + | // next to sound refs. git lists them, and fails when it must follow them. | |||
| 246 | + | func buildDangling(t *testing.T, data string) *Fixture { | |||
| 247 | + | f := NewFixture(t, data, "dangling") | |||
| 248 | + | f.Write("a.txt", "1\n") | |||
| 249 | + | f.Commit("one") | |||
| 250 | + | ghost := strings.Repeat("0123456789", 4) | |||
| 251 | + | for _, ref := range []string{"refs/heads/ghost", "refs/tags/ghost"} { | |||
| 252 | + | require.NoError(t, os.WriteFile(filepath.Join(f.Bare, filepath.FromSlash(ref)), []byte(ghost+"\n"), 0o644)) | |||
| 253 | + | } | |||
| 254 | + | return f | |||
| 255 | + | } | |||
| 256 | + | ||||
| 234 | // buildSubmodule has a gitlink entry, a symlink and an executable. | 257 | // buildSubmodule has a gitlink entry, a symlink and an executable. | ||
| 235 | func buildSubmodule(t *testing.T, data string) *Fixture { | 258 | func buildSubmodule(t *testing.T, data string) *Fixture { | ||
| 236 | f := NewFixture(t, data, "submod") | 259 | f := NewFixture(t, data, "submod") | ||