Skip to content

Commit cd5442e

Browse files
pjbgfpjbgf
committed
gogit: add index/tree plumbing and graduate t1600 + t1601
Lands the surface upstream t/t1600-index.sh and t/t1601-index-bogus.sh exercise and graduates t1601 (4/4 cases) plus t1600 cases 1-5 + 7 (6 of 7 — case 6 uses git submodule add which is out of scope, skipped via the new tests.txt per-test selector). New gogit subcommands: - mktree — reads ls-tree text format from stdin, writes a tree object. Accepts null-hash entries (t1601's bogus-tree premise) by bypassing Tree.Validate via a raw encoder. - read-tree <tree> — populates the index from a tree. Refuses null-sha entries unless GIT_ALLOW_NULL_SHA1=1. Custom tree resolver because go-git's ResolveRevision doesn't handle bare tree SHAs. - write-tree — reads the current index, groups entries by directory, recursively builds tree objects with upstream's base_name_compare ordering (directory entries sort with implicit trailing /). Refuses null-hash entries. - update-index --show-index-version — prints the on-disk index format version. Other update-index modes are out of scope for v1. Existing surface extensions: - gogit add — picks the index format version from GIT_INDEX_VERSION env > index.version config > feature.manyFiles=true > default v2, via a new pickIndexVersion helper. Bogus or out-of-range values emit upstream-compatible warnings to stderr (matching wording is significant because t1600 normalises only the trailing digit before comparing). Version 3 is silently demoted to v2, mirroring upstream's "explicit request for the default" semantic that t1600 case 7's precedence matrix relies on. - gogit config gains --add (accepted as a plain set in v1). Conformance harness: - conformance/tests.txt entries may now carry an optional space- separated selector (e.g. t1600-index.sh 1-5,7). The harness forwards it to upstream's --run= so a single test can graduate with a subset of cases (used here to skip t1600 case 6). Logic that should live in go-git but does not yet is extracted to internal/plumbing/object/mktree.go so it can be upstreamed: - ParseMktreeInput — parses the ls-tree text format used as input by git mktree. Accepts null hashes; downstream validation is the consumer's responsibility. - WriteTreeRaw — serialises tree entries into an EncodedObject without running Tree.Validate, so null-hash entries round-trip cleanly. Both new exports have tests under the external _test package form. After this commit make conformance runs 5 tests and 42 cases pass. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
1 parent cb2d736 commit cd5442e

18 files changed

Lines changed: 1218 additions & 7 deletions

‎cmd/gogit/add.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package main
22

33
import (
44
"fmt"
5+
"os"
6+
"path/filepath"
57

68
"github.com/go-git/go-git/v6"
79
"github.com/spf13/cobra"
@@ -23,6 +25,13 @@ var addCmd = &cobra.Command{
2325

2426
defer r.Close()
2527

28+
gitDir := repoGitDir(r)
29+
_, statErr := os.Stat(filepath.Join(gitDir, "index"))
30+
hadExistingIndex := statErr == nil
31+
32+
repoCfg, _ := r.Config()
33+
version := pickIndexVersion(repoCfg, os.LookupEnv, hadExistingIndex, cmd.ErrOrStderr())
34+
2635
w, err := r.Worktree()
2736
if err != nil {
2837
return fmt.Errorf("failed to open worktree: %w", err)
@@ -34,6 +43,19 @@ var addCmd = &cobra.Command{
3443
}
3544
}
3645

46+
idx, err := r.Storer.Index()
47+
if err != nil {
48+
return err
49+
}
50+
51+
if idx.Version != version {
52+
idx.Version = version
53+
54+
if err := r.Storer.SetIndex(idx); err != nil {
55+
return err
56+
}
57+
}
58+
3759
return nil
3860
},
3961
DisableFlagsInUseLine: true,

‎cmd/gogit/add_test.go‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"os"
55
"path/filepath"
6+
"strings"
67
"testing"
78
)
89

@@ -47,3 +48,83 @@ func TestAddMultiplePaths(t *testing.T) {
4748
t.Fatalf("add: %v", err)
4849
}
4950
}
51+
52+
func TestAddWarnsBogusEnvVersion(t *testing.T) {
53+
t.Parallel()
54+
repo := t.TempDir()
55+
56+
if _, _, err := runGogit(t, repo, "init"); err != nil {
57+
t.Fatalf("init: %v", err)
58+
}
59+
60+
if err := os.WriteFile(filepath.Join(repo, "f"), []byte("x\n"), 0o644); err != nil {
61+
t.Fatal(err)
62+
}
63+
64+
env := append(os.Environ(), "GIT_INDEX_VERSION=2bogus")
65+
66+
_, stderr, err := runGogitEnv(t, repo, env, "add", "f")
67+
if err != nil {
68+
t.Fatalf("add: %v\nstderr: %s", err, stderr)
69+
}
70+
71+
wantPrefix := "warning: GIT_INDEX_VERSION set, but the value is invalid.\nUsing version 2\n"
72+
if !strings.HasPrefix(stderr, wantPrefix) {
73+
t.Fatalf("stderr = %q; want prefix %q", stderr, wantPrefix)
74+
}
75+
}
76+
77+
func TestAddNoWarnWithExistingIndex(t *testing.T) {
78+
t.Parallel()
79+
repo := t.TempDir()
80+
81+
if _, _, err := runGogit(t, repo, "init"); err != nil {
82+
t.Fatalf("init: %v", err)
83+
}
84+
85+
if err := os.WriteFile(filepath.Join(repo, "f"), []byte("x\n"), 0o644); err != nil {
86+
t.Fatal(err)
87+
}
88+
89+
if _, _, err := runGogit(t, repo, "add", "f"); err != nil {
90+
t.Fatalf("first add: %v", err)
91+
}
92+
93+
env := append(os.Environ(), "GIT_INDEX_VERSION=1")
94+
95+
_, stderr, err := runGogitEnv(t, repo, env, "add", "f")
96+
if err != nil {
97+
t.Fatalf("second add: %v", err)
98+
}
99+
100+
if stderr != "" {
101+
t.Fatalf("expected empty stderr with existing index, got %q", stderr)
102+
}
103+
}
104+
105+
func TestAddWarnsBogusConfigVersion(t *testing.T) {
106+
t.Parallel()
107+
repo := t.TempDir()
108+
109+
if _, _, err := runGogit(t, repo, "init"); err != nil {
110+
t.Fatalf("init: %v", err)
111+
}
112+
113+
if err := os.WriteFile(filepath.Join(repo, "f"), []byte("x\n"), 0o644); err != nil {
114+
t.Fatal(err)
115+
}
116+
117+
if _, _, err := runGogit(t, repo, "config", "index.version", "5"); err != nil {
118+
t.Fatalf("config: %v", err)
119+
}
120+
121+
_, stderr, err := runGogit(t, repo, "add", "f")
122+
if err != nil {
123+
t.Fatalf("add: %v", err)
124+
}
125+
126+
wantPrefix := "warning: index.version set, but the value is invalid.\nUsing version 2\n"
127+
if !strings.HasPrefix(stderr, wantPrefix) {
128+
t.Fatalf("stderr = %q; want prefix %q", stderr, wantPrefix)
129+
}
130+
}

