detached ad03ed7

fix(git): harden the commit-graph and Bloom readers

- bloom: on a v1 layer, test both the signed-char and the unsigned (v2)
  hash of keys with a byte >= 0x80, so filters written where char is
  unsigned (arm64, ppc64, s390x) give no false negatives (GO-001)
- bloom: keys git never inserts (empty, leading or doubled slash, "." or
  ".." segment, NUL) are unknown instead of "unchanged"; one trailing
  slash is stripped like git (GO-004)
- graph: close is idempotent and retain fails after close (GO-002)
- graph: reread the chain when a layer vanishes under a concurrent
  split write, retry 3 times, then return errNoGraph wrapping the cause
  (GO-003)
- graph: map files through the core mapFile helper, which builds on
  darwin, freebsd and windows (GO-005)
- graph: check each layer trailer against its chain name (GO-007)
- graph: a wrongly sized GDA2, BIDX or BDAT disables only that feature
  of the layer, as git does (GO-006)
- graph: generation looks the layer up once (GO-009)
- tests: unsigned v1 writer, unusable keys, concurrent close/retain,
  vanished layer hook, split-write stress, checksum, optional chunks,
  and FuzzGraphAssemble covering parseGraphChain and assembleGraph
commit
ad03ed7534163481161bd9583b4c4606bc0bd442
author
brice · · 2026-10-10 20:48 UTC
parent
5658779
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +804 -87
M bloom.go +80 -17
@@ -20,10 +20,18 @@ import (
2020 // taken modulo the number of bits of the filter. Bit n of the filter is
2121 // bit (n & 7) of byte n/8.
2222 //
23-// BDAT header version 1 is the original hash, which sign-extends bytes >=
24-// 0x80 when git was built with a signed char (x86); version 2 fixes it with
25-// unsigned arithmetic. Both are supported; the version is read per layer
26-// from BDAT. For paths made of bytes < 0x80 the two are identical.
23+// BDAT header version 1 is the original hash, which uses plain char: it
24+// sign-extends bytes >= 0x80 when git was built with a signed char (x86) but
25+// not where char is unsigned (Linux arm64, ppc64, s390x), where it equals the
26+// version 2 hash. The file does not say which one wrote it, so on a version 1
27+// layer a key holding a byte >= 0x80 is tested with both hashes and the
28+// answer is "maybe" if either matches; trying both can only add false
29+// positives, never false negatives. Version 2 always uses unsigned
30+// arithmetic. The version is read per layer from BDAT. For paths made of
31+// bytes < 0x80 all hashes are identical.
32+//
33+// Only paths git inserts can be answered: see newBloomKey for the paths that
34+// are reported as unknown.
2735 //
2836 // A filter of one byte 0x00 means "no changed path" and one byte 0xff means
2937 // "too many changed paths (more than 512)": the latter is reported as
@@ -37,20 +45,36 @@ const (
3745 )
3846
3947 // bloomKey holds the precomputed hashes of a path for both hash versions. It
40-// is a small value; build it once per path and reuse it for many commits.
48+// is a small value; build it once per path and reuse it for many commits. The
49+// zero value is not usable and always yields an unknown answer.
4150 type bloomKey struct {
42- v1, v2 [2]uint32
51+ v1, v2 [2]uint32 // signed-char (version 1) and unsigned (version 2) hashes
52+ high bool // the path has a byte >= 0x80, so v1 and v2 differ
53+ usable bool // git can have inserted this key
4354 }
4455
4556 // newBloomKey computes the key of path. It does not allocate.
57+//
58+// One trailing slash is stripped, as git does for pathspecs, so "dir/" and
59+// "dir" give the same key. A path git never inserts yields an unusable key,
60+// for which every lookup is unknown (the caller must diff the trees): the
61+// empty path, a path starting with "/", with an empty segment ("a//b", or a
62+// second trailing slash), with a "." or ".." segment, or holding a NUL byte.
4663 func newBloomKey(path []byte) bloomKey {
47- k := bloomKey{v2: [2]uint32{
64+ if n := len(path); n > 0 && path[n-1] == '/' {
65+ path = path[:n-1]
66+ }
67+ if !bloomPathUsable(path) {
68+ return bloomKey{}
69+ }
70+ k := bloomKey{usable: true, v2: [2]uint32{
4871 murmur3Seeded(bloomSeed0, path, false),
4972 murmur3Seeded(bloomSeed1, path, false),
5073 }}
5174 k.v1 = k.v2
5275 for _, b := range path {
5376 if b >= 0x80 {
77+ k.high = true
5478 k.v1 = [2]uint32{
5579 murmur3Seeded(bloomSeed0, path, true),
5680 murmur3Seeded(bloomSeed1, path, true),
@@ -61,6 +85,33 @@ func newBloomKey(path []byte) bloomKey {
6185 return k
6286 }
6387
88+// bloomPathUsable reports whether path is a normalised, repository-relative
89+// path whose segments are all non-empty and not "." or "..".
90+func bloomPathUsable(path []byte) bool {
91+ if len(path) == 0 {
92+ return false
93+ }
94+ start := 0
95+ for i := 0; i <= len(path); i++ {
96+ if i < len(path) && path[i] != '/' {
97+ if path[i] == 0 {
98+ return false
99+ }
100+ continue
101+ }
102+ switch seg := path[start:i]; {
103+ case len(seg) == 0:
104+ return false
105+ case len(seg) == 1 && seg[0] == '.':
106+ return false
107+ case len(seg) == 2 && seg[0] == '.' && seg[1] == '.':
108+ return false
109+ }
110+ start = i + 1
111+ }
112+ return true
113+}
114+
64115 // newBloomKeyString is newBloomKey for a string path.
65116 func newBloomKeyString(path string) bloomKey {
66117 return newBloomKey([]byte(path))
@@ -146,30 +197,42 @@ func (g *commitGraph) bloomFilter(pos uint32) (l *graphLayer, filter []byte, ok
146197 // maybeChangedKey reports whether the path of k may have changed in the
147198 // commit at pos relative to its first parent. maybe=false is definitive (the
148199 // path did not change); maybe=true may be a false positive. ok=false means
149-// the answer is unknown (no filter, too large, truncated, bad position) and
150-// the caller must diff the trees.
200+// the answer is unknown (unusable key, no filter, too large, truncated, bad
201+// position) and the caller must diff the trees.
151202 func (g *commitGraph) maybeChangedKey(pos uint32, k *bloomKey) (maybe, ok bool) {
203+ if !k.usable {
204+ return false, false
205+ }
152206 l, f, ok := g.bloomFilter(pos)
153207 if !ok {
154208 return false, false
155209 }
156- h := &k.v2
157- if l.bloomVersion == 1 {
158- h = &k.v1
210+ if l.bloomVersion != 1 {
211+ return bloomContains(f, l.bloomHashes, &k.v2), true
212+ }
213+ // Version 1: written with signed or unsigned chars depending on the
214+ // platform of the writer; accept either when they differ.
215+ if bloomContains(f, l.bloomHashes, &k.v1) {
216+ return true, true
159217 }
218+ return k.high && bloomContains(f, l.bloomHashes, &k.v2), true
219+}
220+
221+// bloomContains tests the probes of hash pair h against filter f.
222+func bloomContains(f []byte, hashes uint32, h *[2]uint32) bool {
160223 mod := uint64(len(f)) * 8
161- var h0, h1 = h[0], h[1]
162- for i := uint32(0); i < l.bloomHashes; i++ {
224+ h0, h1 := h[0], h[1]
225+ for i := uint32(0); i < hashes; i++ {
163226 bit := uint64(h0+i*h1) % mod
164227 if f[bit>>3]&(1<<(bit&7)) == 0 {
165- return false, true
228+ return false
166229 }
167230 }
168- return true, true
231+ return true
169232 }
170233
171234 // maybeChanged is maybeChangedKey for a path given as bytes, without a
172-// trailing slash, relative to the repository root. Use newBloomKey when
235+// trailing slash (one is tolerated), relative to the repository root. Use newBloomKey when
173236 // testing the same path against many commits.
174237 func (g *commitGraph) maybeChanged(pos uint32, path []byte) (maybe, ok bool) {
175238 k := newBloomKey(path)
@@ -20,10 +20,18 @@ import (
20 // taken modulo the number of bits of the filter. Bit n of the filter is20 // taken modulo the number of bits of the filter. Bit n of the filter is
21 // bit (n & 7) of byte n/8.21 // bit (n & 7) of byte n/8.
22 //22 //
23-// BDAT header version 1 is the original hash, which sign-extends bytes >=23+// BDAT header version 1 is the original hash, which uses plain char: it
24-// 0x80 when git was built with a signed char (x86); version 2 fixes it with24+// sign-extends bytes >= 0x80 when git was built with a signed char (x86) but
25-// unsigned arithmetic. Both are supported; the version is read per layer25+// not where char is unsigned (Linux arm64, ppc64, s390x), where it equals the
26-// from BDAT. For paths made of bytes < 0x80 the two are identical.26+// version 2 hash. The file does not say which one wrote it, so on a version 1
27+// layer a key holding a byte >= 0x80 is tested with both hashes and the
28+// answer is "maybe" if either matches; trying both can only add false
29+// positives, never false negatives. Version 2 always uses unsigned
30+// arithmetic. The version is read per layer from BDAT. For paths made of
31+// bytes < 0x80 all hashes are identical.
32+//
33+// Only paths git inserts can be answered: see newBloomKey for the paths that
34+// are reported as unknown.
27 //35 //
28 // A filter of one byte 0x00 means "no changed path" and one byte 0xff means36 // A filter of one byte 0x00 means "no changed path" and one byte 0xff means
29 // "too many changed paths (more than 512)": the latter is reported as37 // "too many changed paths (more than 512)": the latter is reported as
@@ -37,20 +45,36 @@ const (
37 )45 )
38 46
39 // bloomKey holds the precomputed hashes of a path for both hash versions. It47 // bloomKey holds the precomputed hashes of a path for both hash versions. It
40-// is a small value; build it once per path and reuse it for many commits.48+// is a small value; build it once per path and reuse it for many commits. The
49+// zero value is not usable and always yields an unknown answer.
41 type bloomKey struct {50 type bloomKey struct {
42- v1, v2 [2]uint3251+ v1, v2 [2]uint32 // signed-char (version 1) and unsigned (version 2) hashes
52+ high bool // the path has a byte >= 0x80, so v1 and v2 differ
53+ usable bool // git can have inserted this key
43 }54 }
44 55
45 // newBloomKey computes the key of path. It does not allocate.56 // newBloomKey computes the key of path. It does not allocate.
57+//
58+// One trailing slash is stripped, as git does for pathspecs, so "dir/" and
59+// "dir" give the same key. A path git never inserts yields an unusable key,
60+// for which every lookup is unknown (the caller must diff the trees): the
61+// empty path, a path starting with "/", with an empty segment ("a//b", or a
62+// second trailing slash), with a "." or ".." segment, or holding a NUL byte.
46 func newBloomKey(path []byte) bloomKey {63 func newBloomKey(path []byte) bloomKey {
47- k := bloomKey{v2: [2]uint32{64+ if n := len(path); n > 0 && path[n-1] == '/' {
65+ path = path[:n-1]
66+ }
67+ if !bloomPathUsable(path) {
68+ return bloomKey{}
69+ }
70+ k := bloomKey{usable: true, v2: [2]uint32{
48 murmur3Seeded(bloomSeed0, path, false),71 murmur3Seeded(bloomSeed0, path, false),
49 murmur3Seeded(bloomSeed1, path, false),72 murmur3Seeded(bloomSeed1, path, false),
50 }}73 }}
51 k.v1 = k.v274 k.v1 = k.v2
52 for _, b := range path {75 for _, b := range path {
53 if b >= 0x80 {76 if b >= 0x80 {
77+ k.high = true
54 k.v1 = [2]uint32{78 k.v1 = [2]uint32{
55 murmur3Seeded(bloomSeed0, path, true),79 murmur3Seeded(bloomSeed0, path, true),
56 murmur3Seeded(bloomSeed1, path, true),80 murmur3Seeded(bloomSeed1, path, true),
@@ -61,6 +85,33 @@ func newBloomKey(path []byte) bloomKey {
61 return k85 return k
62 }86 }
63 87
88+// bloomPathUsable reports whether path is a normalised, repository-relative
89+// path whose segments are all non-empty and not "." or "..".
90+func bloomPathUsable(path []byte) bool {
91+ if len(path) == 0 {
92+ return false
93+ }
94+ start := 0
95+ for i := 0; i <= len(path); i++ {
96+ if i < len(path) && path[i] != '/' {
97+ if path[i] == 0 {
98+ return false
99+ }
100+ continue
101+ }
102+ switch seg := path[start:i]; {
103+ case len(seg) == 0:
104+ return false
105+ case len(seg) == 1 && seg[0] == '.':
106+ return false
107+ case len(seg) == 2 && seg[0] == '.' && seg[1] == '.':
108+ return false
109+ }
110+ start = i + 1
111+ }
112+ return true
113+}
114+
64 // newBloomKeyString is newBloomKey for a string path.115 // newBloomKeyString is newBloomKey for a string path.
65 func newBloomKeyString(path string) bloomKey {116 func newBloomKeyString(path string) bloomKey {
66 return newBloomKey([]byte(path))117 return newBloomKey([]byte(path))
@@ -146,30 +197,42 @@ func (g *commitGraph) bloomFilter(pos uint32) (l *graphLayer, filter []byte, ok
146 // maybeChangedKey reports whether the path of k may have changed in the197 // maybeChangedKey reports whether the path of k may have changed in the
147 // commit at pos relative to its first parent. maybe=false is definitive (the198 // commit at pos relative to its first parent. maybe=false is definitive (the
148 // path did not change); maybe=true may be a false positive. ok=false means199 // path did not change); maybe=true may be a false positive. ok=false means
149-// the answer is unknown (no filter, too large, truncated, bad position) and200+// the answer is unknown (unusable key, no filter, too large, truncated, bad
150-// the caller must diff the trees.201+// position) and the caller must diff the trees.
151 func (g *commitGraph) maybeChangedKey(pos uint32, k *bloomKey) (maybe, ok bool) {202 func (g *commitGraph) maybeChangedKey(pos uint32, k *bloomKey) (maybe, ok bool) {
203+ if !k.usable {
204+ return false, false
205+ }
152 l, f, ok := g.bloomFilter(pos)206 l, f, ok := g.bloomFilter(pos)
153 if !ok {207 if !ok {
154 return false, false208 return false, false
155 }209 }
156- h := &k.v2210+ if l.bloomVersion != 1 {
157- if l.bloomVersion == 1 {211+ return bloomContains(f, l.bloomHashes, &k.v2), true
158- h = &k.v1212+ }
213+ // Version 1: written with signed or unsigned chars depending on the
214+ // platform of the writer; accept either when they differ.
215+ if bloomContains(f, l.bloomHashes, &k.v1) {
216+ return true, true
159 }217 }
218+ return k.high && bloomContains(f, l.bloomHashes, &k.v2), true
219+}
220+
221+// bloomContains tests the probes of hash pair h against filter f.
222+func bloomContains(f []byte, hashes uint32, h *[2]uint32) bool {
160 mod := uint64(len(f)) * 8223 mod := uint64(len(f)) * 8
161- var h0, h1 = h[0], h[1]224+ h0, h1 := h[0], h[1]
162- for i := uint32(0); i < l.bloomHashes; i++ {225+ for i := uint32(0); i < hashes; i++ {
163 bit := uint64(h0+i*h1) % mod226 bit := uint64(h0+i*h1) % mod
164 if f[bit>>3]&(1<<(bit&7)) == 0 {227 if f[bit>>3]&(1<<(bit&7)) == 0 {
165- return false, true228+ return false
166 }229 }
167 }230 }
168- return true, true231+ return true
169 }232 }
170 233
171 // maybeChanged is maybeChangedKey for a path given as bytes, without a234 // maybeChanged is maybeChangedKey for a path given as bytes, without a
172-// trailing slash, relative to the repository root. Use newBloomKey when235+// trailing slash (one is tolerated), relative to the repository root. Use newBloomKey when
173 // testing the same path against many commits.236 // testing the same path against many commits.
174 func (g *commitGraph) maybeChanged(pos uint32, path []byte) (maybe, ok bool) {237 func (g *commitGraph) maybeChanged(pos uint32, path []byte) (maybe, ok bool) {
175 k := newBloomKey(path)238 k := newBloomKey(path)
M bloom_test.go +131 -4
@@ -220,6 +220,8 @@ func TestBloomHighBitVersions(t *testing.T) {
220220 key := newBloomKey([]byte(k))
221221 other := key
222222 other.v1, other.v2 = key.v2, key.v1
223+ // A v1 layer tries both hashes: test one only.
224+ other.high = false
223225 if maybe, ok := g.maybeChangedKey(pos, &other); ok && !maybe {
224226 wrong++
225227 }
@@ -336,10 +338,115 @@ func TestBloomIgnoredSettings(t *testing.T) {
336338 t.Errorf("version %d hashes %d should be unknown", c.version, c.hashes)
337339 }
338340 }
339- // BIDX without BDAT, BDAT too short.
340- s := &synthLayer{ids: synthIDs(2), bidx: []uint32{1, 2}, bdat: []byte{0, 0, 0, 2}}
341- if _, err := parseGraphLayer(s.bytes()); err == nil {
342- t.Error("short BDAT accepted")
341+}
342+
343+// A wrongly sized BIDX or a BDAT shorter than its header disables the Bloom
344+// filters of the layer only, like git; the rest of the layer stays usable.
345+func TestBloomBadChunkDisablesFeature(t *testing.T) {
346+ hdr := binary.BigEndian.AppendUint32(nil, 2)
347+ hdr = binary.BigEndian.AppendUint32(hdr, 7)
348+ hdr = binary.BigEndian.AppendUint32(hdr, 10)
349+ for name, s := range map[string]*synthLayer{
350+ "short BDAT": {ids: synthIDs(2), bidx: []uint32{1, 2}, bdat: []byte{0, 0, 0, 2}},
351+ "empty BDAT": {ids: synthIDs(2), bidx: []uint32{1, 2}, bdat: []byte{}},
352+ "BIDX too few": {ids: synthIDs(3), bidx: []uint32{1, 2}, bdat: append(hdr[:12:12], 0, 0)},
353+ "BIDX too many": {ids: synthIDs(2), bidx: []uint32{1, 2, 2}, bdat: append(hdr[:12:12], 0, 0)},
354+ } {
355+ t.Run(name, func(t *testing.T) {
356+ g := mustParseSynth(t, s.bytes())
357+ defer func() { _ = g.close() }()
358+ if g.layers[0].hasBloom {
359+ t.Fatal("hasBloom")
360+ }
361+ for pos := range g.numCommits() {
362+ if g.hasBloom(pos) {
363+ t.Errorf("pos %d: hasBloom", pos)
364+ }
365+ if _, ok := g.maybeChanged(pos, []byte("a")); ok {
366+ t.Errorf("pos %d: answer without filter", pos)
367+ }
368+ id, err := g.commitID(pos)
369+ if err != nil {
370+ t.Fatal(err)
371+ }
372+ if got, ok := g.lookup(id); !ok || got != pos {
373+ t.Errorf("lookup(%d) = %d, %v", pos, got, ok)
374+ }
375+ }
376+ })
377+ }
378+}
379+
380+// A v1 filter written where char is unsigned (Linux arm64, ppc64, s390x) holds
381+// the same bits as a v2 one. It must be found as well as one written with
382+// signed chars.
383+func TestBloomV1UnsignedWriter(t *testing.T) {
384+ const hashes = 7
385+ path := []byte("d\xe9\xff/\xc3\xa9.txt")
386+ k := newBloomKey(path)
387+ if !k.high || k.v1 == k.v2 {
388+ t.Fatal("test path does not distinguish the hashes")
389+ }
390+ signed := make([]byte, 64)
391+ setBloomBits(signed, hashes, k, true)
392+ unsigned := make([]byte, 64)
393+ setBloomBits(unsigned, hashes, k, false)
394+ other := make([]byte, 64)
395+ setBloomBits(other, hashes, newBloomKey([]byte("zz/other")), false)
396+ g := synthBloom(t, 1, hashes, 10, [][]byte{signed, unsigned, other})
397+ defer func() { _ = g.close() }()
398+ for pos, want := range []bool{true, true, false} {
399+ if maybe, ok := g.maybeChanged(uint32(pos), path); !ok || maybe != want {
400+ t.Errorf("v1 filter %d: maybe=%v ok=%v, want %v", pos, maybe, ok, want)
401+ }
402+ }
403+ // A v2 layer must not accept the signed hash.
404+ g2 := synthBloom(t, 2, hashes, 10, [][]byte{signed, unsigned})
405+ defer func() { _ = g2.close() }()
406+ if maybe, _ := g2.maybeChanged(0, path); maybe {
407+ t.Error("v2 layer matched a signed-char filter")
408+ }
409+ if maybe, _ := g2.maybeChanged(1, path); !maybe {
410+ t.Error("v2 layer missed an unsigned filter")
411+ }
412+}
413+
414+// Keys git never inserts are unknown, not "unchanged".
415+func TestBloomUnusableKeys(t *testing.T) {
416+ const hashes = 7
417+ f := make([]byte, 64)
418+ for _, p := range []string{"a/b", "a", ".hidden", "..x", "a/.b"} {
419+ setBloomBits(f, hashes, newBloomKey([]byte(p)), false)
420+ }
421+ g := synthBloom(t, 2, hashes, 10, [][]byte{f, {0x00}})
422+ defer func() { _ = g.close() }()
423+
424+ for _, p := range []string{"", "/", "//", "/a", "/a/b", "a//b", "a/b//", "./a", "a/./b", "a/.", ".", "..",
425+ "../a", "a/../b", "a/..", "a\x00b"} {
426+ for pos := range uint32(2) {
427+ if maybe, ok := g.maybeChanged(pos, []byte(p)); ok || maybe {
428+ t.Errorf("%q at %d: maybe=%v ok=%v, want unknown", p, pos, maybe, ok)
429+ }
430+ }
431+ if k := newBloomKey([]byte(p)); k.usable {
432+ t.Errorf("%q: key usable", p)
433+ }
434+ }
435+ // One trailing slash is dropped, like git does.
436+ for _, p := range []string{"a/b", "a/b/", "a", "a/", ".hidden", "..x", "a/.b/"} {
437+ if maybe, ok := g.maybeChanged(0, []byte(p)); !ok || !maybe {
438+ t.Errorf("%q: maybe=%v ok=%v, want found", p, maybe, ok)
439+ }
440+ if maybe, ok := g.maybeChanged(1, []byte(p)); !ok || maybe {
441+ t.Errorf("%q in empty filter: maybe=%v ok=%v", p, maybe, ok)
442+ }
443+ }
444+ if newBloomKey([]byte("a/b/")) != newBloomKey([]byte("a/b")) {
445+ t.Error("trailing slash changes the key")
446+ }
447+ var zero bloomKey
448+ if _, ok := g.maybeChangedKey(0, &zero); ok {
449+ t.Error("zero key must be unknown")
343450 }
344451 }
345452
@@ -388,6 +495,12 @@ func FuzzBloomLookup(f *testing.F) {
388495 if maybe && !ok {
389496 t.Fatal("maybe without ok")
390497 }
498+ if ok && !k.usable {
499+ t.Fatal("answer for an unusable key")
500+ }
501+ }
502+ if !k.usable {
503+ return
391504 }
392505 // A path inserted into a valid filter is always found.
393506 half := len(filter) / 2
@@ -403,6 +516,20 @@ func FuzzBloomLookup(f *testing.F) {
403516 if maybe, ok := g2.maybeChangedKey(1, &k); ok && !maybe {
404517 t.Fatal("false negative after insertion")
405518 }
519+ if version == 1 {
520+ // The same filter written with the unsigned hash.
521+ third := append([]byte(nil), filter[half:]...)
522+ setBloomBits(third, hashes, k, false)
523+ s.bdat = append(append(hdr[:12:12], filter[:half]...), third...)
524+ l3, err := parseGraphLayer(s.bytes())
525+ if err != nil {
526+ t.Fatal(err)
527+ }
528+ g3 := &commitGraph{layers: []graphLayer{*l3}, total: l3.n}
529+ if maybe, ok := g3.maybeChangedKey(1, &k); ok && !maybe {
530+ t.Fatal("false negative with an unsigned v1 writer")
531+ }
532+ }
406533 }
407534 })
408535 }
@@ -220,6 +220,8 @@ func TestBloomHighBitVersions(t *testing.T) {
220 key := newBloomKey([]byte(k))220 key := newBloomKey([]byte(k))
221 other := key221 other := key
222 other.v1, other.v2 = key.v2, key.v1222 other.v1, other.v2 = key.v2, key.v1
223+ // A v1 layer tries both hashes: test one only.
224+ other.high = false
223 if maybe, ok := g.maybeChangedKey(pos, &other); ok && !maybe {225 if maybe, ok := g.maybeChangedKey(pos, &other); ok && !maybe {
224 wrong++226 wrong++
225 }227 }
@@ -336,10 +338,115 @@ func TestBloomIgnoredSettings(t *testing.T) {
336 t.Errorf("version %d hashes %d should be unknown", c.version, c.hashes)338 t.Errorf("version %d hashes %d should be unknown", c.version, c.hashes)
337 }339 }
338 }340 }
339- // BIDX without BDAT, BDAT too short.341+}
340- s := &synthLayer{ids: synthIDs(2), bidx: []uint32{1, 2}, bdat: []byte{0, 0, 0, 2}}342+
341- if _, err := parseGraphLayer(s.bytes()); err == nil {343+// A wrongly sized BIDX or a BDAT shorter than its header disables the Bloom
342- t.Error("short BDAT accepted")344+// filters of the layer only, like git; the rest of the layer stays usable.
345+func TestBloomBadChunkDisablesFeature(t *testing.T) {
346+ hdr := binary.BigEndian.AppendUint32(nil, 2)
347+ hdr = binary.BigEndian.AppendUint32(hdr, 7)
348+ hdr = binary.BigEndian.AppendUint32(hdr, 10)
349+ for name, s := range map[string]*synthLayer{
350+ "short BDAT": {ids: synthIDs(2), bidx: []uint32{1, 2}, bdat: []byte{0, 0, 0, 2}},
351+ "empty BDAT": {ids: synthIDs(2), bidx: []uint32{1, 2}, bdat: []byte{}},
352+ "BIDX too few": {ids: synthIDs(3), bidx: []uint32{1, 2}, bdat: append(hdr[:12:12], 0, 0)},
353+ "BIDX too many": {ids: synthIDs(2), bidx: []uint32{1, 2, 2}, bdat: append(hdr[:12:12], 0, 0)},
354+ } {
355+ t.Run(name, func(t *testing.T) {
356+ g := mustParseSynth(t, s.bytes())
357+ defer func() { _ = g.close() }()
358+ if g.layers[0].hasBloom {
359+ t.Fatal("hasBloom")
360+ }
361+ for pos := range g.numCommits() {
362+ if g.hasBloom(pos) {
363+ t.Errorf("pos %d: hasBloom", pos)
364+ }
365+ if _, ok := g.maybeChanged(pos, []byte("a")); ok {
366+ t.Errorf("pos %d: answer without filter", pos)
367+ }
368+ id, err := g.commitID(pos)
369+ if err != nil {
370+ t.Fatal(err)
371+ }
372+ if got, ok := g.lookup(id); !ok || got != pos {
373+ t.Errorf("lookup(%d) = %d, %v", pos, got, ok)
374+ }
375+ }
376+ })
377+ }
378+}
379+
380+// A v1 filter written where char is unsigned (Linux arm64, ppc64, s390x) holds
381+// the same bits as a v2 one. It must be found as well as one written with
382+// signed chars.
383+func TestBloomV1UnsignedWriter(t *testing.T) {
384+ const hashes = 7
385+ path := []byte("d\xe9\xff/\xc3\xa9.txt")
386+ k := newBloomKey(path)
387+ if !k.high || k.v1 == k.v2 {
388+ t.Fatal("test path does not distinguish the hashes")
389+ }
390+ signed := make([]byte, 64)
391+ setBloomBits(signed, hashes, k, true)
392+ unsigned := make([]byte, 64)
393+ setBloomBits(unsigned, hashes, k, false)
394+ other := make([]byte, 64)
395+ setBloomBits(other, hashes, newBloomKey([]byte("zz/other")), false)
396+ g := synthBloom(t, 1, hashes, 10, [][]byte{signed, unsigned, other})
397+ defer func() { _ = g.close() }()
398+ for pos, want := range []bool{true, true, false} {
399+ if maybe, ok := g.maybeChanged(uint32(pos), path); !ok || maybe != want {
400+ t.Errorf("v1 filter %d: maybe=%v ok=%v, want %v", pos, maybe, ok, want)
401+ }
402+ }
403+ // A v2 layer must not accept the signed hash.
404+ g2 := synthBloom(t, 2, hashes, 10, [][]byte{signed, unsigned})
405+ defer func() { _ = g2.close() }()
406+ if maybe, _ := g2.maybeChanged(0, path); maybe {
407+ t.Error("v2 layer matched a signed-char filter")
408+ }
409+ if maybe, _ := g2.maybeChanged(1, path); !maybe {
410+ t.Error("v2 layer missed an unsigned filter")
411+ }
412+}
413+
414+// Keys git never inserts are unknown, not "unchanged".
415+func TestBloomUnusableKeys(t *testing.T) {
416+ const hashes = 7
417+ f := make([]byte, 64)
418+ for _, p := range []string{"a/b", "a", ".hidden", "..x", "a/.b"} {
419+ setBloomBits(f, hashes, newBloomKey([]byte(p)), false)
420+ }
421+ g := synthBloom(t, 2, hashes, 10, [][]byte{f, {0x00}})
422+ defer func() { _ = g.close() }()
423+
424+ for _, p := range []string{"", "/", "//", "/a", "/a/b", "a//b", "a/b//", "./a", "a/./b", "a/.", ".", "..",
425+ "../a", "a/../b", "a/..", "a\x00b"} {
426+ for pos := range uint32(2) {
427+ if maybe, ok := g.maybeChanged(pos, []byte(p)); ok || maybe {
428+ t.Errorf("%q at %d: maybe=%v ok=%v, want unknown", p, pos, maybe, ok)
429+ }
430+ }
431+ if k := newBloomKey([]byte(p)); k.usable {
432+ t.Errorf("%q: key usable", p)
433+ }
434+ }
435+ // One trailing slash is dropped, like git does.
436+ for _, p := range []string{"a/b", "a/b/", "a", "a/", ".hidden", "..x", "a/.b/"} {
437+ if maybe, ok := g.maybeChanged(0, []byte(p)); !ok || !maybe {
438+ t.Errorf("%q: maybe=%v ok=%v, want found", p, maybe, ok)
439+ }
440+ if maybe, ok := g.maybeChanged(1, []byte(p)); !ok || maybe {
441+ t.Errorf("%q in empty filter: maybe=%v ok=%v", p, maybe, ok)
442+ }
443+ }
444+ if newBloomKey([]byte("a/b/")) != newBloomKey([]byte("a/b")) {
445+ t.Error("trailing slash changes the key")
446+ }
447+ var zero bloomKey
448+ if _, ok := g.maybeChangedKey(0, &zero); ok {
449+ t.Error("zero key must be unknown")
343 }450 }
344 }451 }
345 452
@@ -388,6 +495,12 @@ func FuzzBloomLookup(f *testing.F) {
388 if maybe && !ok {495 if maybe && !ok {
389 t.Fatal("maybe without ok")496 t.Fatal("maybe without ok")
390 }497 }
498+ if ok && !k.usable {
499+ t.Fatal("answer for an unusable key")
500+ }
501+ }
502+ if !k.usable {
503+ return
391 }504 }
392 // A path inserted into a valid filter is always found.505 // A path inserted into a valid filter is always found.
393 half := len(filter) / 2506 half := len(filter) / 2
@@ -403,6 +516,20 @@ func FuzzBloomLookup(f *testing.F) {
403 if maybe, ok := g2.maybeChangedKey(1, &k); ok && !maybe {516 if maybe, ok := g2.maybeChangedKey(1, &k); ok && !maybe {
404 t.Fatal("false negative after insertion")517 t.Fatal("false negative after insertion")
405 }518 }
519+ if version == 1 {
520+ // The same filter written with the unsigned hash.
521+ third := append([]byte(nil), filter[half:]...)
522+ setBloomBits(third, hashes, k, false)
523+ s.bdat = append(append(hdr[:12:12], filter[:half]...), third...)
524+ l3, err := parseGraphLayer(s.bytes())
525+ if err != nil {
526+ t.Fatal(err)
527+ }
528+ g3 := &commitGraph{layers: []graphLayer{*l3}, total: l3.n}
529+ if maybe, ok := g3.maybeChangedKey(1, &k); ok && !maybe {
530+ t.Fatal("false negative with an unsigned v1 writer")
531+ }
532+ }
406 }533 }
407 })534 })
408 }535 }
M graph.go +97 -48
@@ -14,7 +14,6 @@ import (
1414 "os"
1515 "path/filepath"
1616 "sync/atomic"
17- "syscall"
1817 )
1918
2019 // This file reads Git commit-graph files (gitformat-commit-graph): a single
@@ -25,11 +24,18 @@ import (
2524 // Lifecycle. openCommitGraph returns a graph holding one reference (the
2625 // owner's). A reader that may outlive a concurrent reopen calls retain
2726 // (which fails once the graph is closed) and release when done. The owner
28-// calls close when it replaces the graph. The mappings are unmapped when the
29-// last reference is dropped; no accessor may be used after that. Layers are
27+// calls close when it replaces the graph; close is idempotent, only the first
28+// call drops the owner's reference. The mappings are unmapped when the last
29+// reference is dropped; no accessor may be used after that. Layers are
3030 // immutable files named after their checksum, so a concurrent
3131 // "git commit-graph write" never changes a mapped file; it can only delete
32-// it, which does not affect an existing mapping.
32+// it, which does not affect an existing mapping. It can however delete a
33+// layer between the moment the chain is read and the moment the layer is
34+// opened: openCommitGraph then starts again from the chain, and gives up with
35+// errNoGraph (callers fall back to the objects) if that keeps happening.
36+//
37+// File mapping is done by mapFile (mmap_*.go), which falls back to reading
38+// the file where mmap is unavailable.
3339 //
3440 // Positions are global across the chain: the commits of the base layer come
3541 // first, then those of the next layer, and so on. Within a layer commits are
@@ -116,58 +122,84 @@ type commitGraph struct {
116122 genV2 bool // every layer has GDA2: generation() is a corrected date
117123
118124 refs atomic.Int64
119- unmaps [][]byte
125+ closed atomic.Bool
126+ unmaps []func() error
120127 }
121128
122-// graphMmap maps a file read-only. The returned slice must be released with
123-// syscall.Munmap.
124-func graphMmap(path string) ([]byte, error) {
129+// graphOpenRetries is how many times openCommitGraph starts over when a
130+// layer disappears under it, after the first attempt.
131+const graphOpenRetries = 3
132+
133+// graphBeforeLayerOpen, when set by a test, is called with the path of each
134+// chain layer right before it is opened.
135+var graphBeforeLayerOpen func(path string)
136+
137+// graphMmap maps a file read-only. The returned function releases the
138+// mapping.
139+func graphMmap(path string) ([]byte, func() error, error) {
125140 f, err := os.Open(path)
126141 if err != nil {
127- return nil, err
142+ return nil, nil, err
128143 }
129144 defer func() { _ = f.Close() }()
130145 st, err := f.Stat()
131146 if err != nil {
132- return nil, err
147+ return nil, nil, err
133148 }
134149 if !st.Mode().IsRegular() {
135- return nil, graphCorruptf("%s: not a regular file", filepath.Base(path))
150+ return nil, nil, graphCorruptf("%s: not a regular file", filepath.Base(path))
136151 }
137152 size := st.Size()
138153 if size < graphHeaderSize+graphChunkEntrySize+graphHashSize {
139- return nil, graphCorruptf("%s: file too short (%d bytes)", filepath.Base(path), size)
154+ return nil, nil, graphCorruptf("%s: file too short (%d bytes)", filepath.Base(path), size)
140155 }
141156 if size > math.MaxInt32 {
142- return nil, graphCorruptf("%s: file too large (%d bytes)", filepath.Base(path), size)
157+ return nil, nil, graphCorruptf("%s: file too large (%d bytes)", filepath.Base(path), size)
143158 }
144- data, err := syscall.Mmap(int(f.Fd()), 0, int(size), syscall.PROT_READ, syscall.MAP_SHARED)
159+ data, unmap, err := mapFile(f, size)
145160 if err != nil {
146- return nil, fmt.Errorf("mmap %s: %w", filepath.Base(path), err)
161+ return nil, nil, fmt.Errorf("map %s: %w", filepath.Base(path), err)
147162 }
148- _ = syscall.Madvise(data, syscall.MADV_RANDOM)
149- return data, nil
163+ return data, unmap, nil
150164 }
151165
152166 // openCommitGraph opens the commit-graph of the objects directory objectsDir:
153167 // objects/info/commit-graph if present, else the split chain. It returns
154168 // errNoGraph when there is none. The result holds one reference.
169+//
170+// A concurrent "git commit-graph write --split" can merge layers and delete
171+// the old ones between the read of the chain and the open of a layer. The
172+// chain is then read again, up to graphOpenRetries more times; if layers keep
173+// disappearing the error wraps errNoGraph and the cause, so that callers fall
174+// back to reading objects.
155175 func openCommitGraph(objectsDir string) (*commitGraph, error) {
176+ var gone error
177+ for attempt := 0; attempt <= graphOpenRetries; attempt++ {
178+ g, err := openCommitGraphOnce(objectsDir)
179+ if err == nil || !errors.Is(err, fs.ErrNotExist) {
180+ return g, err
181+ }
182+ gone = err
183+ }
184+ return nil, fmt.Errorf("%w: layers kept disappearing: %w", errNoGraph, gone)
185+}
186+
187+func openCommitGraphOnce(objectsDir string) (*commitGraph, error) {
156188 infoDir := filepath.Join(objectsDir, "info")
157- data, err := graphMmap(filepath.Join(infoDir, "commit-graph"))
189+ data, unmap, err := graphMmap(filepath.Join(infoDir, "commit-graph"))
158190 switch {
159191 case err == nil:
160192 l, perr := parseGraphLayer(data)
161193 if perr != nil {
162- _ = syscall.Munmap(data)
194+ _ = unmap()
163195 return nil, fmt.Errorf("commit-graph: %w", perr)
164196 }
165197 g, aerr := assembleGraph([]*graphLayer{l}, nil)
166198 if aerr != nil {
167- _ = syscall.Munmap(data)
199+ _ = unmap()
168200 return nil, fmt.Errorf("commit-graph: %w", aerr)
169201 }
170- g.unmaps = [][]byte{data}
202+ g.unmaps = []func() error{unmap}
171203 return g, nil
172204 case errors.Is(err, fs.ErrNotExist):
173205 return openCommitGraphChain(infoDir)
@@ -183,20 +215,24 @@ func openCommitGraphChain(infoDir string) (*commitGraph, error) {
183215 return nil, err
184216 }
185217 layers := make([]*graphLayer, 0, len(names))
186- maps := make([][]byte, 0, len(names))
218+ unmaps := make([]func() error, 0, len(names))
187219 cleanup := func() {
188- for _, m := range maps {
189- _ = syscall.Munmap(m)
220+ for _, u := range unmaps {
221+ _ = u()
190222 }
191223 }
192224 for _, name := range names {
193225 file := "graph-" + hex.EncodeToString(name[:]) + ".graph"
194- data, err := graphMmap(filepath.Join(dir, file))
226+ path := filepath.Join(dir, file)
227+ if graphBeforeLayerOpen != nil {
228+ graphBeforeLayerOpen(path)
229+ }
230+ data, unmap, err := graphMmap(path)
195231 if err != nil {
196232 cleanup()
197233 return nil, fmt.Errorf("commit-graph chain layer: %w", err)
198234 }
199- maps = append(maps, data)
235+ unmaps = append(unmaps, unmap)
200236 l, err := parseGraphLayer(data)
201237 if err != nil {
202238 cleanup()
@@ -209,7 +245,7 @@ func openCommitGraphChain(infoDir string) (*commitGraph, error) {
209245 cleanup()
210246 return nil, fmt.Errorf("commit-graph chain: %w", err)
211247 }
212- g.unmaps = maps
248+ g.unmaps = unmaps
213249 return g, nil
214250 }
215251
@@ -367,8 +403,11 @@ func (l *graphLayer) validate() error {
367403 if len(l.edge)%4 != 0 {
368404 return graphCorruptf("EDGE chunk size %d not a multiple of 4", len(l.edge))
369405 }
406+ // Optional chunks of the wrong size disable their feature in this layer,
407+ // as in git, instead of failing the layer. Without GDA2 in every layer
408+ // generation() falls back to topological levels (see assembleGraph).
370409 if l.gda2 != nil && uint64(len(l.gda2)) != n*4 {
371- return graphCorruptf("GDA2 chunk has size %d for %d commits", len(l.gda2), n)
410+ l.gda2 = nil
372411 }
373412 if len(l.gdo2)%8 != 0 {
374413 return graphCorruptf("GDO2 chunk size %d not a multiple of 8", len(l.gdo2))
@@ -379,14 +418,9 @@ func (l *graphLayer) validate() error {
379418 if l.numBase == 0 && len(l.baseList) != 0 {
380419 return graphCorruptf("BASE chunk present without base graphs")
381420 }
382- // Bloom filters need both chunks; with only one of them git ignores it.
383- if l.bidx != nil && l.bdat != nil {
384- if uint64(len(l.bidx)) != n*4 {
385- return graphCorruptf("BIDX chunk has size %d for %d commits", len(l.bidx), n)
386- }
387- if len(l.bdat) < bloomHeaderSize {
388- return graphCorruptf("BDAT chunk too short (%d bytes)", len(l.bdat))
389- }
421+ // Bloom filters need both chunks, correctly sized; otherwise git ignores
422+ // them (the layer has no changed-path data).
423+ if l.bidx != nil && l.bdat != nil && uint64(len(l.bidx)) == n*4 && len(l.bdat) >= bloomHeaderSize {
390424 l.bloomVersion = binary.BigEndian.Uint32(l.bdat)
391425 l.bloomHashes = binary.BigEndian.Uint32(l.bdat[4:])
392426 l.bloomBits = binary.BigEndian.Uint32(l.bdat[8:])
@@ -410,6 +444,9 @@ func assembleGraph(layers []*graphLayer, names [][graphHashSize]byte) (*commitGr
410444 if names != nil && len(names) != len(layers) {
411445 return nil, graphCorruptf("%d layers for %d chain entries", len(layers), len(names))
412446 }
447+ if names == nil && len(layers) > 1 {
448+ return nil, graphCorruptf("%d layers without chain entries", len(layers))
449+ }
413450 g := &commitGraph{layers: make([]graphLayer, len(layers)), genV2: true}
414451 var total uint64
415452 for k, l := range layers {
@@ -421,6 +458,9 @@ func assembleGraph(layers []*graphLayer, names [][graphHashSize]byte) (*commitGr
421458 return nil, graphCorruptf("layer %d: base graph %d does not match the chain", k, i)
422459 }
423460 }
461+ if names != nil && !bytes.Equal(l.data[len(l.data)-graphHashSize:], names[k][:]) {
462+ return nil, graphCorruptf("layer %d: checksum does not match its name in the chain", k)
463+ }
424464 l.base = uint32(total)
425465 total += uint64(l.n)
426466 if total >= graphParentNone {
@@ -436,8 +476,12 @@ func assembleGraph(layers []*graphLayer, names [][graphHashSize]byte) (*commitGr
436476 return g, nil
437477 }
438478
439-// retain adds a reference. It reports false when the graph is already closed.
479+// retain adds a reference. It reports false once the graph is closed, even
480+// if readers still hold references.
440481 func (g *commitGraph) retain() bool {
482+ if g.closed.Load() {
483+ return false
484+ }
441485 for {
442486 n := g.refs.Load()
443487 if n <= 0 {
@@ -456,8 +500,8 @@ func (g *commitGraph) release() error {
456500 return nil
457501 case n == 0:
458502 var first error
459- for _, m := range g.unmaps {
460- if err := syscall.Munmap(m); err != nil && first == nil {
503+ for _, unmap := range g.unmaps {
504+ if err := unmap(); err != nil && first == nil {
461505 first = err
462506 }
463507 }
@@ -469,8 +513,14 @@ func (g *commitGraph) release() error {
469513 }
470514 }
471515
472-// close drops the owner's reference.
473-func (g *commitGraph) close() error { return g.release() }
516+// close drops the owner's reference. Only the first call has an effect, so
517+// calling it again never drops a reference held by a reader.
518+func (g *commitGraph) close() error {
519+ if !g.closed.CompareAndSwap(false, true) {
520+ return nil
521+ }
522+ return g.release()
523+}
474524
475525 // numCommits is the number of commits in the whole chain.
476526 func (g *commitGraph) numCommits() uint32 { return g.total }
@@ -590,14 +640,15 @@ func (g *commitGraph) topoLevel(pos uint32) (uint32, error) {
590640 // hasGenerationV2, else its topological level. In both cases a commit has a
591641 // strictly greater generation than each of its parents.
592642 func (g *commitGraph) generation(pos uint32) (uint64, error) {
593- l, rec, err := g.record(pos)
594- if err != nil {
595- return 0, err
643+ l, idx := g.layerOf(pos)
644+ if l == nil {
645+ return 0, errGraphPosition
596646 }
647+ off := int(idx) * graphCommitSize
648+ rec := l.cdat[off : off+graphCommitSize]
597649 if !g.genV2 {
598650 return uint64(binary.BigEndian.Uint32(rec[graphHashSize+8:]) >> 2), nil
599651 }
600- _, idx := g.layerOf(pos)
601652 v := binary.BigEndian.Uint32(l.gda2[int(idx)*4:])
602653 offset := uint64(v)
603654 if v&graphGenOverflow != 0 {
@@ -681,9 +732,7 @@ func statGraphFile(path string) graphFileStamp {
681732 return graphFileStamp{}
682733 }
683734 s := graphFileStamp{exists: true, size: st.Size(), mtime: st.ModTime().UnixNano()}
684- if sys, ok := st.Sys().(*syscall.Stat_t); ok {
685- s.ino = uint64(sys.Ino) //nolint:unconvert // width differs across platforms
686- }
735+ s.ino = fileIno(st)
687736 return s
688737 }
689738
@@ -14,7 +14,6 @@ import (
14 "os"14 "os"
15 "path/filepath"15 "path/filepath"
16 "sync/atomic"16 "sync/atomic"
17- "syscall"
18 )17 )
19 18
20 // This file reads Git commit-graph files (gitformat-commit-graph): a single19 // This file reads Git commit-graph files (gitformat-commit-graph): a single
@@ -25,11 +24,18 @@ import (
25 // Lifecycle. openCommitGraph returns a graph holding one reference (the24 // Lifecycle. openCommitGraph returns a graph holding one reference (the
26 // owner's). A reader that may outlive a concurrent reopen calls retain25 // owner's). A reader that may outlive a concurrent reopen calls retain
27 // (which fails once the graph is closed) and release when done. The owner26 // (which fails once the graph is closed) and release when done. The owner
28-// calls close when it replaces the graph. The mappings are unmapped when the27+// calls close when it replaces the graph; close is idempotent, only the first
29-// last reference is dropped; no accessor may be used after that. Layers are28+// call drops the owner's reference. The mappings are unmapped when the last
29+// reference is dropped; no accessor may be used after that. Layers are
30 // immutable files named after their checksum, so a concurrent30 // immutable files named after their checksum, so a concurrent
31 // "git commit-graph write" never changes a mapped file; it can only delete31 // "git commit-graph write" never changes a mapped file; it can only delete
32-// it, which does not affect an existing mapping.32+// it, which does not affect an existing mapping. It can however delete a
33+// layer between the moment the chain is read and the moment the layer is
34+// opened: openCommitGraph then starts again from the chain, and gives up with
35+// errNoGraph (callers fall back to the objects) if that keeps happening.
36+//
37+// File mapping is done by mapFile (mmap_*.go), which falls back to reading
38+// the file where mmap is unavailable.
33 //39 //
34 // Positions are global across the chain: the commits of the base layer come40 // Positions are global across the chain: the commits of the base layer come
35 // first, then those of the next layer, and so on. Within a layer commits are41 // first, then those of the next layer, and so on. Within a layer commits are
@@ -116,58 +122,84 @@ type commitGraph struct {
116 genV2 bool // every layer has GDA2: generation() is a corrected date122 genV2 bool // every layer has GDA2: generation() is a corrected date
117 123
118 refs atomic.Int64124 refs atomic.Int64
119- unmaps [][]byte125+ closed atomic.Bool
126+ unmaps []func() error
120 }127 }
121 128
122-// graphMmap maps a file read-only. The returned slice must be released with129+// graphOpenRetries is how many times openCommitGraph starts over when a
123-// syscall.Munmap.130+// layer disappears under it, after the first attempt.
124-func graphMmap(path string) ([]byte, error) {131+const graphOpenRetries = 3
132+
133+// graphBeforeLayerOpen, when set by a test, is called with the path of each
134+// chain layer right before it is opened.
135+var graphBeforeLayerOpen func(path string)
136+
137+// graphMmap maps a file read-only. The returned function releases the
138+// mapping.
139+func graphMmap(path string) ([]byte, func() error, error) {
125 f, err := os.Open(path)140 f, err := os.Open(path)
126 if err != nil {141 if err != nil {
127- return nil, err142+ return nil, nil, err
128 }143 }
129 defer func() { _ = f.Close() }()144 defer func() { _ = f.Close() }()
130 st, err := f.Stat()145 st, err := f.Stat()
131 if err != nil {146 if err != nil {
132- return nil, err147+ return nil, nil, err
133 }148 }
134 if !st.Mode().IsRegular() {149 if !st.Mode().IsRegular() {
135- return nil, graphCorruptf("%s: not a regular file", filepath.Base(path))150+ return nil, nil, graphCorruptf("%s: not a regular file", filepath.Base(path))
136 }151 }
137 size := st.Size()152 size := st.Size()
138 if size < graphHeaderSize+graphChunkEntrySize+graphHashSize {153 if size < graphHeaderSize+graphChunkEntrySize+graphHashSize {
139- return nil, graphCorruptf("%s: file too short (%d bytes)", filepath.Base(path), size)154+ return nil, nil, graphCorruptf("%s: file too short (%d bytes)", filepath.Base(path), size)
140 }155 }
141 if size > math.MaxInt32 {156 if size > math.MaxInt32 {
142- return nil, graphCorruptf("%s: file too large (%d bytes)", filepath.Base(path), size)157+ return nil, nil, graphCorruptf("%s: file too large (%d bytes)", filepath.Base(path), size)
143 }158 }
144- data, err := syscall.Mmap(int(f.Fd()), 0, int(size), syscall.PROT_READ, syscall.MAP_SHARED)159+ data, unmap, err := mapFile(f, size)
145 if err != nil {160 if err != nil {
146- return nil, fmt.Errorf("mmap %s: %w", filepath.Base(path), err)161+ return nil, nil, fmt.Errorf("map %s: %w", filepath.Base(path), err)
147 }162 }
148- _ = syscall.Madvise(data, syscall.MADV_RANDOM)163+ return data, unmap, nil
149- return data, nil
150 }164 }
151 165
152 // openCommitGraph opens the commit-graph of the objects directory objectsDir:166 // openCommitGraph opens the commit-graph of the objects directory objectsDir:
153 // objects/info/commit-graph if present, else the split chain. It returns167 // objects/info/commit-graph if present, else the split chain. It returns
154 // errNoGraph when there is none. The result holds one reference.168 // errNoGraph when there is none. The result holds one reference.
169+//
170+// A concurrent "git commit-graph write --split" can merge layers and delete
171+// the old ones between the read of the chain and the open of a layer. The
172+// chain is then read again, up to graphOpenRetries more times; if layers keep
173+// disappearing the error wraps errNoGraph and the cause, so that callers fall
174+// back to reading objects.
155 func openCommitGraph(objectsDir string) (*commitGraph, error) {175 func openCommitGraph(objectsDir string) (*commitGraph, error) {
176+ var gone error
177+ for attempt := 0; attempt <= graphOpenRetries; attempt++ {
178+ g, err := openCommitGraphOnce(objectsDir)
179+ if err == nil || !errors.Is(err, fs.ErrNotExist) {
180+ return g, err
181+ }
182+ gone = err
183+ }
184+ return nil, fmt.Errorf("%w: layers kept disappearing: %w", errNoGraph, gone)
185+}
186+
187+func openCommitGraphOnce(objectsDir string) (*commitGraph, error) {
156 infoDir := filepath.Join(objectsDir, "info")188 infoDir := filepath.Join(objectsDir, "info")
157- data, err := graphMmap(filepath.Join(infoDir, "commit-graph"))189+ data, unmap, err := graphMmap(filepath.Join(infoDir, "commit-graph"))
158 switch {190 switch {
159 case err == nil:191 case err == nil:
160 l, perr := parseGraphLayer(data)192 l, perr := parseGraphLayer(data)
161 if perr != nil {193 if perr != nil {
162- _ = syscall.Munmap(data)194+ _ = unmap()
163 return nil, fmt.Errorf("commit-graph: %w", perr)195 return nil, fmt.Errorf("commit-graph: %w", perr)
164 }196 }
165 g, aerr := assembleGraph([]*graphLayer{l}, nil)197 g, aerr := assembleGraph([]*graphLayer{l}, nil)
166 if aerr != nil {198 if aerr != nil {
167- _ = syscall.Munmap(data)199+ _ = unmap()
168 return nil, fmt.Errorf("commit-graph: %w", aerr)200 return nil, fmt.Errorf("commit-graph: %w", aerr)
169 }201 }
170- g.unmaps = [][]byte{data}202+ g.unmaps = []func() error{unmap}
171 return g, nil203 return g, nil
172 case errors.Is(err, fs.ErrNotExist):204 case errors.Is(err, fs.ErrNotExist):
173 return openCommitGraphChain(infoDir)205 return openCommitGraphChain(infoDir)
@@ -183,20 +215,24 @@ func openCommitGraphChain(infoDir string) (*commitGraph, error) {
183 return nil, err215 return nil, err
184 }216 }
185 layers := make([]*graphLayer, 0, len(names))217 layers := make([]*graphLayer, 0, len(names))
186- maps := make([][]byte, 0, len(names))218+ unmaps := make([]func() error, 0, len(names))
187 cleanup := func() {219 cleanup := func() {
188- for _, m := range maps {220+ for _, u := range unmaps {
189- _ = syscall.Munmap(m)221+ _ = u()
190 }222 }
191 }223 }
192 for _, name := range names {224 for _, name := range names {
193 file := "graph-" + hex.EncodeToString(name[:]) + ".graph"225 file := "graph-" + hex.EncodeToString(name[:]) + ".graph"
194- data, err := graphMmap(filepath.Join(dir, file))226+ path := filepath.Join(dir, file)
227+ if graphBeforeLayerOpen != nil {
228+ graphBeforeLayerOpen(path)
229+ }
230+ data, unmap, err := graphMmap(path)
195 if err != nil {231 if err != nil {
196 cleanup()232 cleanup()
197 return nil, fmt.Errorf("commit-graph chain layer: %w", err)233 return nil, fmt.Errorf("commit-graph chain layer: %w", err)
198 }234 }
199- maps = append(maps, data)235+ unmaps = append(unmaps, unmap)
200 l, err := parseGraphLayer(data)236 l, err := parseGraphLayer(data)
201 if err != nil {237 if err != nil {
202 cleanup()238 cleanup()
@@ -209,7 +245,7 @@ func openCommitGraphChain(infoDir string) (*commitGraph, error) {
209 cleanup()245 cleanup()
210 return nil, fmt.Errorf("commit-graph chain: %w", err)246 return nil, fmt.Errorf("commit-graph chain: %w", err)
211 }247 }
212- g.unmaps = maps248+ g.unmaps = unmaps
213 return g, nil249 return g, nil
214 }250 }
215 251
@@ -367,8 +403,11 @@ func (l *graphLayer) validate() error {
367 if len(l.edge)%4 != 0 {403 if len(l.edge)%4 != 0 {
368 return graphCorruptf("EDGE chunk size %d not a multiple of 4", len(l.edge))404 return graphCorruptf("EDGE chunk size %d not a multiple of 4", len(l.edge))
369 }405 }
406+ // Optional chunks of the wrong size disable their feature in this layer,
407+ // as in git, instead of failing the layer. Without GDA2 in every layer
408+ // generation() falls back to topological levels (see assembleGraph).
370 if l.gda2 != nil && uint64(len(l.gda2)) != n*4 {409 if l.gda2 != nil && uint64(len(l.gda2)) != n*4 {
371- return graphCorruptf("GDA2 chunk has size %d for %d commits", len(l.gda2), n)410+ l.gda2 = nil
372 }411 }
373 if len(l.gdo2)%8 != 0 {412 if len(l.gdo2)%8 != 0 {
374 return graphCorruptf("GDO2 chunk size %d not a multiple of 8", len(l.gdo2))413 return graphCorruptf("GDO2 chunk size %d not a multiple of 8", len(l.gdo2))
@@ -379,14 +418,9 @@ func (l *graphLayer) validate() error {
379 if l.numBase == 0 && len(l.baseList) != 0 {418 if l.numBase == 0 && len(l.baseList) != 0 {
380 return graphCorruptf("BASE chunk present without base graphs")419 return graphCorruptf("BASE chunk present without base graphs")
381 }420 }
382- // Bloom filters need both chunks; with only one of them git ignores it.421+ // Bloom filters need both chunks, correctly sized; otherwise git ignores
383- if l.bidx != nil && l.bdat != nil {422+ // them (the layer has no changed-path data).
384- if uint64(len(l.bidx)) != n*4 {423+ if l.bidx != nil && l.bdat != nil && uint64(len(l.bidx)) == n*4 && len(l.bdat) >= bloomHeaderSize {
385- return graphCorruptf("BIDX chunk has size %d for %d commits", len(l.bidx), n)
386- }
387- if len(l.bdat) < bloomHeaderSize {
388- return graphCorruptf("BDAT chunk too short (%d bytes)", len(l.bdat))
389- }
390 l.bloomVersion = binary.BigEndian.Uint32(l.bdat)424 l.bloomVersion = binary.BigEndian.Uint32(l.bdat)
391 l.bloomHashes = binary.BigEndian.Uint32(l.bdat[4:])425 l.bloomHashes = binary.BigEndian.Uint32(l.bdat[4:])
392 l.bloomBits = binary.BigEndian.Uint32(l.bdat[8:])426 l.bloomBits = binary.BigEndian.Uint32(l.bdat[8:])
@@ -410,6 +444,9 @@ func assembleGraph(layers []*graphLayer, names [][graphHashSize]byte) (*commitGr
410 if names != nil && len(names) != len(layers) {444 if names != nil && len(names) != len(layers) {
411 return nil, graphCorruptf("%d layers for %d chain entries", len(layers), len(names))445 return nil, graphCorruptf("%d layers for %d chain entries", len(layers), len(names))
412 }446 }
447+ if names == nil && len(layers) > 1 {
448+ return nil, graphCorruptf("%d layers without chain entries", len(layers))
449+ }
413 g := &commitGraph{layers: make([]graphLayer, len(layers)), genV2: true}450 g := &commitGraph{layers: make([]graphLayer, len(layers)), genV2: true}
414 var total uint64451 var total uint64
415 for k, l := range layers {452 for k, l := range layers {
@@ -421,6 +458,9 @@ func assembleGraph(layers []*graphLayer, names [][graphHashSize]byte) (*commitGr
421 return nil, graphCorruptf("layer %d: base graph %d does not match the chain", k, i)458 return nil, graphCorruptf("layer %d: base graph %d does not match the chain", k, i)
422 }459 }
423 }460 }
461+ if names != nil && !bytes.Equal(l.data[len(l.data)-graphHashSize:], names[k][:]) {
462+ return nil, graphCorruptf("layer %d: checksum does not match its name in the chain", k)
463+ }
424 l.base = uint32(total)464 l.base = uint32(total)
425 total += uint64(l.n)465 total += uint64(l.n)
426 if total >= graphParentNone {466 if total >= graphParentNone {
@@ -436,8 +476,12 @@ func assembleGraph(layers []*graphLayer, names [][graphHashSize]byte) (*commitGr
436 return g, nil476 return g, nil
437 }477 }
438 478
439-// retain adds a reference. It reports false when the graph is already closed.479+// retain adds a reference. It reports false once the graph is closed, even
480+// if readers still hold references.
440 func (g *commitGraph) retain() bool {481 func (g *commitGraph) retain() bool {
482+ if g.closed.Load() {
483+ return false
484+ }
441 for {485 for {
442 n := g.refs.Load()486 n := g.refs.Load()
443 if n <= 0 {487 if n <= 0 {
@@ -456,8 +500,8 @@ func (g *commitGraph) release() error {
456 return nil500 return nil
457 case n == 0:501 case n == 0:
458 var first error502 var first error
459- for _, m := range g.unmaps {503+ for _, unmap := range g.unmaps {
460- if err := syscall.Munmap(m); err != nil && first == nil {504+ if err := unmap(); err != nil && first == nil {
461 first = err505 first = err
462 }506 }
463 }507 }
@@ -469,8 +513,14 @@ func (g *commitGraph) release() error {
469 }513 }
470 }514 }
471 515
472-// close drops the owner's reference.516+// close drops the owner's reference. Only the first call has an effect, so
473-func (g *commitGraph) close() error { return g.release() }517+// calling it again never drops a reference held by a reader.
518+func (g *commitGraph) close() error {
519+ if !g.closed.CompareAndSwap(false, true) {
520+ return nil
521+ }
522+ return g.release()
523+}
474 524
475 // numCommits is the number of commits in the whole chain.525 // numCommits is the number of commits in the whole chain.
476 func (g *commitGraph) numCommits() uint32 { return g.total }526 func (g *commitGraph) numCommits() uint32 { return g.total }
@@ -590,14 +640,15 @@ func (g *commitGraph) topoLevel(pos uint32) (uint32, error) {
590 // hasGenerationV2, else its topological level. In both cases a commit has a640 // hasGenerationV2, else its topological level. In both cases a commit has a
591 // strictly greater generation than each of its parents.641 // strictly greater generation than each of its parents.
592 func (g *commitGraph) generation(pos uint32) (uint64, error) {642 func (g *commitGraph) generation(pos uint32) (uint64, error) {
593- l, rec, err := g.record(pos)643+ l, idx := g.layerOf(pos)
594- if err != nil {644+ if l == nil {
595- return 0, err645+ return 0, errGraphPosition
596 }646 }
647+ off := int(idx) * graphCommitSize
648+ rec := l.cdat[off : off+graphCommitSize]
597 if !g.genV2 {649 if !g.genV2 {
598 return uint64(binary.BigEndian.Uint32(rec[graphHashSize+8:]) >> 2), nil650 return uint64(binary.BigEndian.Uint32(rec[graphHashSize+8:]) >> 2), nil
599 }651 }
600- _, idx := g.layerOf(pos)
601 v := binary.BigEndian.Uint32(l.gda2[int(idx)*4:])652 v := binary.BigEndian.Uint32(l.gda2[int(idx)*4:])
602 offset := uint64(v)653 offset := uint64(v)
603 if v&graphGenOverflow != 0 {654 if v&graphGenOverflow != 0 {
@@ -681,9 +732,7 @@ func statGraphFile(path string) graphFileStamp {
681 return graphFileStamp{}732 return graphFileStamp{}
682 }733 }
683 s := graphFileStamp{exists: true, size: st.Size(), mtime: st.ModTime().UnixNano()}734 s := graphFileStamp{exists: true, size: st.Size(), mtime: st.ModTime().UnixNano()}
684- if sys, ok := st.Sys().(*syscall.Stat_t); ok {735+ s.ino = fileIno(st)
685- s.ino = uint64(sys.Ino) //nolint:unconvert // width differs across platforms
686- }
687 return s736 return s
688 }737 }
689 738
M graph_test.go +496 -18
@@ -9,13 +9,17 @@ import (
99 "encoding/hex"
1010 "errors"
1111 "fmt"
12+ "io/fs"
1213 "os"
1314 "os/exec"
1415 "path/filepath"
1516 "sort"
1617 "strconv"
1718 "strings"
19+ "sync"
20+ "sync/atomic"
1821 "testing"
22+ "time"
1923 )
2024
2125 // oid is the raw object id used by the graph code until the core's ObjectID
@@ -505,6 +509,132 @@ func TestGraphRefcount(t *testing.T) {
505509 }
506510 }
507511
512+// close is idempotent: a second call must not drop a reader's reference.
513+func TestGraphCloseIdempotent(t *testing.T) {
514+ r := newGTRepo(t)
515+ r.commit("a", "a\n")
516+ r.git("commit-graph", "write", "--reachable")
517+ g, err := openCommitGraph(r.objects())
518+ if err != nil {
519+ t.Fatal(err)
520+ }
521+ if !g.retain() {
522+ t.Fatal("retain failed on open graph")
523+ }
524+ var unmapped atomic.Int32
525+ for i, u := range g.unmaps {
526+ g.unmaps[i] = func() error { unmapped.Add(1); return u() }
527+ }
528+ for range 3 {
529+ if err := g.close(); err != nil {
530+ t.Fatal(err)
531+ }
532+ }
533+ if unmapped.Load() != 0 {
534+ t.Fatal("unmapped while a reader holds a reference")
535+ }
536+ if g.retain() {
537+ t.Fatal("retain succeeded after close")
538+ }
539+ if _, err := g.commitID(0); err != nil {
540+ t.Fatal(err)
541+ }
542+ if err := g.release(); err != nil {
543+ t.Fatal(err)
544+ }
545+ if unmapped.Load() != int32(len(g.layers)) {
546+ t.Fatalf("unmapped %d files, want %d", unmapped.Load(), len(g.layers))
547+ }
548+ if err := g.close(); err != nil {
549+ t.Fatal(err)
550+ }
551+ if unmapped.Load() != int32(len(g.layers)) {
552+ t.Fatal("close after the last release unmapped again")
553+ }
554+}
555+
556+// Readers retain, read and release while the owner closes (more than once).
557+// The files must never be unmapped while a reader holds a reference; a read
558+// of an unmapped file would also crash the test.
559+func TestGraphConcurrentCloseRetain(t *testing.T) {
560+ r := newGTRepo(t)
561+ for i := range 5 {
562+ r.commit(fmt.Sprintf("f%d", i), "x\n")
563+ r.git("commit-graph", "write", "--reachable", "--split=no-merge", "--changed-paths")
564+ }
565+ const readers = 16
566+ rounds := 20
567+ if testing.Short() {
568+ rounds = 3
569+ }
570+ for range rounds {
571+ g, err := openCommitGraph(r.objects())
572+ if err != nil {
573+ t.Fatal(err)
574+ }
575+ var active, unmapped atomic.Int32
576+ for i, u := range g.unmaps {
577+ g.unmaps[i] = func() error {
578+ if active.Load() != 0 {
579+ t.Error("unmapped while referenced")
580+ }
581+ unmapped.Store(1)
582+ return u()
583+ }
584+ }
585+ var wg sync.WaitGroup
586+ start := make(chan struct{})
587+ var retained atomic.Int64
588+ for range readers {
589+ wg.Go(func() {
590+ <-start
591+ for range 200 {
592+ if !g.retain() {
593+ return
594+ }
595+ retained.Add(1)
596+ active.Add(1)
597+ if unmapped.Load() != 0 {
598+ t.Error("reference granted on unmapped graph")
599+ }
600+ for pos := range g.numCommits() {
601+ id, err := g.commitID(pos)
602+ if err != nil {
603+ t.Error(err)
604+ }
605+ if got, ok := g.lookup(id); !ok || got != pos {
606+ t.Errorf("lookup(%d) = %d, %v", pos, got, ok)
607+ }
608+ if _, err := g.appendParents(nil, pos); err != nil {
609+ t.Error(err)
610+ }
611+ g.maybeChanged(pos, []byte("f1"))
612+ }
613+ active.Add(-1)
614+ if err := g.release(); err != nil {
615+ t.Error(err)
616+ }
617+ }
618+ })
619+ }
620+ close(start)
621+ for range 3 {
622+ wg.Go(func() {
623+ if err := g.close(); err != nil {
624+ t.Error(err)
625+ }
626+ })
627+ }
628+ wg.Wait()
629+ if unmapped.Load() != 1 {
630+ t.Fatal("files never unmapped")
631+ }
632+ if g.retain() {
633+ t.Fatal("retain succeeded after close")
634+ }
635+ }
636+}
637+
508638 // A layer removed or rewritten while a graph is open must not affect it.
509639 func TestGraphSurvivesRewrite(t *testing.T) {
510640 r := newGTRepo(t)
@@ -532,6 +662,124 @@ func TestGraphSurvivesRewrite(t *testing.T) {
532662 }
533663 }
534664
665+// setLayerHook installs graphBeforeLayerOpen for the duration of the test.
666+func setLayerHook(t *testing.T, f func(path string)) {
667+ t.Helper()
668+ graphBeforeLayerOpen = f
669+ t.Cleanup(func() { graphBeforeLayerOpen = nil })
670+}
671+
672+// splitChainRepo has a graph chain of several layers.
673+func splitChainRepo(t *testing.T) *gtRepo {
674+ r := newGTRepo(t)
675+ for i := range 3 {
676+ r.commit(fmt.Sprintf("f%d", i), "x\n")
677+ r.git("commit-graph", "write", "--reachable", "--split=no-merge", "--changed-paths")
678+ }
679+ return r
680+}
681+
682+// A concurrent merging "commit-graph write --split" deletes the layers named
683+// by the chain just read: opening starts again and succeeds.
684+func TestGraphOpenRetriesVanishedLayer(t *testing.T) {
685+ r := splitChainRepo(t)
686+ calls := 0
687+ setLayerHook(t, func(string) {
688+ if calls++; calls == 1 {
689+ r.git("commit-graph", "write", "--reachable", "--split=replace", "--changed-paths", "--expire-time=now")
690+ }
691+ })
692+ g, err := openCommitGraph(r.objects())
693+ if err != nil {
694+ t.Fatal(err)
695+ }
696+ defer func() { _ = g.close() }()
697+ if g.numLayers() != 1 {
698+ t.Fatalf("layers = %d, want the replaced chain of 1", g.numLayers())
699+ }
700+ checkGraphAgainstGit(t, r.dir, g, false)
701+}
702+
703+// Layers that keep vanishing end in errNoGraph, which callers treat as
704+// "read the objects instead", wrapping the cause.
705+func TestGraphOpenGivesUpOnVanishedLayer(t *testing.T) {
706+ r := splitChainRepo(t)
707+ calls := 0
708+ setLayerHook(t, func(path string) {
709+ calls++
710+ _ = os.Remove(path)
711+ })
712+ g, err := openCommitGraph(r.objects())
713+ if err == nil {
714+ _ = g.close()
715+ t.Fatal("opened a graph with a missing layer")
716+ }
717+ if !errors.Is(err, errNoGraph) || !errors.Is(err, fs.ErrNotExist) {
718+ t.Fatalf("err = %v, want errNoGraph wrapping ErrNotExist", err)
719+ }
720+ if calls != 1+graphOpenRetries {
721+ t.Fatalf("%d attempts, want %d", calls, 1+graphOpenRetries)
722+ }
723+}
724+
725+// Opening in a loop while git rewrites the chain: success or errNoGraph only.
726+func TestGraphOpenStressSplitWrite(t *testing.T) {
727+ r := newGTRepo(t)
728+ r.commit("a", "a\n")
729+ r.git("commit-graph", "write", "--reachable", "--split", "--changed-paths")
730+ commits := 150
731+ if testing.Short() {
732+ commits = 10
733+ }
734+ var done atomic.Bool
735+ writerErr := make(chan error, 1)
736+ go func() {
737+ defer done.Store(true)
738+ for i := range commits {
739+ for _, args := range [][]string{
740+ {"commit", "-q", "--allow-empty", "-m", fmt.Sprint("s", i)},
741+ {"commit-graph", "write", "--reachable", "--split", "--changed-paths", "--expire-time=now"},
742+ } {
743+ cmd := exec.Command("git", args...)
744+ cmd.Dir = r.dir
745+ cmd.Env = graphGitEnv(0)
746+ if out, err := cmd.CombinedOutput(); err != nil {
747+ writerErr <- fmt.Errorf("git %v: %v\n%s", args, err, out)
748+ return
749+ }
750+ }
751+ }
752+ writerErr <- nil
753+ }()
754+ // Count the layer opens beyond the chain length: retries.
755+ var layerOpens atomic.Int64
756+ setLayerHook(t, func(string) { layerOpens.Add(1) })
757+ var opened, none int
758+ deadline := time.Now().Add(2 * time.Minute)
759+ for !done.Load() && time.Now().Before(deadline) {
760+ g, err := openCommitGraph(r.objects())
761+ switch {
762+ case err == nil:
763+ opened++
764+ if g.numCommits() == 0 {
765+ t.Error("empty graph")
766+ }
767+ _ = g.close()
768+ case errors.Is(err, errNoGraph):
769+ none++
770+ default:
771+ t.Fatalf("open: %v", err)
772+ }
773+ }
774+ if err := <-writerErr; err != nil {
775+ t.Fatal(err)
776+ }
777+ t.Logf("opened %d, no graph %d, layer opens %d", opened, none, layerOpens.Load())
778+ if opened == 0 {
779+ t.Error("never opened a graph")
780+ }
781+}
782+
535783 func TestGraphStamp(t *testing.T) {
536784 r := newGTRepo(t)
537785 r.commit("a", "a\n")
@@ -939,6 +1187,148 @@ func TestGraphChainMismatch(t *testing.T) {
9391187 }
9401188 }
9411189
1190+// A layer file whose trailer is not the name the chain gives it is rejected.
1191+func TestGraphLayerChecksumMustMatchName(t *testing.T) {
1192+ mk := func(sum byte, base []oid) *graphLayer {
1193+ s := &synthLayer{ids: synthIDs(3), base: base}
1194+ b := s.bytes()
1195+ for i := len(b) - graphHashSize; i < len(b); i++ {
1196+ b[i] = sum
1197+ }
1198+ l, err := parseGraphLayer(b)
1199+ if err != nil {
1200+ t.Fatal(err)
1201+ }
1202+ return l
1203+ }
1204+ name := func(b byte) [graphHashSize]byte { return [graphHashSize]byte(bytes.Repeat([]byte{b}, graphHashSize)) }
1205+ chain := func(a, b byte) (*commitGraph, error) {
1206+ return assembleGraph([]*graphLayer{mk(0xa1, nil), mk(0xb2, []oid{name(0xa1)})},
1207+ [][graphHashSize]byte{name(a), name(b)})
1208+ }
1209+ if _, err := chain(0xa1, 0xb2); err != nil {
1210+ t.Fatalf("matching names: %v", err)
1211+ }
1212+ if _, err := chain(0xa1, 0xb3); !errors.Is(err, errGraphCorrupt) {
1213+ t.Errorf("top layer renamed: err = %v", err)
1214+ }
1215+ // The base list of the top layer names the base correctly, but the base
1216+ // file itself carries another checksum.
1217+ if _, err := assembleGraph([]*graphLayer{mk(0xee, nil), mk(0xb2, []oid{name(0xa1)})},
1218+ [][graphHashSize]byte{name(0xa1), name(0xb2)}); !errors.Is(err, errGraphCorrupt) {
1219+ t.Errorf("base layer with another checksum: err = %v", err)
1220+ }
1221+ // Several layers need chain names.
1222+ if _, err := assembleGraph([]*graphLayer{mk(1, nil), mk(2, []oid{name(1)})}, nil); !errors.Is(err, errGraphCorrupt) {
1223+ t.Errorf("two layers without names: err = %v", err)
1224+ }
1225+}
1226+
1227+// On disk: a layer copied under another name is not accepted as that layer.
1228+func TestGraphRenamedLayerRejected(t *testing.T) {
1229+ r := newGTRepo(t)
1230+ r.commit("a", "a\n")
1231+ r.git("commit-graph", "write", "--reachable", "--split")
1232+ dir := filepath.Join(r.objects(), "info", "commit-graphs")
1233+ chainPath := filepath.Join(dir, "commit-graph-chain")
1234+ raw, err := os.ReadFile(chainPath)
1235+ if err != nil {
1236+ t.Fatal(err)
1237+ }
1238+ good := strings.TrimSpace(string(raw))
1239+ fake := strings.Repeat("0", 40)
1240+ b, err := os.ReadFile(filepath.Join(dir, "graph-"+good+".graph"))
1241+ if err != nil {
1242+ t.Fatal(err)
1243+ }
1244+ if err := os.WriteFile(filepath.Join(dir, "graph-"+fake+".graph"), b, 0o644); err != nil {
1245+ t.Fatal(err)
1246+ }
1247+ if err := os.Chmod(chainPath, 0o644); err != nil {
1248+ t.Fatal(err)
1249+ }
1250+ if err := os.WriteFile(chainPath, []byte(fake+"\n"), 0o644); err != nil {
1251+ t.Fatal(err)
1252+ }
1253+ if g, err := openCommitGraph(r.objects()); !errors.Is(err, errGraphCorrupt) {
1254+ if err == nil {
1255+ _ = g.close()
1256+ }
1257+ t.Fatalf("err = %v, want corrupt", err)
1258+ }
1259+}
1260+
1261+// Optional chunks of the wrong size disable their feature only.
1262+func TestGraphOptionalChunkSizes(t *testing.T) {
1263+ const n = 4
1264+ hdr := binary.BigEndian.AppendUint32(nil, 2)
1265+ hdr = binary.BigEndian.AppendUint32(hdr, 7)
1266+ hdr = binary.BigEndian.AppendUint32(hdr, 10)
1267+ mk := func() *synthLayer {
1268+ s := &synthLayer{ids: synthIDs(n), parents: make([][]uint32, n), times: make([]int64, n), levels: []uint32{5, 6, 7, 8}}
1269+ s.times = []int64{100, 200, 300, 400}
1270+ s.parents[1] = []uint32{0}
1271+ s.gda2 = []uint32{0, 1, 2, 3}
1272+ s.bdat = append(hdr[:12:12], 0x00)
1273+ s.bidx = []uint32{1, 1, 1, 1}
1274+ return s
1275+ }
1276+ t.Run("ok", func(t *testing.T) {
1277+ g := mustParseSynth(t, mk().bytes())
1278+ defer func() { _ = g.close() }()
1279+ if !g.hasGenerationV2() || !g.layers[0].hasBloom {
1280+ t.Fatal("features missing on a good layer")
1281+ }
1282+ if v, _ := g.generation(1); v != 201 {
1283+ t.Fatalf("generation = %d, want 201", v)
1284+ }
1285+ })
1286+ t.Run("GDA2", func(t *testing.T) {
1287+ for _, size := range []int{n - 1, n + 1} {
1288+ s := mk()
1289+ s.gda2 = make([]uint32, size)
1290+ g := mustParseSynth(t, s.bytes())
1291+ if g.hasGenerationV2() {
1292+ t.Errorf("GDA2 of %d entries accepted", size)
1293+ }
1294+ if v, err := g.generation(1); err != nil || v != 6 {
1295+ t.Errorf("generation = %d, %v, want the level 6", v, err)
1296+ }
1297+ if !g.layers[0].hasBloom {
1298+ t.Error("a bad GDA2 disabled the Bloom filters")
1299+ }
1300+ if _, err := g.appendParents(nil, 1); err != nil {
1301+ t.Error(err)
1302+ }
1303+ _ = g.close()
1304+ }
1305+ })
1306+ t.Run("BIDX", func(t *testing.T) {
1307+ s := mk()
1308+ s.bidx = s.bidx[:n-1]
1309+ g := mustParseSynth(t, s.bytes())
1310+ defer func() { _ = g.close() }()
1311+ if g.layers[0].hasBloom || g.hasBloom(0) {
1312+ t.Error("short BIDX accepted")
1313+ }
1314+ if !g.hasGenerationV2() {
1315+ t.Error("a bad BIDX disabled the generation")
1316+ }
1317+ })
1318+ t.Run("BDAT", func(t *testing.T) {
1319+ s := mk()
1320+ s.bdat = s.bdat[:bloomHeaderSize-1]
1321+ g := mustParseSynth(t, s.bytes())
1322+ defer func() { _ = g.close() }()
1323+ if g.layers[0].hasBloom || g.hasBloom(0) {
1324+ t.Error("short BDAT accepted")
1325+ }
1326+ if !g.hasGenerationV2() {
1327+ t.Error("a bad BDAT disabled the generation")
1328+ }
1329+ })
1330+}
1331+
9421332 func TestGraphCorruptFiles(t *testing.T) {
9431333 r := newGTRepo(t)
9441334 r.commit("a", "a\n")
@@ -986,27 +1376,33 @@ func exerciseGraphBytes(data []byte) {
9861376 {layers: []graphLayer{one}, total: one.n, genV2: one.gda2 != nil},
9871377 {layers: []graphLayer{one, two}, total: 2 * one.n, genV2: one.gda2 != nil},
9881378 } {
989- var buf [4]uint32
990- n := min(g.total, 256)
991- for pos := range n {
992- id, _ := g.commitID(pos)
993- g.lookup(id)
994- _, _ = g.rootTree(pos)
995- _, _ = g.commitTime(pos)
996- _, _ = g.topoLevel(pos)
997- _, _ = g.generation(pos)
998- _, _ = g.appendParents(buf[:0], pos)
999- g.maybeChanged(pos, []byte("src/main.go"))
1000- g.hasBloom(pos)
1001- }
1002- _, _ = g.commitID(g.total)
1003- _, _ = g.appendParents(nil, g.total+1)
1004- g.maybeChanged(g.total, nil)
1005- g.lookup(oid{0xff, 0xff})
1006- g.lookup(oid{})
1379+ exerciseGraph(g)
10071380 }
10081381 }
10091382
1383+// exerciseGraph runs every accessor over (up to 256 positions of) g.
1384+func exerciseGraph(g *commitGraph) {
1385+ var buf [4]uint32
1386+ n := min(g.total, 256)
1387+ for pos := range n {
1388+ id, _ := g.commitID(pos)
1389+ g.lookup(id)
1390+ _, _ = g.rootTree(pos)
1391+ _, _ = g.commitTime(pos)
1392+ _, _ = g.topoLevel(pos)
1393+ _, _ = g.generation(pos)
1394+ _, _ = g.appendParents(buf[:0], pos)
1395+ g.maybeChanged(pos, []byte("src/main.go"))
1396+ g.maybeChanged(pos, []byte("caf\xc3\xa9/"))
1397+ g.hasBloom(pos)
1398+ }
1399+ _, _ = g.commitID(g.total)
1400+ _, _ = g.appendParents(nil, g.total+1)
1401+ g.maybeChanged(g.total, nil)
1402+ g.lookup(oid{0xff, 0xff})
1403+ g.lookup(oid{})
1404+}
1405+
10101406 func graphSeedFiles(tb testing.TB) map[string][]byte {
10111407 tb.Helper()
10121408 entries, err := os.ReadDir(filepath.Join("testdata", "graph"))
@@ -1050,6 +1446,88 @@ func FuzzGraphChain(f *testing.F) {
10501446 })
10511447 }
10521448
1449+// graphChainSeeds returns, for each committed split chain, the chain file
1450+// content and its layer files (base first).
1451+func graphChainSeeds(tb testing.TB) (chains [][]byte, layers [][2][]byte) {
1452+ tb.Helper()
1453+ seeds := graphSeedFiles(tb)
1454+ for _, prefix := range []string{"split-bloom-v1", "split-bloom-v2", "split-gen1"} {
1455+ l0, l1 := seeds[prefix+"-layer0.graph"], seeds[prefix+"-layer1.graph"]
1456+ if l0 == nil || l1 == nil {
1457+ tb.Fatalf("seed %s missing", prefix)
1458+ }
1459+ chain := hex.EncodeToString(l0[len(l0)-graphHashSize:]) + "\n" +
1460+ hex.EncodeToString(l1[len(l1)-graphHashSize:]) + "\n"
1461+ chains = append(chains, []byte(chain))
1462+ layers = append(layers, [2][]byte{l0, l1})
1463+ }
1464+ return chains, layers
1465+}
1466+
1467+// assembleChainBytes is openCommitGraphChain on in-memory files: layer i of
1468+// the chain is files[i%len(files)].
1469+func assembleChainBytes(chain []byte, files ...[]byte) (*commitGraph, error) {
1470+ names, err := parseGraphChain(chain)
1471+ if err != nil {
1472+ return nil, err
1473+ }
1474+ layers := make([]*graphLayer, 0, len(names))
1475+ for i := range names {
1476+ l, err := parseGraphLayer(files[i%len(files)])
1477+ if err != nil {
1478+ return nil, err
1479+ }
1480+ layers = append(layers, l)
1481+ }
1482+ return assembleGraph(layers, names)
1483+}
1484+
1485+func TestGraphChainSeedsAssemble(t *testing.T) {
1486+ chains, layers := graphChainSeeds(t)
1487+ for i := range chains {
1488+ g, err := assembleChainBytes(chains[i], layers[i][0], layers[i][1])
1489+ if err != nil {
1490+ t.Fatalf("seed %d: %v", i, err)
1491+ }
1492+ if g.numLayers() != 2 || g.numCommits() == 0 {
1493+ t.Errorf("seed %d: %d layers, %d commits", i, g.numLayers(), g.numCommits())
1494+ }
1495+ exerciseGraph(g)
1496+ // Swapped layers or a damaged name must be refused, not crash.
1497+ if _, err := assembleChainBytes(chains[i], layers[i][1], layers[i][0]); err == nil {
1498+ t.Errorf("seed %d: swapped layers accepted", i)
1499+ }
1500+ bad := append([]byte(nil), chains[i]...)
1501+ bad[3] ^= 1
1502+ if _, err := assembleChainBytes(bad, layers[i][0], layers[i][1]); !errors.Is(err, errGraphCorrupt) {
1503+ t.Errorf("seed %d: wrong layer name: err = %v", i, err)
1504+ }
1505+ }
1506+}
1507+
1508+// FuzzGraphAssemble reaches parseGraphChain, parseGraphLayer and
1509+// assembleGraph together: a chain file and two layer files (used round-robin
1510+// for the layers the chain lists).
1511+func FuzzGraphAssemble(f *testing.F) {
1512+ chains, layers := graphChainSeeds(f)
1513+ for i := range chains {
1514+ f.Add(chains[i], layers[i][0], layers[i][1])
1515+ f.Add(chains[i][:41], layers[i][0], layers[i][1]) // base layer alone
1516+ f.Add(append(chains[i][:41:41], chains[i][:41]...), layers[i][0], layers[i][0])
1517+ }
1518+ f.Add([]byte(strings.Repeat("00", 20)+"\n"), (&synthLayer{ids: synthIDs(3)}).bytes(), []byte{})
1519+ f.Fuzz(func(t *testing.T, chain, l0, l1 []byte) {
1520+ g, err := assembleChainBytes(chain, l0, l1)
1521+ if err != nil {
1522+ return
1523+ }
1524+ if g.numLayers() == 0 || g.numLayers() > graphMaxLayers {
1525+ t.Fatalf("%d layers", g.numLayers())
1526+ }
1527+ exerciseGraph(g)
1528+ })
1529+}
1530+
10531531 // TestGraphSeedsParse checks the committed seeds stay valid files.
10541532 func TestGraphSeedsParse(t *testing.T) {
10551533 for name, b := range graphSeedFiles(t) {
@@ -9,13 +9,17 @@ import (
9 "encoding/hex"9 "encoding/hex"
10 "errors"10 "errors"
11 "fmt"11 "fmt"
12+ "io/fs"
12 "os"13 "os"
13 "os/exec"14 "os/exec"
14 "path/filepath"15 "path/filepath"
15 "sort"16 "sort"
16 "strconv"17 "strconv"
17 "strings"18 "strings"
19+ "sync"
20+ "sync/atomic"
18 "testing"21 "testing"
22+ "time"
19 )23 )
20 24
21 // oid is the raw object id used by the graph code until the core's ObjectID25 // oid is the raw object id used by the graph code until the core's ObjectID
@@ -505,6 +509,132 @@ func TestGraphRefcount(t *testing.T) {
505 }509 }
506 }510 }
507 511
512+// close is idempotent: a second call must not drop a reader's reference.
513+func TestGraphCloseIdempotent(t *testing.T) {
514+ r := newGTRepo(t)
515+ r.commit("a", "a\n")
516+ r.git("commit-graph", "write", "--reachable")
517+ g, err := openCommitGraph(r.objects())
518+ if err != nil {
519+ t.Fatal(err)
520+ }
521+ if !g.retain() {
522+ t.Fatal("retain failed on open graph")
523+ }
524+ var unmapped atomic.Int32
525+ for i, u := range g.unmaps {
526+ g.unmaps[i] = func() error { unmapped.Add(1); return u() }
527+ }
528+ for range 3 {
529+ if err := g.close(); err != nil {
530+ t.Fatal(err)
531+ }
532+ }
533+ if unmapped.Load() != 0 {
534+ t.Fatal("unmapped while a reader holds a reference")
535+ }
536+ if g.retain() {
537+ t.Fatal("retain succeeded after close")
538+ }
539+ if _, err := g.commitID(0); err != nil {
540+ t.Fatal(err)
541+ }
542+ if err := g.release(); err != nil {
543+ t.Fatal(err)
544+ }
545+ if unmapped.Load() != int32(len(g.layers)) {
546+ t.Fatalf("unmapped %d files, want %d", unmapped.Load(), len(g.layers))
547+ }
548+ if err := g.close(); err != nil {
549+ t.Fatal(err)
550+ }
551+ if unmapped.Load() != int32(len(g.layers)) {
552+ t.Fatal("close after the last release unmapped again")
553+ }
554+}
555+
556+// Readers retain, read and release while the owner closes (more than once).
557+// The files must never be unmapped while a reader holds a reference; a read
558+// of an unmapped file would also crash the test.
559+func TestGraphConcurrentCloseRetain(t *testing.T) {
560+ r := newGTRepo(t)
561+ for i := range 5 {
562+ r.commit(fmt.Sprintf("f%d", i), "x\n")
563+ r.git("commit-graph", "write", "--reachable", "--split=no-merge", "--changed-paths")
564+ }
565+ const readers = 16
566+ rounds := 20
567+ if testing.Short() {
568+ rounds = 3
569+ }
570+ for range rounds {
571+ g, err := openCommitGraph(r.objects())
572+ if err != nil {
573+ t.Fatal(err)
574+ }
575+ var active, unmapped atomic.Int32
576+ for i, u := range g.unmaps {
577+ g.unmaps[i] = func() error {
578+ if active.Load() != 0 {
579+ t.Error("unmapped while referenced")
580+ }
581+ unmapped.Store(1)
582+ return u()
583+ }
584+ }
585+ var wg sync.WaitGroup
586+ start := make(chan struct{})
587+ var retained atomic.Int64
588+ for range readers {
589+ wg.Go(func() {
590+ <-start
591+ for range 200 {
592+ if !g.retain() {
593+ return
594+ }
595+ retained.Add(1)
596+ active.Add(1)
597+ if unmapped.Load() != 0 {
598+ t.Error("reference granted on unmapped graph")
599+ }
600+ for pos := range g.numCommits() {
601+ id, err := g.commitID(pos)
602+ if err != nil {
603+ t.Error(err)
604+ }
605+ if got, ok := g.lookup(id); !ok || got != pos {
606+ t.Errorf("lookup(%d) = %d, %v", pos, got, ok)
607+ }
608+ if _, err := g.appendParents(nil, pos); err != nil {
609+ t.Error(err)
610+ }
611+ g.maybeChanged(pos, []byte("f1"))
612+ }
613+ active.Add(-1)
614+ if err := g.release(); err != nil {
615+ t.Error(err)
616+ }
617+ }
618+ })
619+ }
620+ close(start)
621+ for range 3 {
622+ wg.Go(func() {
623+ if err := g.close(); err != nil {
624+ t.Error(err)
625+ }
626+ })
627+ }
628+ wg.Wait()
629+ if unmapped.Load() != 1 {
630+ t.Fatal("files never unmapped")
631+ }
632+ if g.retain() {
633+ t.Fatal("retain succeeded after close")
634+ }
635+ }
636+}
637+
508 // A layer removed or rewritten while a graph is open must not affect it.638 // A layer removed or rewritten while a graph is open must not affect it.
509 func TestGraphSurvivesRewrite(t *testing.T) {639 func TestGraphSurvivesRewrite(t *testing.T) {
510 r := newGTRepo(t)640 r := newGTRepo(t)
@@ -532,6 +662,124 @@ func TestGraphSurvivesRewrite(t *testing.T) {
532 }662 }
533 }663 }
534 664
665+// setLayerHook installs graphBeforeLayerOpen for the duration of the test.
666+func setLayerHook(t *testing.T, f func(path string)) {
667+ t.Helper()
668+ graphBeforeLayerOpen = f
669+ t.Cleanup(func() { graphBeforeLayerOpen = nil })
670+}
671+
672+// splitChainRepo has a graph chain of several layers.
673+func splitChainRepo(t *testing.T) *gtRepo {
674+ r := newGTRepo(t)
675+ for i := range 3 {
676+ r.commit(fmt.Sprintf("f%d", i), "x\n")
677+ r.git("commit-graph", "write", "--reachable", "--split=no-merge", "--changed-paths")
678+ }
679+ return r
680+}
681+
682+// A concurrent merging "commit-graph write --split" deletes the layers named
683+// by the chain just read: opening starts again and succeeds.
684+func TestGraphOpenRetriesVanishedLayer(t *testing.T) {
685+ r := splitChainRepo(t)
686+ calls := 0
687+ setLayerHook(t, func(string) {
688+ if calls++; calls == 1 {
689+ r.git("commit-graph", "write", "--reachable", "--split=replace", "--changed-paths", "--expire-time=now")
690+ }
691+ })
692+ g, err := openCommitGraph(r.objects())
693+ if err != nil {
694+ t.Fatal(err)
695+ }
696+ defer func() { _ = g.close() }()
697+ if g.numLayers() != 1 {
698+ t.Fatalf("layers = %d, want the replaced chain of 1", g.numLayers())
699+ }
700+ checkGraphAgainstGit(t, r.dir, g, false)
701+}
702+
703+// Layers that keep vanishing end in errNoGraph, which callers treat as
704+// "read the objects instead", wrapping the cause.
705+func TestGraphOpenGivesUpOnVanishedLayer(t *testing.T) {
706+ r := splitChainRepo(t)
707+ calls := 0
708+ setLayerHook(t, func(path string) {
709+ calls++
710+ _ = os.Remove(path)
711+ })
712+ g, err := openCommitGraph(r.objects())
713+ if err == nil {
714+ _ = g.close()
715+ t.Fatal("opened a graph with a missing layer")
716+ }
717+ if !errors.Is(err, errNoGraph) || !errors.Is(err, fs.ErrNotExist) {
718+ t.Fatalf("err = %v, want errNoGraph wrapping ErrNotExist", err)
719+ }
720+ if calls != 1+graphOpenRetries {
721+ t.Fatalf("%d attempts, want %d", calls, 1+graphOpenRetries)
722+ }
723+}
724+
725+// Opening in a loop while git rewrites the chain: success or errNoGraph only.
726+func TestGraphOpenStressSplitWrite(t *testing.T) {
727+ r := newGTRepo(t)
728+ r.commit("a", "a\n")
729+ r.git("commit-graph", "write", "--reachable", "--split", "--changed-paths")
730+ commits := 150
731+ if testing.Short() {
732+ commits = 10
733+ }
734+ var done atomic.Bool
735+ writerErr := make(chan error, 1)
736+ go func() {
737+ defer done.Store(true)
738+ for i := range commits {
739+ for _, args := range [][]string{
740+ {"commit", "-q", "--allow-empty", "-m", fmt.Sprint("s", i)},
741+ {"commit-graph", "write", "--reachable", "--split", "--changed-paths", "--expire-time=now"},
742+ } {
743+ cmd := exec.Command("git", args...)
744+ cmd.Dir = r.dir
745+ cmd.Env = graphGitEnv(0)
746+ if out, err := cmd.CombinedOutput(); err != nil {
747+ writerErr <- fmt.Errorf("git %v: %v\n%s", args, err, out)
748+ return
749+ }
750+ }
751+ }
752+ writerErr <- nil
753+ }()
754+ // Count the layer opens beyond the chain length: retries.
755+ var layerOpens atomic.Int64
756+ setLayerHook(t, func(string) { layerOpens.Add(1) })
757+ var opened, none int
758+ deadline := time.Now().Add(2 * time.Minute)
759+ for !done.Load() && time.Now().Before(deadline) {
760+ g, err := openCommitGraph(r.objects())
761+ switch {
762+ case err == nil:
763+ opened++
764+ if g.numCommits() == 0 {
765+ t.Error("empty graph")
766+ }
767+ _ = g.close()
768+ case errors.Is(err, errNoGraph):
769+ none++
770+ default:
771+ t.Fatalf("open: %v", err)
772+ }
773+ }
774+ if err := <-writerErr; err != nil {
775+ t.Fatal(err)
776+ }
777+ t.Logf("opened %d, no graph %d, layer opens %d", opened, none, layerOpens.Load())
778+ if opened == 0 {
779+ t.Error("never opened a graph")
780+ }
781+}
782+
535 func TestGraphStamp(t *testing.T) {783 func TestGraphStamp(t *testing.T) {
536 r := newGTRepo(t)784 r := newGTRepo(t)
537 r.commit("a", "a\n")785 r.commit("a", "a\n")
@@ -939,6 +1187,148 @@ func TestGraphChainMismatch(t *testing.T) {
939 }1187 }
940 }1188 }
941 1189
1190+// A layer file whose trailer is not the name the chain gives it is rejected.
1191+func TestGraphLayerChecksumMustMatchName(t *testing.T) {
1192+ mk := func(sum byte, base []oid) *graphLayer {
1193+ s := &synthLayer{ids: synthIDs(3), base: base}
1194+ b := s.bytes()
1195+ for i := len(b) - graphHashSize; i < len(b); i++ {
1196+ b[i] = sum
1197+ }
1198+ l, err := parseGraphLayer(b)
1199+ if err != nil {
1200+ t.Fatal(err)
1201+ }
1202+ return l
1203+ }
1204+ name := func(b byte) [graphHashSize]byte { return [graphHashSize]byte(bytes.Repeat([]byte{b}, graphHashSize)) }
1205+ chain := func(a, b byte) (*commitGraph, error) {
1206+ return assembleGraph([]*graphLayer{mk(0xa1, nil), mk(0xb2, []oid{name(0xa1)})},
1207+ [][graphHashSize]byte{name(a), name(b)})
1208+ }
1209+ if _, err := chain(0xa1, 0xb2); err != nil {
1210+ t.Fatalf("matching names: %v", err)
1211+ }
1212+ if _, err := chain(0xa1, 0xb3); !errors.Is(err, errGraphCorrupt) {
1213+ t.Errorf("top layer renamed: err = %v", err)
1214+ }
1215+ // The base list of the top layer names the base correctly, but the base
1216+ // file itself carries another checksum.
1217+ if _, err := assembleGraph([]*graphLayer{mk(0xee, nil), mk(0xb2, []oid{name(0xa1)})},
1218+ [][graphHashSize]byte{name(0xa1), name(0xb2)}); !errors.Is(err, errGraphCorrupt) {
1219+ t.Errorf("base layer with another checksum: err = %v", err)
1220+ }
1221+ // Several layers need chain names.
1222+ if _, err := assembleGraph([]*graphLayer{mk(1, nil), mk(2, []oid{name(1)})}, nil); !errors.Is(err, errGraphCorrupt) {
1223+ t.Errorf("two layers without names: err = %v", err)
1224+ }
1225+}
1226+
1227+// On disk: a layer copied under another name is not accepted as that layer.
1228+func TestGraphRenamedLayerRejected(t *testing.T) {
1229+ r := newGTRepo(t)
1230+ r.commit("a", "a\n")
1231+ r.git("commit-graph", "write", "--reachable", "--split")
1232+ dir := filepath.Join(r.objects(), "info", "commit-graphs")
1233+ chainPath := filepath.Join(dir, "commit-graph-chain")
1234+ raw, err := os.ReadFile(chainPath)
1235+ if err != nil {
1236+ t.Fatal(err)
1237+ }
1238+ good := strings.TrimSpace(string(raw))
1239+ fake := strings.Repeat("0", 40)
1240+ b, err := os.ReadFile(filepath.Join(dir, "graph-"+good+".graph"))
1241+ if err != nil {
1242+ t.Fatal(err)
1243+ }
1244+ if err := os.WriteFile(filepath.Join(dir, "graph-"+fake+".graph"), b, 0o644); err != nil {
1245+ t.Fatal(err)
1246+ }
1247+ if err := os.Chmod(chainPath, 0o644); err != nil {
1248+ t.Fatal(err)
1249+ }
1250+ if err := os.WriteFile(chainPath, []byte(fake+"\n"), 0o644); err != nil {
1251+ t.Fatal(err)
1252+ }
1253+ if g, err := openCommitGraph(r.objects()); !errors.Is(err, errGraphCorrupt) {
1254+ if err == nil {
1255+ _ = g.close()
1256+ }
1257+ t.Fatalf("err = %v, want corrupt", err)
1258+ }
1259+}
1260+
1261+// Optional chunks of the wrong size disable their feature only.
1262+func TestGraphOptionalChunkSizes(t *testing.T) {
1263+ const n = 4
1264+ hdr := binary.BigEndian.AppendUint32(nil, 2)
1265+ hdr = binary.BigEndian.AppendUint32(hdr, 7)
1266+ hdr = binary.BigEndian.AppendUint32(hdr, 10)
1267+ mk := func() *synthLayer {
1268+ s := &synthLayer{ids: synthIDs(n), parents: make([][]uint32, n), times: make([]int64, n), levels: []uint32{5, 6, 7, 8}}
1269+ s.times = []int64{100, 200, 300, 400}
1270+ s.parents[1] = []uint32{0}
1271+ s.gda2 = []uint32{0, 1, 2, 3}
1272+ s.bdat = append(hdr[:12:12], 0x00)
1273+ s.bidx = []uint32{1, 1, 1, 1}
1274+ return s
1275+ }
1276+ t.Run("ok", func(t *testing.T) {
1277+ g := mustParseSynth(t, mk().bytes())
1278+ defer func() { _ = g.close() }()
1279+ if !g.hasGenerationV2() || !g.layers[0].hasBloom {
1280+ t.Fatal("features missing on a good layer")
1281+ }
1282+ if v, _ := g.generation(1); v != 201 {
1283+ t.Fatalf("generation = %d, want 201", v)
1284+ }
1285+ })
1286+ t.Run("GDA2", func(t *testing.T) {
1287+ for _, size := range []int{n - 1, n + 1} {
1288+ s := mk()
1289+ s.gda2 = make([]uint32, size)
1290+ g := mustParseSynth(t, s.bytes())
1291+ if g.hasGenerationV2() {
1292+ t.Errorf("GDA2 of %d entries accepted", size)
1293+ }
1294+ if v, err := g.generation(1); err != nil || v != 6 {
1295+ t.Errorf("generation = %d, %v, want the level 6", v, err)
1296+ }
1297+ if !g.layers[0].hasBloom {
1298+ t.Error("a bad GDA2 disabled the Bloom filters")
1299+ }
1300+ if _, err := g.appendParents(nil, 1); err != nil {
1301+ t.Error(err)
1302+ }
1303+ _ = g.close()
1304+ }
1305+ })
1306+ t.Run("BIDX", func(t *testing.T) {
1307+ s := mk()
1308+ s.bidx = s.bidx[:n-1]
1309+ g := mustParseSynth(t, s.bytes())
1310+ defer func() { _ = g.close() }()
1311+ if g.layers[0].hasBloom || g.hasBloom(0) {
1312+ t.Error("short BIDX accepted")
1313+ }
1314+ if !g.hasGenerationV2() {
1315+ t.Error("a bad BIDX disabled the generation")
1316+ }
1317+ })
1318+ t.Run("BDAT", func(t *testing.T) {
1319+ s := mk()
1320+ s.bdat = s.bdat[:bloomHeaderSize-1]
1321+ g := mustParseSynth(t, s.bytes())
1322+ defer func() { _ = g.close() }()
1323+ if g.layers[0].hasBloom || g.hasBloom(0) {
1324+ t.Error("short BDAT accepted")
1325+ }
1326+ if !g.hasGenerationV2() {
1327+ t.Error("a bad BDAT disabled the generation")
1328+ }
1329+ })
1330+}
1331+
942 func TestGraphCorruptFiles(t *testing.T) {1332 func TestGraphCorruptFiles(t *testing.T) {
943 r := newGTRepo(t)1333 r := newGTRepo(t)
944 r.commit("a", "a\n")1334 r.commit("a", "a\n")
@@ -986,27 +1376,33 @@ func exerciseGraphBytes(data []byte) {
986 {layers: []graphLayer{one}, total: one.n, genV2: one.gda2 != nil},1376 {layers: []graphLayer{one}, total: one.n, genV2: one.gda2 != nil},
987 {layers: []graphLayer{one, two}, total: 2 * one.n, genV2: one.gda2 != nil},1377 {layers: []graphLayer{one, two}, total: 2 * one.n, genV2: one.gda2 != nil},
988 } {1378 } {
989- var buf [4]uint321379+ exerciseGraph(g)
990- n := min(g.total, 256)
991- for pos := range n {
992- id, _ := g.commitID(pos)
993- g.lookup(id)
994- _, _ = g.rootTree(pos)
995- _, _ = g.commitTime(pos)
996- _, _ = g.topoLevel(pos)
997- _, _ = g.generation(pos)
998- _, _ = g.appendParents(buf[:0], pos)
999- g.maybeChanged(pos, []byte("src/main.go"))
1000- g.hasBloom(pos)
1001- }
1002- _, _ = g.commitID(g.total)
1003- _, _ = g.appendParents(nil, g.total+1)
1004- g.maybeChanged(g.total, nil)
1005- g.lookup(oid{0xff, 0xff})
1006- g.lookup(oid{})
1007 }1380 }
1008 }1381 }
1009 1382
1383+// exerciseGraph runs every accessor over (up to 256 positions of) g.
1384+func exerciseGraph(g *commitGraph) {
1385+ var buf [4]uint32
1386+ n := min(g.total, 256)
1387+ for pos := range n {
1388+ id, _ := g.commitID(pos)
1389+ g.lookup(id)
1390+ _, _ = g.rootTree(pos)
1391+ _, _ = g.commitTime(pos)
1392+ _, _ = g.topoLevel(pos)
1393+ _, _ = g.generation(pos)
1394+ _, _ = g.appendParents(buf[:0], pos)
1395+ g.maybeChanged(pos, []byte("src/main.go"))
1396+ g.maybeChanged(pos, []byte("caf\xc3\xa9/"))
1397+ g.hasBloom(pos)
1398+ }
1399+ _, _ = g.commitID(g.total)
1400+ _, _ = g.appendParents(nil, g.total+1)
1401+ g.maybeChanged(g.total, nil)
1402+ g.lookup(oid{0xff, 0xff})
1403+ g.lookup(oid{})
1404+}
1405+
1010 func graphSeedFiles(tb testing.TB) map[string][]byte {1406 func graphSeedFiles(tb testing.TB) map[string][]byte {
1011 tb.Helper()1407 tb.Helper()
1012 entries, err := os.ReadDir(filepath.Join("testdata", "graph"))1408 entries, err := os.ReadDir(filepath.Join("testdata", "graph"))
@@ -1050,6 +1446,88 @@ func FuzzGraphChain(f *testing.F) {
1050 })1446 })
1051 }1447 }
1052 1448
1449+// graphChainSeeds returns, for each committed split chain, the chain file
1450+// content and its layer files (base first).
1451+func graphChainSeeds(tb testing.TB) (chains [][]byte, layers [][2][]byte) {
1452+ tb.Helper()
1453+ seeds := graphSeedFiles(tb)
1454+ for _, prefix := range []string{"split-bloom-v1", "split-bloom-v2", "split-gen1"} {
1455+ l0, l1 := seeds[prefix+"-layer0.graph"], seeds[prefix+"-layer1.graph"]
1456+ if l0 == nil || l1 == nil {
1457+ tb.Fatalf("seed %s missing", prefix)
1458+ }
1459+ chain := hex.EncodeToString(l0[len(l0)-graphHashSize:]) + "\n" +
1460+ hex.EncodeToString(l1[len(l1)-graphHashSize:]) + "\n"
1461+ chains = append(chains, []byte(chain))
1462+ layers = append(layers, [2][]byte{l0, l1})
1463+ }
1464+ return chains, layers
1465+}
1466+
1467+// assembleChainBytes is openCommitGraphChain on in-memory files: layer i of
1468+// the chain is files[i%len(files)].
1469+func assembleChainBytes(chain []byte, files ...[]byte) (*commitGraph, error) {
1470+ names, err := parseGraphChain(chain)
1471+ if err != nil {
1472+ return nil, err
1473+ }
1474+ layers := make([]*graphLayer, 0, len(names))
1475+ for i := range names {
1476+ l, err := parseGraphLayer(files[i%len(files)])
1477+ if err != nil {
1478+ return nil, err
1479+ }
1480+ layers = append(layers, l)
1481+ }
1482+ return assembleGraph(layers, names)
1483+}
1484+
1485+func TestGraphChainSeedsAssemble(t *testing.T) {
1486+ chains, layers := graphChainSeeds(t)
1487+ for i := range chains {
1488+ g, err := assembleChainBytes(chains[i], layers[i][0], layers[i][1])
1489+ if err != nil {
1490+ t.Fatalf("seed %d: %v", i, err)
1491+ }
1492+ if g.numLayers() != 2 || g.numCommits() == 0 {
1493+ t.Errorf("seed %d: %d layers, %d commits", i, g.numLayers(), g.numCommits())
1494+ }
1495+ exerciseGraph(g)
1496+ // Swapped layers or a damaged name must be refused, not crash.
1497+ if _, err := assembleChainBytes(chains[i], layers[i][1], layers[i][0]); err == nil {
1498+ t.Errorf("seed %d: swapped layers accepted", i)
1499+ }
1500+ bad := append([]byte(nil), chains[i]...)
1501+ bad[3] ^= 1
1502+ if _, err := assembleChainBytes(bad, layers[i][0], layers[i][1]); !errors.Is(err, errGraphCorrupt) {
1503+ t.Errorf("seed %d: wrong layer name: err = %v", i, err)
1504+ }
1505+ }
1506+}
1507+
1508+// FuzzGraphAssemble reaches parseGraphChain, parseGraphLayer and
1509+// assembleGraph together: a chain file and two layer files (used round-robin
1510+// for the layers the chain lists).
1511+func FuzzGraphAssemble(f *testing.F) {
1512+ chains, layers := graphChainSeeds(f)
1513+ for i := range chains {
1514+ f.Add(chains[i], layers[i][0], layers[i][1])
1515+ f.Add(chains[i][:41], layers[i][0], layers[i][1]) // base layer alone
1516+ f.Add(append(chains[i][:41:41], chains[i][:41]...), layers[i][0], layers[i][0])
1517+ }
1518+ f.Add([]byte(strings.Repeat("00", 20)+"\n"), (&synthLayer{ids: synthIDs(3)}).bytes(), []byte{})
1519+ f.Fuzz(func(t *testing.T, chain, l0, l1 []byte) {
1520+ g, err := assembleChainBytes(chain, l0, l1)
1521+ if err != nil {
1522+ return
1523+ }
1524+ if g.numLayers() == 0 || g.numLayers() > graphMaxLayers {
1525+ t.Fatalf("%d layers", g.numLayers())
1526+ }
1527+ exerciseGraph(g)
1528+ })
1529+}
1530+
1053 // TestGraphSeedsParse checks the committed seeds stay valid files.1531 // TestGraphSeedsParse checks the committed seeds stay valid files.
1054 func TestGraphSeedsParse(t *testing.T) {1532 func TestGraphSeedsParse(t *testing.T) {
1055 for name, b := range graphSeedFiles(t) {1533 for name, b := range graphSeedFiles(t) {

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 1

Tags 2

↑↓ moveenter openesc close