conformance: expand the upstream-test gate from t2008+t5308 to five tests / 42 cases#60
Merged
Conversation
Lands the pack-write half of upstream-Git conformance and graduates
t/t5325-reverse-index.sh (16 cases) into the gate alongside the
existing t2008 (9 cases) and t5308 (6 cases).
New subcommands:
- hash-object [-w] [-t <type>] [--stdin] [--literally]
- count-objects
- fsck [--full] (object structure + .rev file validation)
- show-index (idx reader)
- pack-objects <basename> with --all (full ref-walk including tag
targets), --index-version (1 or 2; hand-rolled v1 encoder since
go-git's idxfile.Encode is v2-only), --no-reuse-object (accept+no-op),
--rev-index
- repack [-a] [-d]
- rev-parse <ref>
- rev-list --objects --no-object-names --all
- tag <name> (lightweight)
- config <key> [<value>] (with --unset-all)
- diff --no-index --ignore-cr-at-eol (Git-for-Windows GIT_TEST_CMP shim)
index-pack extensions: -o, --fsck-objects, --index-version, --verify
(also verifies .rev when paired with --rev-index), --rev-index,
--no-rev-index, plus a positional-pack-file mode.
cat-file gains --batch-check=<format> with %(objectname),
%(objecttype), %(objectsize), %(objectsize:disk) tokens.
Top-level -c <key>=<value> config override.
--strict on index-pack now streams: bytes fan out through an
io.MultiWriter to a temp file and to a pipe consumed by a concurrent
duplicate-detection parser. Memory is bounded by the parser working
set rather than buffering the full pack.
Every git.PlainOpen[WithOptions] now has a paired defer r.Close().
TestMain captures m.Run()'s exit code and cleans up the build tempdir.
Logic that should live in go-git but doesn't is extracted to internal/
packages whose paths mirror the eventual upstream home:
- internal/plumbing/format/idxfile: EncodeV1 +
ErrOffsetTooLargeForV1 (go-git ships v2 only).
- internal/plumbing/format/revfile: ValidateHeader,
ValidateRowPositions, FsckMessage and matching sentinels - go-git's
revfile.Decode collapses magic/version/hash/row errors onto one
sentinel; the extension keeps them distinct.
- internal/plumbing/object: ParseGitDate, the "<unix-seconds> <HHMM>"
parser used by GIT_*_DATE handling.
Each internal package has tests using the external _test package form.
The harness's test-tool stub now handles sha1/sha256 [-b]
(lib-pack.sh trailer construction) and is rewritten every run so
script changes take effect without manual cache invalidation.
conformance/run.sh uses the bash-3.2-safe \${arr[@]+...} idiom so
macOS default shell does not bail under set -u with an empty
selector array.
Other curated tests (t5302, t5313) remain blocked on additional
setup-side subcommands (update-index, write-tree, commit-tree,
ls-tree, update-ref, and a test-tool genrandom stub); documented
inline in conformance/tests.txt.
Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Paulo Gomes <paulo@entire.io>
make conformance now defaults to a terse summary: harness headers, the per-script "# passed all N test(s)" / "# failed" lines, the TAP plan, and any "not ok" failures. Verbose mode keeps the full upstream trace and is opt-in via CONFORMANCE_VERBOSE=1. CI sets that env var so its captured logs and TAP artifacts remain inspectable. cmd/gogit/main.go: drop cmd.Usage() from rootCmd's bare-invocation RunE and set SilenceUsage/SilenceErrors. test-lib only needs the exit-1 behaviour to detect a working git binary; printing usage during its sanity check was leaking into summary-mode harness output. conformance/run.sh: comment block above the upstream-test fetch clarifies why those git clone/fetch/reset calls intentionally use the host's git binary (gogit is not yet built; the calls fetch test infrastructure, not the system under test). README.md: new sections on the CLI's purpose, the internal/ staging area for missing go-git features, and how make conformance is structured / how curated tests are selected. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
Lands the surface upstream t/t5303-pack-corruption-resilience.sh exercises and replaces the harness's inline shell test-tool stub with a proper Go binary. Result: 14/36 t5303 cases pass against gogit; the remaining 22 are blocked on a go-git pack-reader change (documented inline in conformance/tests.txt) so t5303 is not yet graduated. New gogit surface: - gogit prune-packed — deletes loose objects whose hash also lives in a pack-*.idx under .git/objects/pack/. - gogit cat-file <type> <oid> — typed-print form: validates the object's type matches and writes its content to stdout, silently exiting 1 on miss or type mismatch. New helper binary at cmd/gogit-test-tool/ (built into \$FAKE_BUILD_DIR/t/helper/test-tool by the harness on every run): - genrandom <seed> [<length>] — byte-equivalent to upstream's t/helper/test-genrandom.c: do-while seed fold (next = next*11 + c including the implicit NUL) and POSIX.1-2001 LCG output (next = next*1103515245 + 12345; byte = (next>>16) & 0xff). Golden test pins the first 8 bytes for seed "foo". - delta -p <base> <delta> <out> — applies a binary delta via go-git's packfile.PatchDelta. - sha1, sha256 (with optional -b for raw), date is64bit/time_t-is64bit, path-utils file-size, env-helper — Go ports of the previous shell stub subcommands. Harness changes: - conformance/run.sh now go-builds the helper binary into the expected path on every run, replacing the heredoc shell stub. An explicit rm -f before the build clears any leftover non-Go-built file from previous shell-stub runs (Go refuses to overwrite those). Makefile: - make build writes binaries to build/bin/ rather than build/ root. build/tools/ continues to host golangci-lint and other tools. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
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>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Expands the gogit conformance harness and CLI surface area to run and pass a larger curated subset of upstream Git’s t/ tests (moving from 2 tests / 15 cases to 5 tests / 42 cases), including reverse-index (.rev) validation and Git index version behaviors.
Changes:
- Extend conformance harness (new curated tests, per-test
--run=selectors, summary vs verbose output, and a Go-basedtest-toolhelper). - Add internal “staging” plumbing to mirror upstreamable go-git features (revfile header/row validation, v1 idx encoder, mktree parsing/raw tree writing, git date parsing).
- Add/extend many
gogitsubcommands needed by upstream tests (pack/index plumbing, fsck, index/tree plumbing, config override support, etc.) with accompanying unit tests.
Reviewed changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents CLI purpose and conformance workflow |
| Makefile | Changes build output directory to build/bin/ |
| internal/plumbing/object/mktree.go | Parse mktree input and write raw tree objects |
| internal/plumbing/object/mktree_test.go | Tests for mktree parsing and raw tree writing |
| internal/plumbing/object/identity.go | Git date (<secs> <±HHMM>) parser for env identity |
| internal/plumbing/object/identity_test.go | Tests for git date parsing edge cases |
| internal/plumbing/format/revfile/validate.go | .rev header + row-position validation helpers and fsck message formatting |
| internal/plumbing/format/revfile/validate_test.go | Tests for revfile header/row validation + message mapping |
| internal/plumbing/format/idxfile/encoder_v1.go | v1 pack index encoder |
| internal/plumbing/format/idxfile/encoder_v1_test.go | Tests for v1 idx encoding + offset bounds |
| conformance/tests.txt | Curated upstream tests list expanded + selector for t1600 |
| conformance/run.sh | Harness improvements: selectors, summary output mode, Go test-tool build |
| cmd/gogit/write-tree.go | Adds write-tree command |
| cmd/gogit/write-tree_test.go | Tests for write-tree behavior |
| cmd/gogit/upload-pack.go | Ensures repository Close() is deferred |
| cmd/gogit/update-server-info.go | Ensures repository Close() is deferred |
| cmd/gogit/update-index.go | Adds update-index --show-index-version |
| cmd/gogit/update-index_test.go | Tests for update-index --show-index-version |
| cmd/gogit/treebuild.go | Tree construction/writing from index entries |
| cmd/gogit/tag.go | Adds lightweight tag creation |
| cmd/gogit/tag_test.go | Tests for lightweight tag creation |
| cmd/gogit/show-index.go | Adds show-index (read idx on stdin) |
| cmd/gogit/show-index_test.go | Tests for show-index against generated idx |
| cmd/gogit/rev-parse.go | Adds rev-parse (rev → hash resolution) |
| cmd/gogit/rev-parse_test.go | Tests for rev-parse HEAD |
| cmd/gogit/rev-list.go | Adds minimal rev-list --all --objects support |
| cmd/gogit/rev-list_test.go | Tests for rev-list --all --objects output shape |
| cmd/gogit/rev-index.go | Helpers to encode/write .rev files |
| cmd/gogit/repack.go | Adds repack -a [-d] |
| cmd/gogit/repack_test.go | Tests for repack -ad cleaning loose objects |
| cmd/gogit/receive-pack.go | Ensures repository Close() is deferred |
| cmd/gogit/read-tree.go | Adds read-tree (tree-ish → index), with null-SHA gating |
| cmd/gogit/read-tree_test.go | Tests for read-tree null-SHA behavior |
| cmd/gogit/push.go | Ensures repository Close() is deferred |
| cmd/gogit/pull.go | Ensures repository Close() is deferred |
| cmd/gogit/prune-packed.go | Adds prune-packed |
| cmd/gogit/prune-packed_test.go | Tests for pruning loose objects after packing |
| cmd/gogit/pack-objects.go | Adds pack-objects and shared pack/idx writing helpers |
| cmd/gogit/pack-objects_test.go | Tests for pack creation, idx v1, --all, and .rev writing |
| cmd/gogit/mktree.go | Adds mktree command |
| cmd/gogit/mktree_test.go | Tests for mktree including null SHA acceptance |
| cmd/gogit/main.go | Default “no subcommand” behavior + persistent -c key=value override |
| cmd/gogit/indexversion.go | Implements index version selection/warnings logic |
| cmd/gogit/indexversion_test.go | Tests for index version selection precedence/warnings |
| cmd/gogit/index-pack.go | Expands index-pack flags/modes and strict streaming implementation |
| cmd/gogit/index-pack_test.go | Tests for index-pack new modes (-o, --verify, .rev) |
| cmd/gogit/hash-object.go | Adds hash-object |
| cmd/gogit/hash-object_test.go | Tests for hashing/writing objects and --literally |
| cmd/gogit/fsck.go | Adds fsck including .rev validation integration |
| cmd/gogit/fsck_test.go | Tests for fsck clean repo + corrupted loose object |
| cmd/gogit/fetch.go | Ensures repository Close() is deferred |
| cmd/gogit/count-objects.go | Adds count-objects + shared repoGitDir helper |
| cmd/gogit/count-objects_test.go | Tests for count-objects output format |
| cmd/gogit/config.go | Implements top-level -c key=value config override plumbing |
| cmd/gogit/config-cmd.go | Adds config command (get/set/unset-all) |
| cmd/gogit/config_test.go | Tests for -c parsing and boolean override behavior |
| cmd/gogit/commit.go | Uses shared git date parser from internal/plumbing/object |
| cmd/gogit/commit_test.go | Removes old date parser test (moved to internal pkg) |
| cmd/gogit/cat-file.go | Adds cat-file <type> <oid> and --batch-check=<format> rendering |
| cmd/gogit/cat-file_test.go | Tests for typed printing and mismatch behavior |
| cmd/gogit/add.go | Adds index version selection + warnings behavior on add |
| cmd/gogit/add_test.go | Tests for warnings and “no warning with existing index” behavior |
| cmd/gogit-test-tool/stubs.go | Implements helper subcommands (sha1/sha256/date/path-utils/env-helper) |
| cmd/gogit-test-tool/main.go | Entry point and subcommand dispatch for test-tool helper |
| cmd/gogit-test-tool/genrandom.go | Implements upstream-compatible genrandom |
| cmd/gogit-test-tool/genrandom_test.go | Tests for genrandom determinism and golden output |
| cmd/gogit-test-tool/delta.go | Implements delta -p via go-git delta patching |
| cmd/gogit-test-tool/delta_test.go | Tests for delta apply and corrupt delta rejection |
| .github/workflows/build.yml | CI runs conformance in verbose mode |
| .entire/settings.json | Adds Entire configuration (enabled + telemetry) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
+35
| for range length { | ||
| next = next*1103515245 + 12345 | ||
|
|
||
| if err := bw.WriteByte(byte((next >> 16) & 0xff)); err != nil { | ||
| return err | ||
| } | ||
| } |
Comment on lines
+100
to
+102
| needsPostProcess := opts.output != "" || opts.fsckObjects || opts.indexVersion != 2 || opts.revIndex || opts.strict | ||
| if !needsPostProcess { | ||
| return indexPackRun(r, cmd.InOrStdin(), false) |
Comment on lines
+306
to
+310
| if idxPath == "" { | ||
| idxPath = packPath[:len(packPath)-len(".pack")] + ".idx" | ||
| } | ||
|
|
||
| if err := writeIdxFile(idxPath, idx, 2, packHash); err != nil { |
Comment on lines
+78
to
+85
| for { | ||
| e, err := iter.Next() | ||
| if err != nil { | ||
| break | ||
| } | ||
|
|
||
| out[e.Hash] = struct{}{} | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Expands the
gogitconformance harness from the two passing upstreamtests we started with (
t2008,t5308→ 15 cases) to five testsand 42 cases by landing the gogit surface that the new graduations
require. Each commit adds a cohesive slice of plumbing/porcelain plus
the harness wiring needed to drive it.
t2008-checkout-subdir.sht5308-pack-detect-duplicates.sh--strict(already gated)t5325-reverse-index.sh.revfiles: write + verify + fsckt1600-index.sh.git/indexversion selection and warningst1601-index-bogus.shmake conformanceexits 0; CI matrix unchanged.Per-commit summary
gogit: add pack-write surface and graduate t5325Lands the pack-write half of upstream-Git conformance. New subcommands:
hash-object,count-objects,fsck(incl..revfile validation),show-index,pack-objects(with--all,--index-version, plus ahand-rolled v1 idx encoder),
repack [-a] [-d],rev-parse,rev-list --objects --no-object-names --all,tag(lightweight),config [<key> [<value>]],diff --no-index --ignore-cr-at-eol(Git-for-Windows
GIT_TEST_CMPshim).index-packgains-o,--fsck-objects,--index-version,--verify(also verifies.revwhen paired with
--rev-index),--rev-index,--no-rev-index, anda positional-pack-file mode.
cat-filegains--batch-check=<format>with%(objectname|objecttype|objectsize| objectsize:disk)tokens. Top-level-c <key>=<value>config override.index-pack --strictis now streaming (memory bounded by the parserworking set, not the full pack). Every
git.PlainOpen*site has apaired
defer r.Close(). Logic that should live in go-git is stagedunder
internal/plumbing/format/{idxfile,revfile}andinternal/plumbing/object(date parser).conformance: add summary output mode and README contextmake conformancedefaults to a terse summary (harness headers,# passed alllines, the TAP plan, and anynot okfailures). Fullupstream trace stays opt-in via
CONFORMANCE_VERBOSE=1. CI sets thatso captured logs and TAP artifacts remain inspectable.
cmd/gogit/ main.godrops the bare-invocation usage barf (test-lib only needsthe exit-1 detection).
conformance/run.shgains a comment blockexplaining why upstream-test fetching deliberately uses the host
gitbinary. New README sections on the CLI's purpose, theinternal/staging area for missing go-git features, and howmake conformanceworks.gogit: add t5303 conformance surface and gogit-test-tool helperLands the surface t5303 exercises. Result: 14/36 t5303 cases pass;
the remaining 22 are blocked on a go-git pack-reader change
(documented inline in
conformance/tests.txt) so t5303 itself is notyet graduated. New gogit:
prune-packed,cat-file <type> <oid>.Replaces the harness's inline shell
test-toolstub with a proper Gobinary at
cmd/gogit-test-tool/supplyinggenrandom(byte-equivalentto upstream's
t/helper/test-genrandom.cPRNG) anddelta -p(viago-git's
packfile.PatchDelta), plus Go ports of the previous shellsubcommands (
sha1,sha256,date,path-utils,env-helper).make buildwrites binaries tobuild/bin/rather thanbuild/root.gogit: add index/tree plumbing and graduate t1600 + t1601Graduates t1601 (4/4) and t1600 cases 1-5,7 (6 of 7 — case 6 uses
git submodule add, skipped via a new per-test selector intests.txt). New subcommands:mktree,read-tree <tree>(refusesnull SHAs unless
GIT_ALLOW_NULL_SHA1=1),write-tree,update-index --show-index-version.gogit addlearns to honorGIT_INDEX_VERSIONenv >index.versionconfig >feature.manyFilesFollow-ups
on additional setup-side subcommands (
commit-tree,ls-tree,update-ref).delta-base resolution via the storer's OID lookup when the in-pack
base read fails (concrete repro inline in
tests.txt).support and
index.skipHashhonoring land.