Fix reflist diffs failing to compact when one of the inputs ends

The previous reflist logic would early-exit the loop body if one of the
lists was empty, but that skips the compacting logic entirely.

Instead of doing the early-exit, we can leave a list's ref as nil when
the list end is reached and then flip the comparison result, which will
essentially treat it as being greater than all others. This should
preserve the general behavior without omitting the compaction.

Signed-off-by: Ryan Gonzalez <[email protected]>

deb/reflist.go

@@ -196,31 +196,21 @@ func (l *PackageRefList) Diff(r *PackageRefList, packageCollection *PackageColle
 
 	// until we reached end of both lists
 	for il < ll || ir < lr {
-		// if we've exhausted left list, pull the rest from the right
-		if il == ll {
-			pr, err = packageCollection.ByKey(r.Refs[ir])
-			if err != nil {
-				return nil, err
-			}
-			result = append(result, PackageDiff{Left: nil, Right: pr})
-			ir++
-			continue
+		var rl, rr []byte
+		if il < ll {
+			rl = l.Refs[il]
 		}
-		// if we've exhausted right list, pull the rest from the left
-		if ir == lr {
-			pl, err = packageCollection.ByKey(l.Refs[il])
-			if err != nil {
-				return nil, err
-			}
-			result = append(result, PackageDiff{Left: pl, Right: nil})
-			il++
-			continue
+		if ir < lr {
+			rr = r.Refs[ir]
 		}
 
-		// refs on both sides are present, load them
-		rl, rr := l.Refs[il], r.Refs[ir]
 		// compare refs
 		rel := bytes.Compare(rl, rr)
+		// an unset ref is less than all others, but since it represents the end
+		// of a reflist, it should be *greater*, so flip the comparison result
+		if rl == nil || rr == nil {
+			rel = -rel
+		}
 
 		if rel == 0 {
 			// refs are identical, so are packages, advance pointer
@@ -229,14 +219,14 @@ func (l *PackageRefList) Diff(r *PackageRefList, packageCollection *PackageColle
 			pl, pr = nil, nil
 		} else {
 			// load pl & pr if they haven't been loaded before
-			if pl == nil {
+			if pl == nil && rl != nil {
 				pl, err = packageCollection.ByKey(rl)
 				if err != nil {
 					return nil, err
 				}
 			}
 
-			if pr == nil {
+			if pr == nil && rr != nil {
 				pr, err = packageCollection.ByKey(rr)
 				if err != nil {
 					return nil, err

deb/reflist_test.go

@@ -237,6 +237,41 @@ func (s *PackageRefListSuite) TestDiff(c *C) {
 
 }
 
+func (s *PackageRefListSuite) TestDiffCompactsAtEnd(c *C) {
+	db, _ := goleveldb.NewOpenDB(c.MkDir())
+	coll := NewPackageCollection(db)
+
+	packages := []*Package{
+		{Name: "app", Version: "1.1~bp1", Architecture: "i386"},  //0
+		{Name: "app", Version: "1.1~bp2", Architecture: "i386"},  //1
+		{Name: "app", Version: "1.1~bp2", Architecture: "amd64"}, //2
+	}
+
+	for _, p := range packages {
+		coll.Update(p)
+	}
+
+	listA := NewPackageList()
+	listA.Add(packages[0])
+
+	listB := NewPackageList()
+	listB.Add(packages[1])
+	listB.Add(packages[2])
+
+	reflistA := NewPackageRefListFromPackageList(listA)
+	reflistB := NewPackageRefListFromPackageList(listB)
+
+	diffAB, err := reflistA.Diff(reflistB, coll)
+	c.Check(err, IsNil)
+	c.Check(diffAB, HasLen, 2)
+
+	c.Check(diffAB[0].Left, IsNil)
+	c.Check(diffAB[0].Right.String(), Equals, "app_1.1~bp2_amd64")
+
+	c.Check(diffAB[1].Left.String(), Equals, "app_1.1~bp1_i386")
+	c.Check(diffAB[1].Right.String(), Equals, "app_1.1~bp2_i386")
+}
+
 func (s *PackageRefListSuite) TestMerge(c *C) {
 	db, _ := goleveldb.NewOpenDB(c.MkDir())
 	coll := NewPackageCollection(db)