From 522327b572276fe94e76ff9bb5e41b1efdf69dee Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Wed, 24 Jan 2018 18:32:16 +0100 Subject: plumbing: format/packfile, fix crash with cycle deltas Resolving cycles relied on ObjectToPack objects having Original. This is no longer true with the changes from #720. This commit changes: * Save original type, hash and size in ObjectToPack * Use SetObject to set both Original and resolved type, hash and size * Restore original object before using BackToOriginal (cycle resolution) * Update encoder test to check this case Signed-off-by: Javi Fontan --- plumbing/format/packfile/delta_selector.go | 3 ++- plumbing/format/packfile/encoder.go | 1 + plumbing/format/packfile/encoder_test.go | 21 +++++++++++++++++++++ plumbing/format/packfile/object_pack.go | 27 +++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) (limited to 'plumbing') diff --git a/plumbing/format/packfile/delta_selector.go b/plumbing/format/packfile/delta_selector.go index cd38c16..1d9fb5f 100644 --- a/plumbing/format/packfile/delta_selector.go +++ b/plumbing/format/packfile/delta_selector.go @@ -196,7 +196,8 @@ func (dw *deltaSelector) restoreOriginal(otp *ObjectToPack) error { return err } - otp.Original = obj + otp.SetOriginal(obj) + return nil } diff --git a/plumbing/format/packfile/encoder.go b/plumbing/format/packfile/encoder.go index 6686dd5..b077918 100644 --- a/plumbing/format/packfile/encoder.go +++ b/plumbing/format/packfile/encoder.go @@ -87,6 +87,7 @@ func (e *Encoder) entry(o *ObjectToPack) error { // (for example due to a concurrent repack) and a different base // was chosen, forcing a cycle. Select something other than a // delta, and write this object. + e.selector.restoreOriginal(o) o.BackToOriginal() } diff --git a/plumbing/format/packfile/encoder_test.go b/plumbing/format/packfile/encoder_test.go index 320036b..06cb1c6 100644 --- a/plumbing/format/packfile/encoder_test.go +++ b/plumbing/format/packfile/encoder_test.go @@ -202,6 +202,15 @@ func (s *EncoderSuite) deltaOverDeltaCyclicTest(c *C) { o3 := newObject(plumbing.BlobObject, []byte("011111")) o4 := newObject(plumbing.BlobObject, []byte("01111100000")) + _, err := s.store.SetEncodedObject(o1) + c.Assert(err, IsNil) + _, err = s.store.SetEncodedObject(o2) + c.Assert(err, IsNil) + _, err = s.store.SetEncodedObject(o3) + c.Assert(err, IsNil) + _, err = s.store.SetEncodedObject(o4) + c.Assert(err, IsNil) + d2, err := GetDelta(o1, o2) c.Assert(err, IsNil) @@ -219,6 +228,18 @@ func (s *EncoderSuite) deltaOverDeltaCyclicTest(c *C) { pd3.SetDelta(pd4, d3) pd4.SetDelta(pd3, d4) + // SetOriginal is used by delta selector when generating ObjectToPack. + // It also fills type, hash and size values to be used when Original + // is nil. + po1.SetOriginal(po1.Original) + pd2.SetOriginal(pd2.Original) + pd2.Original = nil + + pd3.SetOriginal(pd3.Original) + pd3.Original = nil + + pd4.SetOriginal(pd4.Original) + encHash, err := s.enc.encode([]*ObjectToPack{ po1, pd2, diff --git a/plumbing/format/packfile/object_pack.go b/plumbing/format/packfile/object_pack.go index 1563517..284871f 100644 --- a/plumbing/format/packfile/object_pack.go +++ b/plumbing/format/packfile/object_pack.go @@ -23,6 +23,12 @@ type ObjectToPack struct { // offset in pack when object has been already written, or 0 if it // has not been written yet Offset int64 + + // Information from the original object + resolvedOriginal bool + originalType plumbing.ObjectType + originalSize int64 + originalHash plumbing.Hash } // newObjectToPack creates a correct ObjectToPack based on a non-delta object @@ -71,11 +77,24 @@ func (o *ObjectToPack) WantWrite() bool { return o.Offset == 1 } +// SetOriginal sets both Original and saves size, type and hash +func (o *ObjectToPack) SetOriginal(obj plumbing.EncodedObject) { + o.Original = obj + o.originalSize = obj.Size() + o.originalType = obj.Type() + o.originalHash = obj.Hash() + o.resolvedOriginal = true +} + func (o *ObjectToPack) Type() plumbing.ObjectType { if o.Original != nil { return o.Original.Type() } + if o.resolvedOriginal { + return o.originalType + } + if o.Base != nil { return o.Base.Type() } @@ -92,6 +111,10 @@ func (o *ObjectToPack) Hash() plumbing.Hash { return o.Original.Hash() } + if o.resolvedOriginal { + return o.originalHash + } + do, ok := o.Object.(plumbing.DeltaObject) if ok { return do.ActualHash() @@ -105,6 +128,10 @@ func (o *ObjectToPack) Size() int64 { return o.Original.Size() } + if o.resolvedOriginal { + return o.originalSize + } + do, ok := o.Object.(plumbing.DeltaObject) if ok { return do.ActualSize() -- cgit From d5f74d2d3cd63325c6d16f5e93f78d6462ec9308 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 25 Jan 2018 10:29:46 +0100 Subject: plumbing: format/packfile, check nil objects in ObjectToPack SetOriginal now skips setting resolved values if the provided object is nil. BackToOriginal also skips nil Original objects. Signed-off-by: Javi Fontan --- plumbing/format/packfile/encoder_test.go | 4 ++-- plumbing/format/packfile/object_pack.go | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) (limited to 'plumbing') diff --git a/plumbing/format/packfile/encoder_test.go b/plumbing/format/packfile/encoder_test.go index 06cb1c6..63dfafa 100644 --- a/plumbing/format/packfile/encoder_test.go +++ b/plumbing/format/packfile/encoder_test.go @@ -233,10 +233,10 @@ func (s *EncoderSuite) deltaOverDeltaCyclicTest(c *C) { // is nil. po1.SetOriginal(po1.Original) pd2.SetOriginal(pd2.Original) - pd2.Original = nil + pd2.SetOriginal(nil) pd3.SetOriginal(pd3.Original) - pd3.Original = nil + pd3.SetOriginal(nil) pd4.SetOriginal(pd4.Original) diff --git a/plumbing/format/packfile/object_pack.go b/plumbing/format/packfile/object_pack.go index 284871f..877581e 100644 --- a/plumbing/format/packfile/object_pack.go +++ b/plumbing/format/packfile/object_pack.go @@ -53,7 +53,7 @@ func newDeltaObjectToPack(base *ObjectToPack, original, delta plumbing.EncodedOb // BackToOriginal converts that ObjectToPack to a non-deltified object if it was one func (o *ObjectToPack) BackToOriginal() { - if o.IsDelta() { + if o.IsDelta() && o.Original != nil { o.Object = o.Original o.Base = nil o.Depth = 0 @@ -77,13 +77,17 @@ func (o *ObjectToPack) WantWrite() bool { return o.Offset == 1 } -// SetOriginal sets both Original and saves size, type and hash +// SetOriginal sets both Original and saves size, type and hash. If object +// is nil Original is set but previous resolved values are kept func (o *ObjectToPack) SetOriginal(obj plumbing.EncodedObject) { o.Original = obj - o.originalSize = obj.Size() - o.originalType = obj.Type() - o.originalHash = obj.Hash() - o.resolvedOriginal = true + + if obj != nil { + o.originalSize = obj.Size() + o.originalType = obj.Type() + o.originalHash = obj.Hash() + o.resolvedOriginal = true + } } func (o *ObjectToPack) Type() plumbing.ObjectType { -- cgit