‎cmd/gogit/config-cmd.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,15 @@ import (
1111
"github.com/spf13/cobra"
1212
)
1313

14-
var configUnsetAll bool
14+
var (
15+
configUnsetAll bool
16+
configAdd bool
17+
)
1518

1619
func init() {
1720
configCmd.Flags().BoolVar(&configUnsetAll, "unset-all", false, "Remove all occurrences of the key")
21+
configCmd.Flags().BoolVar(&configAdd, "add", false,
22+
"Add a new value without altering existing ones (treated as a plain set for v1)")
1823
rootCmd.AddCommand(configCmd)
1924
}
2025

‎cmd/gogit/indexversion.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"io"
6+
"strconv"
7+
8+
"github.com/go-git/go-git/v6/config"
9+
)
10+
11+
const (
12+
defaultIndexVersion uint32 = 2
13+
manyFilesIndexVersion uint32 = 4
14+
)
15+
16+
// envLookup mirrors os.LookupEnv. Injected so tests can run without touching
17+
// the process environment.
18+
type envLookup func(string) (string, bool)
19+
20+
// pickIndexVersion returns the index format version to use when writing a
21+
// new index. Precedence (highest first):
22+
//
23+
// 1. GIT_INDEX_VERSION env var
24+
// 2. index.version config
25+
// 3. feature.manyFiles=true → version 4
26+
// 4. default version 2
27+
//
28+
// Bogus or out-of-range values at steps 1 and 2 fall through to the next
29+
// source. If hadExistingIndex is false, the fall-through emits an upstream-
30+
// compatible warning to stderr.
31+
//
32+
// Version 3 is parseable but mirrors upstream's behaviour: it is silently
33+
// demoted to the default. Upstream treats it as an "explicit request for the
34+
// default" so neither a warning nor further fall-through is appropriate.
35+
func pickIndexVersion(cfg *config.Config, env envLookup, hadExistingIndex bool, stderr io.Writer) uint32 {
36+
if v, ok := env("GIT_INDEX_VERSION"); ok {
37+
if parsed, valid := parseGitIndexVersion(v); valid {
38+
return demoteV3(parsed)
39+
}
40+
41+
if !hadExistingIndex {
42+
fmt.Fprintf(stderr,
43+
"warning: GIT_INDEX_VERSION set, but the value is invalid.\nUsing version %d\n",
44+
defaultIndexVersion)
45+
}
46+
}
47+
48+
if cfg != nil {
49+
if cv := cfg.Raw.Section("index").Option("version"); cv != "" {
50+
if parsed, valid := parseGitIndexVersion(cv); valid {
51+
return demoteV3(parsed)
52+
}
53+
54+
if !hadExistingIndex {
55+
fmt.Fprintf(stderr,
56+
"warning: index.version set, but the value is invalid.\nUsing version %d\n",
57+
defaultIndexVersion)
58+
}
59+
}
60+
61+
if mf := cfg.Raw.Section("feature").Option("manyFiles"); mf == "true" {
62+
return manyFilesIndexVersion
63+
}
64+
}
65+
66+
return defaultIndexVersion
67+
}
68+
69+
func parseGitIndexVersion(s string) (uint32, bool) {
70+
v, err := strconv.ParseUint(s, 10, 32)
71+
if err != nil {
72+
return 0, false
73+
}
74+
75+
if v < 2 || v > 4 {
76+
return 0, false
77+
}
78+
79+
return uint32(v), true
80+
}
81+
82+
func demoteV3(v uint32) uint32 {
83+
if v == 3 {
84+
return defaultIndexVersion
85+
}
86+
87+
return v
88+
}

