Skip to content

Commit 522327b

Browse files
committed
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 src-d#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 <[email protected]>
1 parent 834cd6f commit 522327b

File tree

4 files changed

+51
-1
lines changed

4 files changed

+51
-1
lines changed

plumbing/format/packfile/delta_selector.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,8 @@ func (dw *deltaSelector) restoreOriginal(otp *ObjectToPack) error {
196196
return err
197197
}
198198

199-
otp.Original = obj
199+
otp.SetOriginal(obj)
200+
200201
return nil
201202
}
202203

plumbing/format/packfile/encoder.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ func (e *Encoder) entry(o *ObjectToPack) error {
8787
// (for example due to a concurrent repack) and a different base
8888
// was chosen, forcing a cycle. Select something other than a
8989
// delta, and write this object.
90+
e.selector.restoreOriginal(o)
9091
o.BackToOriginal()
9192
}
9293

plumbing/format/packfile/encoder_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,15 @@ func (s *EncoderSuite) deltaOverDeltaCyclicTest(c *C) {
202202
o3 := newObject(plumbing.BlobObject, []byte("011111"))
203203
o4 := newObject(plumbing.BlobObject, []byte("01111100000"))
204204

205+
_, err := s.store.SetEncodedObject(o1)
206+
c.Assert(err, IsNil)
207+
_, err = s.store.SetEncodedObject(o2)
208+
c.Assert(err, IsNil)
209+
_, err = s.store.SetEncodedObject(o3)
210+
c.Assert(err, IsNil)
211+
_, err = s.store.SetEncodedObject(o4)
212+
c.Assert(err, IsNil)
213+
205214
d2, err := GetDelta(o1, o2)
206215
c.Assert(err, IsNil)
207216

@@ -219,6 +228,18 @@ func (s *EncoderSuite) deltaOverDeltaCyclicTest(c *C) {
219228
pd3.SetDelta(pd4, d3)
220229
pd4.SetDelta(pd3, d4)
221230

231+
// SetOriginal is used by delta selector when generating ObjectToPack.
232+
// It also fills type, hash and size values to be used when Original
233+
// is nil.
234+
po1.SetOriginal(po1.Original)
235+
pd2.SetOriginal(pd2.Original)
236+
pd2.Original = nil
237+
238+
pd3.SetOriginal(pd3.Original)
239+
pd3.Original = nil
240+
241+
pd4.SetOriginal(pd4.Original)
242+
222243
encHash, err := s.enc.encode([]*ObjectToPack{
223244
po1,
224245
pd2,

plumbing/format/packfile/object_pack.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ type ObjectToPack struct {
2323
// offset in pack when object has been already written, or 0 if it
2424
// has not been written yet
2525
Offset int64
26+
27+
// Information from the original object
28+
resolvedOriginal bool
29+
originalType plumbing.ObjectType
30+
originalSize int64
31+
originalHash plumbing.Hash
2632
}
2733

2834
// newObjectToPack creates a correct ObjectToPack based on a non-delta object
@@ -71,11 +77,24 @@ func (o *ObjectToPack) WantWrite() bool {
7177
return o.Offset == 1
7278
}
7379

80+
// SetOriginal sets both Original and saves size, type and hash
81+
func (o *ObjectToPack) SetOriginal(obj plumbing.EncodedObject) {
82+
o.Original = obj
83+
o.originalSize = obj.Size()
84+
o.originalType = obj.Type()
85+
o.originalHash = obj.Hash()
86+
o.resolvedOriginal = true
87+
}
88+
7489
func (o *ObjectToPack) Type() plumbing.ObjectType {
7590
if o.Original != nil {
7691
return o.Original.Type()
7792
}
7893

94+
if o.resolvedOriginal {
95+
return o.originalType
96+
}
97+
7998
if o.Base != nil {
8099
return o.Base.Type()
81100
}
@@ -92,6 +111,10 @@ func (o *ObjectToPack) Hash() plumbing.Hash {
92111
return o.Original.Hash()
93112
}
94113

114+
if o.resolvedOriginal {
115+
return o.originalHash
116+
}
117+
95118
do, ok := o.Object.(plumbing.DeltaObject)
96119
if ok {
97120
return do.ActualHash()
@@ -105,6 +128,10 @@ func (o *ObjectToPack) Size() int64 {
105128
return o.Original.Size()
106129
}
107130

131+
if o.resolvedOriginal {
132+
return o.originalSize
133+
}
134+
108135
do, ok := o.Object.(plumbing.DeltaObject)
109136
if ok {
110137
return do.ActualSize()

0 commit comments

Comments
 (0)