From 66d82a5720259df235ad10665d430d4202661d61 Mon Sep 17 00:00:00 2001 From: Javier Peletier Date: Tue, 23 Oct 2018 18:04:10 +0200 Subject: plumbing/format/packfile: Fix broken "thin" packfile support. Fixes #991 Signed-off-by: Javier Peletier --- plumbing/format/packfile/parser.go | 92 +++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/plumbing/format/packfile/parser.go b/plumbing/format/packfile/parser.go index 28582b5..5a62d63 100644 --- a/plumbing/format/packfile/parser.go +++ b/plumbing/format/packfile/parser.go @@ -38,15 +38,14 @@ type Observer interface { // Parser decodes a packfile and calls any observer associated to it. Is used // to generate indexes. type Parser struct { - storage storer.EncodedObjectStorer - scanner *Scanner - count uint32 - oi []*objectInfo - oiByHash map[plumbing.Hash]*objectInfo - oiByOffset map[int64]*objectInfo - hashOffset map[plumbing.Hash]int64 - pendingRefDeltas map[plumbing.Hash][]*objectInfo - checksum plumbing.Hash + storage storer.EncodedObjectStorer + scanner *Scanner + count uint32 + oi []*objectInfo + oiByHash map[plumbing.Hash]*objectInfo + oiByOffset map[int64]*objectInfo + hashOffset map[plumbing.Hash]int64 + checksum plumbing.Hash cache *cache.BufferLRU // delta content by offset, only used if source is not seekable @@ -78,13 +77,12 @@ func NewParserWithStorage( } return &Parser{ - storage: storage, - scanner: scanner, - ob: ob, - count: 0, - cache: cache.NewBufferLRUDefault(), - pendingRefDeltas: make(map[plumbing.Hash][]*objectInfo), - deltas: deltas, + storage: storage, + scanner: scanner, + ob: ob, + count: 0, + cache: cache.NewBufferLRUDefault(), + deltas: deltas, }, nil } @@ -150,10 +148,6 @@ func (p *Parser) Parse() (plumbing.Hash, error) { return plumbing.ZeroHash, err } - if len(p.pendingRefDeltas) > 0 { - return plumbing.ZeroHash, ErrReferenceDeltaNotFound - } - if err := p.onFooter(p.checksum); err != nil { return plumbing.ZeroHash, err } @@ -205,18 +199,21 @@ func (p *Parser) indexObjects() error { parent.Children = append(parent.Children, ota) case plumbing.REFDeltaObject: delta = true - parent, ok := p.oiByHash[oh.Reference] - if ok { - ota = newDeltaObject(oh.Offset, oh.Length, t, parent) - parent.Children = append(parent.Children, ota) - } else { - ota = newBaseObject(oh.Offset, oh.Length, t) - p.pendingRefDeltas[oh.Reference] = append( - p.pendingRefDeltas[oh.Reference], - ota, - ) + if !ok { + // can't find referenced object in this pack file + // this must be a "thin" pack. + parent = &objectInfo{ //Placeholder parent + SHA1: oh.Reference, + ExternalRef: true, // mark as an external reference that must be resolved + Type: plumbing.AnyObject, + DiskType: plumbing.AnyObject, + } + p.oiByHash[oh.Reference] = parent } + ota = newDeltaObject(oh.Offset, oh.Length, t, parent) + parent.Children = append(parent.Children, ota) + default: ota = newBaseObject(oh.Offset, oh.Length, t) } @@ -297,16 +294,20 @@ func (p *Parser) resolveDeltas() error { return nil } -func (p *Parser) get(o *objectInfo) ([]byte, error) { - b, ok := p.cache.Get(o.Offset) +func (p *Parser) get(o *objectInfo) (b []byte, err error) { + var ok bool + if !o.ExternalRef { // skip cache check for placeholder parents + b, ok = p.cache.Get(o.Offset) + } + // If it's not on the cache and is not a delta we can try to find it in the - // storage, if there's one. + // storage, if there's one. External refs must enter here. if !ok && p.storage != nil && !o.Type.IsDelta() { - var err error e, err := p.storage.EncodedObject(plumbing.AnyObject, o.SHA1) if err != nil { return nil, err } + o.Type = e.Type() r, err := e.Reader() if err != nil { @@ -323,6 +324,11 @@ func (p *Parser) get(o *objectInfo) ([]byte, error) { return b, nil } + if o.ExternalRef { + // we were not able to resolve a ref in a thin pack + return nil, ErrReferenceDeltaNotFound + } + var data []byte if o.DiskType.IsDelta() { base, err := p.get(o.Parent) @@ -335,7 +341,6 @@ func (p *Parser) get(o *objectInfo) ([]byte, error) { return nil, err } } else { - var err error data, err = p.readData(o) if err != nil { return nil, err @@ -367,14 +372,6 @@ func (p *Parser) resolveObject( return nil, err } - if pending, ok := p.pendingRefDeltas[o.SHA1]; ok { - for _, po := range pending { - po.Parent = o - o.Children = append(o.Children, po) - } - delete(p.pendingRefDeltas, o.SHA1) - } - if p.storage != nil { obj := new(plumbing.MemoryObject) obj.SetSize(o.Size()) @@ -447,10 +444,11 @@ func getSHA1(t plumbing.ObjectType, data []byte) (plumbing.Hash, error) { } type objectInfo struct { - Offset int64 - Length int64 - Type plumbing.ObjectType - DiskType plumbing.ObjectType + Offset int64 + Length int64 + Type plumbing.ObjectType + DiskType plumbing.ObjectType + ExternalRef bool // indicates this is an external reference in a thin pack file Crc32 uint32 -- cgit From 38ebf56975eb2d4eec2c42fbd84dc62f17718a14 Mon Sep 17 00:00:00 2001 From: Javier Peletier Date: Mon, 12 Nov 2018 14:46:03 +0100 Subject: plumbing/format/packfile: Added thin pack test Signed-off-by: Javier Peletier --- plumbing/format/packfile/parser_test.go | 50 +++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/plumbing/format/packfile/parser_test.go b/plumbing/format/packfile/parser_test.go index 012a140..6e7c84b 100644 --- a/plumbing/format/packfile/parser_test.go +++ b/plumbing/format/packfile/parser_test.go @@ -1,10 +1,13 @@ package packfile_test import ( + "io" "testing" + git "gopkg.in/src-d/go-git.v4" "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" + "gopkg.in/src-d/go-git.v4/plumbing/storer" . "gopkg.in/check.v1" "gopkg.in/src-d/go-git-fixtures.v3" @@ -74,6 +77,53 @@ func (s *ParserSuite) TestParserHashes(c *C) { c.Assert(obs.objects, DeepEquals, objs) } +func (s *ParserSuite) TestThinPack(c *C) { + + // Initialize an empty repository + fs, err := git.PlainInit(c.MkDir(), true) + c.Assert(err, IsNil) + + // Try to parse a thin pack without having the required objects in the repo to + // see if the correct errors are returned + thinpack := fixtures.ByTag("thinpack").One() + scanner := packfile.NewScanner(thinpack.Packfile()) + parser, err := packfile.NewParserWithStorage(scanner, fs.Storer) // ParserWithStorage writes to the storer all parsed objects! + c.Assert(err, IsNil) + + _, err = parser.Parse() + c.Assert(err, Equals, plumbing.ErrObjectNotFound) + + // start over with a clean repo + fs, err = git.PlainInit(c.MkDir(), true) + c.Assert(err, IsNil) + + // Now unpack a base packfile into our empty repo: + f := fixtures.ByURL("https://github.com/spinnaker/spinnaker.git").One() + w, err := fs.Storer.(storer.PackfileWriter).PackfileWriter() + c.Assert(err, IsNil) + _, err = io.Copy(w, f.Packfile()) + c.Assert(err, IsNil) + w.Close() + + // Check that the test object that will come with our thin pack is *not* in the repo + _, err = fs.Storer.EncodedObject(plumbing.CommitObject, thinpack.Head) + c.Assert(err, Equals, plumbing.ErrObjectNotFound) + + // Now unpack the thin pack: + scanner = packfile.NewScanner(thinpack.Packfile()) + parser, err = packfile.NewParserWithStorage(scanner, fs.Storer) // ParserWithStorage writes to the storer all parsed objects! + c.Assert(err, IsNil) + + h, err := parser.Parse() + c.Assert(err, IsNil) + c.Assert(h, Equals, plumbing.NewHash("1288734cbe0b95892e663221d94b95de1f5d7be8")) + + // Check that our test object is now accessible + _, err = fs.Storer.EncodedObject(plumbing.CommitObject, thinpack.Head) + c.Assert(err, IsNil) + +} + type observerObject struct { hash string otype plumbing.ObjectType -- cgit