‎cmd/gogit/indexversion_test.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"strings"
6+
"testing"
7+
8+
gogitconfig "github.com/go-git/go-git/v6/config"
9+
)
10+
11+
func TestPickIndexVersion(t *testing.T) {
12+
t.Parallel()
13+
14+
tests := []struct {
15+
name string
16+
envValue string
17+
envSet bool
18+
configVersion string
19+
manyFiles string
20+
hadExistingIndex bool
21+
wantVersion uint32
22+
wantWarnPrefix string
23+
}{
24+
{name: "default", wantVersion: 2},
25+
{name: "env=3 silently demotes to 2", envSet: true, envValue: "3", wantVersion: 2},
26+
{name: "env=4", envSet: true, envValue: "4", wantVersion: 4},
27+
{
28+
name: "env=2bogus", envSet: true, envValue: "2bogus", wantVersion: 2,
29+
wantWarnPrefix: "warning: GIT_INDEX_VERSION set, but the value is invalid.\nUsing version 2\n",
30+
},
31+
{
32+
name: "env=1 out of bounds", envSet: true, envValue: "1", wantVersion: 2,
33+
wantWarnPrefix: "warning: GIT_INDEX_VERSION set, but the value is invalid.\nUsing version 2\n",
34+
},
35+
{name: "env bogus but existing index", envSet: true, envValue: "1", hadExistingIndex: true, wantVersion: 2},
36+
{name: "config=3 silently demotes to 2", configVersion: "3", wantVersion: 2},
37+
{
38+
name: "config=5 invalid", configVersion: "5", wantVersion: 2,
39+
wantWarnPrefix: "warning: index.version set, but the value is invalid.\nUsing version 2\n",
40+
},
41+
{name: "config invalid but existing index", configVersion: "5", hadExistingIndex: true, wantVersion: 2},
42+
{name: "manyFiles default 4", manyFiles: "true", wantVersion: 4},
43+
{name: "manyFiles overridden by config=2", configVersion: "2", manyFiles: "true", wantVersion: 2},
44+
{name: "env wins over config", envSet: true, envValue: "4", configVersion: "2", wantVersion: 4},
45+
{name: "env wins over manyFiles", envSet: true, envValue: "2", manyFiles: "true", wantVersion: 2},
46+
}
47+
48+
for _, tc := range tests {
49+
t.Run(tc.name, func(t *testing.T) {
50+
t.Parallel()
51+
52+
cfg := gogitconfig.NewConfig()
53+
if tc.configVersion != "" {
54+
cfg.Raw.Section("index").SetOption("version", tc.configVersion)
55+
}
56+
57+
if tc.manyFiles != "" {
58+
cfg.Raw.Section("feature").SetOption("manyFiles", tc.manyFiles)
59+
}
60+
61+
env := func(string) (string, bool) { return "", false }
62+
if tc.envSet {
63+
env = func(name string) (string, bool) {
64+
if name == "GIT_INDEX_VERSION" {
65+
return tc.envValue, true
66+
}
67+
68+
return "", false
69+
}
70+
}
71+
72+
var stderr bytes.Buffer
73+
74+
got := pickIndexVersion(cfg, env, tc.hadExistingIndex, &stderr)
75+
if got != tc.wantVersion {
76+
t.Fatalf("version = %d; want %d", got, tc.wantVersion)
77+
}
78+
79+
if tc.wantWarnPrefix == "" {
80+
if stderr.Len() != 0 {
81+
t.Fatalf("unexpected stderr: %q", stderr.String())
82+
}
83+
84+
return
85+
}
86+
87+
if !strings.HasPrefix(stderr.String(), tc.wantWarnPrefix) {
88+
t.Fatalf("stderr = %q; want prefix %q", stderr.String(), tc.wantWarnPrefix)
89+
}
90+
})
91+
}
92+
}

