Draft
Conversation
Add RefStorage type to plumbing/format/config with constants for "files" and "reftable" backends. Parse and marshal the extensions.refStorage key in the config Extensions struct. This is preparatory work for reftable backend support. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implement a read-only parser for the reftable binary reference storage format (https://git-scm.com/docs/reftable). The package supports: - Varint encoding/decoding - Ref records (hash, hash+peeled, symbolic, deletion tombstones) - Log records (zlib-compressed blocks with committer info) - Index block traversal for O(log n) lookups - Restart-point binary search within blocks - Stack reader (tables.list with newest-first dedup) - Both reftable v1 (SHA-1) and v2 (SHA-256) formats Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Integrate the reftable format parser into the filesystem storage layer: - Add ReftableReferenceStorage implementing storer.ReferenceStorer - Add ReftableReflogStorage implementing storer.ReflogStorer - Refactor Storage to use interface fields for reference and reflog storage, enabling pluggable backends - Detect extensions.refStorage=reftable in NewStorageWithOptions and instantiate reftable-backed storage when present - Declare support for the refstorage extension in SupportsExtension - Write operations return reftable.ErrReadOnly until write support is implemented Repositories using reftable can now be opened with PlainOpen for read operations (reference lookup, iteration, reflog reading). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pjbgf
requested changes
Apr 7, 2026
| @@ -0,0 +1 @@ | |||
| 0x000000000001-0x000000000006-6b4a0580.ref | |||
Member
There was a problem hiding this comment.
Please delete the testdata/repo1. Instead, use fixtures.ByTag("reftable") to get a fixture that depends on that format.
Currently we only have a single fixture, but that should be a good start.
| // Each byte contributes 7 bits to the value. When the continuation flag | ||
| // is set, the accumulated value is incremented by 1 before shifting to | ||
| // avoid redundant encodings. | ||
| func getVarint(buf []byte) (val uint64, n int) { |
Member
There was a problem hiding this comment.
This logic exist in different parts of the codebase, we should extract it and reuse across the board instead of duplicating it here.
Comment on lines
+44
to
+46
| // Cached full file data for simplicity. For very large tables, | ||
| // this could be replaced with on-demand block reads. | ||
| data []byte |
Member
There was a problem hiding this comment.
Let's avoid fully loading the data into memory, that's a pattern we are trying to avoid across the board.
Comment on lines
+17
to
+25
| // SetReference is not supported for reftable (read-only). | ||
| func (r *ReftableReferenceStorage) SetReference(_ *plumbing.Reference) error { | ||
| return reftable.ErrReadOnly | ||
| } | ||
|
|
||
| // CheckAndSetReference is not supported for reftable (read-only). | ||
| func (r *ReftableReferenceStorage) CheckAndSetReference(_, _ *plumbing.Reference) error { | ||
| return reftable.ErrReadOnly | ||
| } |
Member
There was a problem hiding this comment.
Without write support this would caught users off-guard. They would be able to open repositories but then on ref updates everything would fail.
Let's implement the write part of this as part of this PR.
- Extract varint logic to utils/binary/ to eliminate duplication
with existing ReadVariableWidthInt/WriteVariableWidthInt
- Remove full-file memory caching from Table; use io.ReaderAt for
on-demand block reads instead of loading entire file into memory
- Add reftable write support: writer.go encodes ref/log records with
prefix compression, restart points, zlib-compressed log blocks, and
CRC-32 footer; Stack gains SetRef/RemoveRef/AddLog methods;
ReftableReferenceStorage and ReftableReflogStorage now implement
all write operations
- Replace testdata/repo1 with fixtures.ByTag("reftable") from
go-git-fixtures
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The reftable block reader used a fixed file-header offset (24 bytes) when adjusting restart-point positions for binary search, which is only correct for the first block. Subsequent blocks have no file header, so the offset must be 4 (block header only). Fix seek() to use the block's own headerLen instead of a caller-supplied constant. Also fix the git index decoder to accept all-zero checksums, which modern Git writes when index.skipHash is enabled. This is a pre-existing bug on main unrelated to reftable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AngerM
commented
Apr 8, 2026
| if h.IsZero() { | ||
| trace.Internal.Printf("index: stored checksum is all-zero (skipHash index), accepting") | ||
| return nil | ||
| } |
Author
There was a problem hiding this comment.
skipHash can also be set in global (not per-repo) config, which go-git was missing and causing a bunch of local tests to fail
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
2 participants
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.
RE: #1827
Fed Opus 4.6 the reftable spec from git and asked it to add support.
Tested locally:
entirecli and replacing my systems version with the forked build_examples/openpackage