From d01870e56dadca4175ca812edb228302f7cbb4e7 Mon Sep 17 00:00:00 2001
From: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Date: Fri, 10 Nov 2023 10:23:42 -0600
Subject: [PATCH] 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 <ryan.gonzalez@collabora.com>
---
 deb/reflist.go      | 34 ++++++++++++----------------------
 deb/reflist_test.go | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/deb/reflist.go b/deb/reflist.go
index 25cb0b6e..30396548 100644
--- a/deb/reflist.go
+++ b/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
diff --git a/deb/reflist_test.go b/deb/reflist_test.go
index d0ce21f5..ec7ed09f 100644
--- a/deb/reflist_test.go
+++ b/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)