‎cmd/gogit/mktree.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
6+
internalobject "github.com/go-git/cli/internal/plumbing/object"
7+
"github.com/go-git/go-git/v6"
8+
"github.com/go-git/go-git/v6/plumbing"
9+
"github.com/spf13/cobra"
10+
)
11+
12+
func init() {
13+
rootCmd.AddCommand(mktreeCmd)
14+
}
15+
16+
var mktreeCmd = &cobra.Command{
17+
Use: "mktree",
18+
Short: "Build a tree-object from ls-tree formatted text",
19+
Args: cobra.NoArgs,
20+
RunE: func(cmd *cobra.Command, _ []string) error {
21+
r, err := git.PlainOpenWithOptions(".", &git.PlainOpenOptions{DetectDotGit: true})
22+
if err != nil {
23+
return fmt.Errorf("failed to open repository: %w", err)
24+
}
25+
26+
defer r.Close()
27+
28+
entries, err := internalobject.ParseMktreeInput(cmd.InOrStdin())
29+
if err != nil {
30+
return err
31+
}
32+
33+
obj := r.Storer.NewEncodedObject()
34+
obj.SetType(plumbing.TreeObject)
35+
36+
if err := internalobject.WriteTreeRaw(obj, entries); err != nil {
37+
return err
38+
}
39+
40+
hash, err := r.Storer.SetEncodedObject(obj)
41+
if err != nil {
42+
return err
43+
}
44+
45+
fmt.Fprintln(cmd.OutOrStdout(), hash)
46+
47+
return nil
48+
},
49+
DisableFlagsInUseLine: true,
50+
SilenceUsage: true,
51+
SilenceErrors: true,
52+
}

0 commit comments

Comments
 (0)