swupd: Add deltas to the packs

Make pack creation call delta generation, so the delta packs have
deltas inside instead of only fullfiles.

To achieve this, the delta generation was modified in following ways:

- Return a list of all the deltas, not only the failed ones, this is
  useful when packing. Otherwise a combination of peeking at DeltaPeer
  and the failed list was needed.

- Simplify the goroutine logic: since we have a "home" for all the
  error values (in Delta struct), we can just feed all of the deltas
  to be processed and wait for the goroutines to finish.

- Instead of calling linkPeerAndChange, call a variant that does not
  change the versions, that allows the manifest to be used later on
  for the rest of packing.

Signed-off-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>

swupd/create_manifests.go

@@ -253,6 +253,11 @@ func CreateManifests(version uint32, minVersion uint32, format uint, statedir st
 		return nil, err
 	}
 
+	// Bootstrap delta directory, so we can assume every version will have one.
+	if err = os.MkdirAll(filepath.Join(verOutput, "delta"), 0755); err != nil {
+		return nil, err
+	}
+
 	newMoM := Manifest{
 		Name: "MoM",
 		Header: ManifestHeader{

swupd/delta.go

@@ -3,17 +3,40 @@ package swupd
 import (
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"sync"
+	"syscall"
 
 	"github.com/clearlinux/mixer-tools/helpers"
 	"github.com/pkg/errors"
 )
 
-// CreateDeltas creates all delta files between the previous and current
-// version of the supplied manifest, returning a list of files that
-// delta creation failed for.
-func CreateDeltas(manifest, statedir string, from, to uint32) ([]*File, error) {
+const (
+	// From swupd-server's include/swupd.h:
+	//
+	//     Approximately the smallest size of a pair of input files which differ by a
+	//     single bit that bsdiff can produce a more compact deltafile. Files smaller
+	//     than this are always marked as different. See the magic 200 value in the
+	//     bsdiff/src/diff.c code.
+	//
+	minimumSizeToMakeDeltaInBytes = 200
+)
+
+// Delta represents a delta file between two other files. If Error is present, it
+// indicates that the delta couldn't be created.
+type Delta struct {
+	Path  string
+	Error error
+	from  *File
+	to    *File
+}
+
+// CreateDeltas creates all delta files between the previous and current version of the
+// supplied manifest. Returns a list of deltas (which contains information about
+// individual delta errors). Returns error (and no deltas) if it can't assemble the delta
+// list.
+func CreateDeltas(manifest, statedir string, from, to uint32) ([]Delta, error) {
 	var c config
 
 	c, err := getConfig(statedir)
@@ -31,149 +54,122 @@ func CreateDeltas(manifest, statedir string, from, to uint32) ([]*File, error) {
 		return nil, err
 	}
 
-	// Must be sorted before passing into linkPeersAndChange()
-	oldManifest.sortFilesName()
-	newManifest.sortFilesName()
+	return createDeltasFromManifests(&c, oldManifest, newManifest)
+}
 
-	_, _, _ = newManifest.linkPeersAndChange(oldManifest, from)
-
-	deltas, err := consolidateDeltaFiles(newManifest, from, c)
+func createDeltasFromManifests(c *config, oldManifest, newManifest *Manifest) ([]Delta, error) {
+	deltas, err := findDeltas(c, oldManifest, newManifest)
 	if err != nil {
-		return nil, errors.Wrapf(err, "Failed to create deltas list %s", manifest)
+		return nil, errors.Wrapf(err, "Failed to create deltas list %s", newManifest.Name)
 	}
 
+	if len(deltas) == 0 {
+		return []Delta{}, nil
+	}
+
+	// TODO: Unify these maxRoutines constants, get a better default (maybe NumCPUs)
+	// and make it configurable.
 	const maxRoutines = 3
-	var deltaQueue = make(chan *File)
-	var failedChan = make(chan *File)
-	var failedList = make(chan []*File)
+	var deltaQueue = make(chan *Delta)
 	var wg sync.WaitGroup
 	wg.Add(maxRoutines)
 
-	// Start a collector for the failure cases
-	go func() {
-		var failed []*File
-		// Drains the failedChan appropriately
-		for f := range failedChan {
-			failed = append(failed, f)
-		}
-		failedList <- failed // This list is what we return in the end
-	}()
-
-	// Don't flood the system with goroutines, delta creation takes up an
-	// incredibly amount of memory, so we cannot max out on goroutines
+	// Delta creation takes a lot of memory, so create a limited amount of goroutines.
 	for i := 0; i < maxRoutines; i++ {
-		go func(statedir string) {
+		go func() {
 			defer wg.Done()
-
-			// Take in jobs from the queue and try to make a delta
-			for f := range deltaQueue {
-				err := createDelta(c, f, statedir, f.DeltaPeer.Version, f.DeltaPeer.Hash)
-				if err != nil {
-					failedChan <- f
-				}
+			for delta := range deltaQueue {
+				delta.Error = createDelta(c, delta)
 			}
-		}(statedir)
+		}()
 	}
 
-	// Send jobs to the queue for delta goroutines to pick up
-	for _, file := range deltas {
-		deltaQueue <- file
+	// Send jobs to the queue for delta goroutines to pick up.
+	for i := range deltas {
+		deltaQueue <- &deltas[i]
 	}
 
 	// Send message that no more jobs are being sent
 	close(deltaQueue)
 	wg.Wait()
 
-	// Once wait finishes we can signal the collector that no more jobs exist
-	close(failedChan)
-
-	return <-failedList, nil
+	return deltas, nil
 }
 
-func consolidateDeltaFiles(manifest *Manifest, from uint32, c config) ([]*File, error) {
-	var files []*File
+func createDelta(c *config, delta *Delta) error {
+	oldPath := filepath.Join(c.imageBase, fmt.Sprint(delta.from.Version), "full", delta.from.Name)
+	newPath := filepath.Join(c.imageBase, fmt.Sprint(delta.to.Version), "full", delta.to.Name)
+	dir := filepath.Join(c.outputDir, fmt.Sprint(delta.to.Version), "delta")
+	name := fmt.Sprintf("%d-%d-%s-%s", delta.from.Version, delta.to.Version, delta.from.Hash, delta.to.Hash)
+	delta.Path = filepath.Join(dir, name)
 
-	if manifest == nil {
-		return nil, nil
+	if _, err := os.Stat(delta.Path); err == nil {
+		// Skip existing deltas. Not verifying since client is resilient about that.
+		return nil
 	}
 
-	for _, file := range manifest.Files {
-		if file.Version <= from {
-			continue
-		}
-		if file.DeltaPeer == nil {
-			continue
-		}
-		if file.Type != TypeFile ||
-			file.DeltaPeer.Type != TypeFile {
-			continue
-		}
-
-		deltadir := filepath.Join(c.outputDir, fmt.Sprintf("%d", file.Version), "delta")
-		fromToString := fmt.Sprintf("%d-%d-%s-%s", file.DeltaPeer.Version, file.Version, file.DeltaPeer.Hash, file.Hash)
-
-		deltaFile := filepath.Join(deltadir, fromToString)
-
-		// Only add to list if the delta file does not already exist on the system
-		if _, err := os.Stat(deltaFile); os.IsNotExist(err) {
-			// Only create deltas for files bigger than 200 bytes
-			fullname := filepath.Join(c.imageBase, fmt.Sprintf("%d", file.Version), "full", file.Name)
-			file.Info, _ = os.Stat(fullname)
-			if file.Info.Size() > 200 {
-				files = append(files, file)
+	if err := helpers.RunCommandSilent("bsdiff", oldPath, newPath, delta.Path); err != nil {
+		_ = os.Remove(delta.Path)
+		if exitErr, ok := errors.Cause(err).(*exec.ExitError); ok {
+			// bsdiff returns 1 that stands for "FULLDL", i.e. it decided that
+			// a delta is not worth. Give a better error message for that case.
+			if status, ok := exitErr.Sys().(syscall.WaitStatus); ok {
+				if status == 1 {
+					return fmt.Errorf("bsdiff returned FULLDL, not using delta")
+				}
 			}
 		}
-
+		return errors.Wrapf(err, "Failed to create delta for %s (%d) -> %s (%d)", delta.from.Name, delta.from.Version, delta.to.Name, delta.to.Version)
 	}
 
-	return files, nil
-}
-
-func createDelta(c config, file *File, statedir string, fromVersion uint32, fromHash Hashval) error {
-	// File without peers cannot have deltas
-	if file.DeltaPeer == nil {
-		return nil
+	// Check that the delta actually applies correctly.
+	testPath := delta.Path + ".testnewfile"
+	if err := helpers.RunCommandSilent("bspatch", oldPath, testPath, delta.Path); err != nil {
+		return errors.Wrapf(err, "Failed to apply delta %s", delta.Path)
 	}
+	defer func() {
+		_ = os.Remove(testPath)
+	}()
 
-	// We only support deltas between two regular files for now
-	// TODO: Support directory -> file in the future, this is a complex case
-	if file.Type != TypeFile || file.DeltaPeer.Type != TypeFile {
-		return nil
-	}
-
-	newfile := filepath.Join(c.imageBase, fmt.Sprintf("%d", file.Version), "full", file.Name)
-	original := filepath.Join(c.imageBase, fmt.Sprintf("%d", fromVersion), "full", file.Name)
-
-	deltadir := filepath.Join(c.outputDir, fmt.Sprintf("%d", file.Version), "delta")
-	fromToString := fmt.Sprintf("%d-%d-%s-%s", fromVersion, file.Version, fromHash, file.Hash)
-
-	// Files to create and validate deltas with
-	deltafile := filepath.Join(deltadir, fromToString)
-	testnewfile := filepath.Join(deltadir, "."+fromToString+".testnewfile")
-
-	// Shell out to bsdiff for now...false = don't print to screen
-	if err := helpers.RunCommandSilent("bsdiff", original, newfile, deltafile); err != nil {
-		_ = os.Remove(deltafile) // Might have returned FULLDL
-		return errors.Wrapf(err, "Failed to create delta for %s -> %s", original, newfile)
-	}
-
-	// Check that the delta actually applies correctly
-	if err := helpers.RunCommandSilent("bspatch", original, testnewfile, deltafile); err != nil {
-		return errors.Wrapf(err, "Failed to apply delta %s", deltafile)
-	}
-
-	deltahash, err := Hashcalc(testnewfile)
+	testHash, err := Hashcalc(testPath)
 	if err != nil {
-		return errors.Wrap(err, "Failed to calculate hash for new file")
+		_ = os.Remove(delta.Path)
+		return errors.Wrap(err, "Failed to calculate hash for test file created applying delta")
 	}
-
-	if !HashEquals(deltahash, file.Hash) {
-		return errors.Wrapf(err, "Delta mismatch: %s -> %s via delta: %s", original, newfile, deltafile)
+	if testHash != delta.to.Hash {
+		_ = os.Remove(delta.Path)
+		return errors.Wrapf(err, "Delta mismatch: %s -> %s via delta: %s", oldPath, newPath, delta.Path)
 	}
 
-	_ = os.Remove(testnewfile)
-
-	// add rename code here
-
 	return nil
 }
+
+func findDeltas(c *config, oldManifest, newManifest *Manifest) ([]Delta, error) {
+	oldManifest.sortFilesName()
+	newManifest.sortFilesName()
+
+	err := linkDeltaPeers(c, oldManifest, newManifest)
+	if err != nil {
+		return nil, err
+	}
+
+	deltaCount := 0
+	for _, nf := range newManifest.Files {
+		if nf.DeltaPeer != nil {
+			deltaCount++
+		}
+	}
+
+	deltas := make([]Delta, 0, deltaCount)
+	for _, nf := range newManifest.Files {
+		if nf.DeltaPeer == nil {
+			continue
+		}
+		deltas = append(deltas, Delta{
+			from: nf.DeltaPeer,
+			to:   nf,
+		})
+	}
+
+	return deltas, nil
+}

swupd/delta_test.go

@@ -21,6 +21,6 @@ func TestCreateDeltas(t *testing.T) {
 	mustCreateManifestsStandard(t, 20, testDir)
 	mustMkdir(t, filepath.Join(testDir, "www/20/delta"))
 
-	mustCreateDeltas(t, "Manifest.full", testDir, 10, 20)
+	mustCreateAllDeltas(t, "Manifest.full", testDir, 10, 20)
 	mustExistDelta(t, testDir, "/bar", 10, 20)
 }

swupd/helpers_test.go

@@ -174,7 +174,7 @@ func resetHash() {
 }
 
 func mustMkdir(t *testing.T, name string) {
-	err := os.Mkdir(name, 0755)
+	err := os.MkdirAll(name, 0755)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -275,16 +275,20 @@ func checkManifestMatches(t *testing.T, testDir, ver, name string, res ...*regex
 	}
 }
 
-func mustCreateDeltas(t *testing.T, manifest, statedir string, from, to uint32) {
-	failed, err := CreateDeltas(manifest, statedir, from, to)
+func mustCreateAllDeltas(t *testing.T, manifest, statedir string, from, to uint32) {
+	deltas, err := CreateDeltas(manifest, statedir, from, to)
 	if err != nil {
-		if len(failed) > 0 {
-			t.Fatalf("%s: \n%d deltas did not get created for %s: %v", err, len(failed), manifest, failed)
-			return
+		t.Fatalf("couldn't create deltas for %s: %s", manifest, err)
+	}
+
+	for _, d := range deltas {
+		if d.Error != nil {
+			t.Errorf("couldn't create delta for %s %d -> %s %d: %s", d.from.Name, d.from.Version, d.to.Name, d.to.Version, err)
 		}
-		t.Fatal(err)
-	} else if len(failed) > 0 {
-		t.Fatalf("CreateDeltas succeeded but %d deltas did not get created for %s: %v", len(failed), manifest, failed)
+	}
+
+	if t.Failed() {
+		t.Fatalf("couldn't create all deltas due to errors above")
 	}
 }
 

swupd/manifest.go

@@ -16,7 +16,6 @@ package swupd
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -27,6 +26,8 @@ import (
 	"strings"
 	"text/template"
 	"time"
+
+	"github.com/pkg/errors"
 )
 
 const manifestFieldDelim = "\t"
@@ -474,6 +475,68 @@ func (m *Manifest) newDeleted(df *File) {
 	m.Files = append(m.Files, df)
 }
 
+// linkDeltaPeersForPack sets the DeltaPeer of the files in newManifest that have the corresponding files
+// in oldManifest.
+func linkDeltaPeersForPack(c *config, oldManifest, newManifest *Manifest) error {
+	newIndex := 0
+	oldIndex := 0
+
+	for newIndex < len(newManifest.Files) && oldIndex < len(oldManifest.Files) {
+		nf := newManifest.Files[newIndex]
+		of := oldManifest.Files[oldIndex]
+
+		switch {
+		case nf.Name < of.Name:
+			// Old file that is not in new manifest, no chance of delta.
+			newIndex++
+
+		case nf.Name > of.Name:
+			// New file that wasn't present before, no chance of delta.
+			oldIndex++
+
+		default:
+			newIndex++
+			oldIndex++
+
+			// Matching names, we can have a delta if pass all the
+			// requirements below.
+			if nf.Version <= of.Version {
+				continue
+			}
+
+			if nf.Hash == of.Hash {
+				continue
+			}
+
+			if !nf.Present() || !nf.Present() {
+				continue
+			}
+
+			if nf.Type != TypeFile || nf.Type != of.Type {
+				continue
+			}
+
+			// Check file size to decide whether it should have a delta.
+			newPath := filepath.Join(c.imageBase, fmt.Sprint(nf.Version), "full", nf.Name)
+			fi, err := os.Stat(newPath)
+			if err != nil {
+				return errors.Wrapf(err, "error accessing %s to decide whether it can have a delta or not")
+			}
+			if fi.Size() < minimumSizeToMakeDeltaInBytes {
+				continue
+			}
+
+			nf.DeltaPeer = of
+			of.DeltaPeer = nf
+		}
+	}
+
+	// TODO: Once rename logic is in. Add code to identify what extra DeltaPeers
+	// should be created based on rename flag in newManifest.
+
+	return nil
+}
+
 func (m *Manifest) readIncludes(bundles []*Manifest, c config) error {
 	// read in <imageBase>/<version>/noship/<bundle>-includes
 	var err error

swupd/packs.go

@@ -60,6 +60,23 @@ type PackInfo struct {
 	Warnings []string
 }
 
+// Empty tells if the pack has contents or not. Packs without contents are not generated.
+func (info PackInfo) Empty() bool {
+	return info.FullfileCount == 0 && info.DeltaCount == 0
+}
+
+func (state PackState) String() string {
+	switch state {
+	case NotPacked:
+		return "not packed"
+	case PackedDelta:
+		return "packed delta"
+	case PackedFullfile:
+		return "packed fullfile"
+	}
+	return "invalid"
+}
+
 // WritePack writes the pack between two Manifests, or a zero pack if fromManifest is
 // nil. The toManifest should always be non nil. The outputDir is used to pick deltas and
 // fullfiles. If not empty, chrootDir is tried first as a fast alternative to
@@ -74,11 +91,35 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 	toVersion := toManifest.Header.Version
 
 	var fromVersion uint32
+	var deltas []Delta
 	if fromManifest != nil {
 		fromVersion = fromManifest.Header.Version
 		if fromVersion >= toVersion {
 			return nil, fmt.Errorf("fromManifest version (%d) must be smaller than toManifest version (%d)", fromVersion, toVersion)
 		}
+
+	}
+
+	if debugPacks {
+		log.Printf("DEBUG: WritePack for bundle %s from %d to %d", toManifest.Name, fromVersion, toVersion)
+	}
+
+	if fromManifest != nil {
+		// TODO: Make WritePack itself take a Config.
+		var c config
+		c, err = getConfig(filepath.Join(outputDir, ".."))
+		if err != nil {
+			return nil, err
+		}
+
+		deltas, err = createDeltasFromManifests(&c, fromManifest, toManifest)
+		if err != nil {
+			return nil, err
+		}
+
+		if debugPacks {
+			log.Printf("DEBUG: %d potential deltas to use in pack", len(deltas))
+		}
 	}
 
 	if debugPacks {
@@ -89,7 +130,6 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 		}
 	}
 
-	// TODO: Should preallocate capacity.
 	info = &PackInfo{
 		Entries: make([]PackEntry, len(toManifest.Files)),
 	}
@@ -107,6 +147,10 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 		hashesInFrom = make(map[Hashval]bool)
 	}
 
+	if debugPacks {
+		log.Printf("DEBUG: %d hashes already in from manifest", len(hashesInFrom))
+	}
+
 	xw, err := NewExternalWriter(w, "xz")
 	if err != nil {
 		return nil, err
@@ -142,6 +186,30 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 		fullChrootDir = filepath.Join(chrootDir, fmt.Sprint(toVersion), "full")
 	}
 
+	// Add all deltas that have not failed.
+	hasDelta := make(map[Hashval]bool)
+	for i := range deltas {
+		d := &deltas[i]
+		if d.Error != nil {
+			info.Warnings = append(info.Warnings, d.Error.Error())
+			continue
+		}
+		var fallback bool
+		fallback, err = copyFromDelta(tw, d)
+		if err != nil {
+			// If copy from delta fails before writing to the pack, we can
+			// fallback to use the fullfile later.
+			if fallback {
+				info.Warnings = append(info.Warnings, err.Error())
+				continue
+			}
+			return nil, err
+		}
+
+		info.DeltaCount++
+		hasDelta[d.to.Hash] = true
+	}
+
 	done := make(map[Hashval]bool)
 	for i, f := range toManifest.Files {
 		entry := &info.Entries[i]
@@ -151,6 +219,10 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 			entry.Reason = "already in from manifest"
 			continue
 		}
+		if hasDelta[f.Hash] {
+			entry.State = PackedDelta
+			continue
+		}
 		if done[f.Hash] {
 			entry.Reason = "hash already packed"
 			continue
@@ -166,7 +238,6 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 
 		done[f.Hash] = true
 
-		// TODO: Pack deltas when available.
 		entry.State = PackedFullfile
 		entry.Reason = "from fullfile"
 		info.FullfileCount++
@@ -174,6 +245,8 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 			var fallback bool
 			fallback, err = copyFromFullChrootFile(tw, fullChrootDir, f)
 			if (err != nil) && fallback {
+				// If copy from chroot file fails before writing to the pack, we can
+				// fallback to try copying from the fullfile.
 				info.Warnings = append(info.Warnings, err.Error())
 				err = copyFromFullfile(tw, outputDir, f)
 			} else {
@@ -187,6 +260,10 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 		}
 	}
 
+	if debugPacks {
+		log.Printf("DEBUG: pack created with %d fullfiles and %d deltas", info.FullfileCount, info.DeltaCount)
+	}
+
 	err = tw.Close()
 	if err != nil {
 		return nil, err
@@ -194,6 +271,42 @@ func WritePack(w io.Writer, fromManifest, toManifest *Manifest, outputDir, chroo
 	return info, nil
 }
 
+func copyFromDelta(tw *tar.Writer, delta *Delta) (fallback bool, err error) {
+	f, err := os.Open(delta.Path)
+	if err != nil {
+		return true, err
+	}
+	defer func() {
+		_ = f.Close()
+	}()
+	fi, err := f.Stat()
+	if err != nil {
+		return true, err
+	}
+	if !fi.Mode().IsRegular() {
+		return true, fmt.Errorf("delta %s is not a regular file", delta.Path)
+	}
+	hdr, err := getHeaderFromFileInfo(fi)
+	if err != nil {
+		return true, err
+	}
+	hdr.Name = "delta/" + fi.Name()
+	hdr.Typeflag = tar.TypeReg
+
+	// After we start writing on the tar writer, we can't let the caller fallback to another
+	// option anymore.
+
+	err = tw.WriteHeader(hdr)
+	if err != nil {
+		return false, err
+	}
+	_, err = io.Copy(tw, f)
+	if err != nil {
+		return false, err
+	}
+	return false, nil
+}
+
 func copyFromFullChrootFile(tw *tar.Writer, fullChrootDir string, f *File) (fallback bool, err error) {
 	realname := filepath.Join(fullChrootDir, f.Name)
 	fi, err := os.Lstat(realname)
@@ -428,6 +541,7 @@ func FindBundlesToPack(from *Manifest, to *Manifest) (map[string]*BundleToPack,
 
 // CreatePack creates the pack file for a specific bundle between two versions. The pack is written
 // in the TO version subdirectory of outputDir (e.g. a pack from 10 to 20 is written to "www/20").
+// Empty packs will lead to not creating the pack.
 func CreatePack(name string, fromVersion, toVersion uint32, outputDir, chrootDir string) (*PackInfo, error) {
 	toDir := filepath.Join(outputDir, fmt.Sprint(toVersion))
 	toM, err := ParseManifestFile(filepath.Join(toDir, "Manifest."+name))
@@ -450,6 +564,7 @@ func CreatePack(name string, fromVersion, toVersion uint32, outputDir, chrootDir
 	}
 	info, err := WritePack(output, fromM, toM, outputDir, chrootDir)
 	if err != nil {
+		_ = output.Close()
 		_ = os.RemoveAll(packPath)
 		return nil, err
 	}
@@ -459,5 +574,11 @@ func CreatePack(name string, fromVersion, toVersion uint32, outputDir, chrootDir
 		return nil, err
 	}
 
+	if info.Empty() {
+		// Don't bother leaving empty packs around. Not failing if remove fails since an
+		// empty pack is not incorrect.
+		_ = os.Remove(packPath)
+	}
+
 	return info, nil
 }

swupd/packs_test.go

@@ -113,10 +113,13 @@ func TestFindBundlesToPack(t *testing.T) {
 		m.Files = append(m.Files, bundle)
 	}
 
-	sortAndPrintBundles := func(bundles []BundleToPack) {
+	sortBundles := func(bundles []BundleToPack) {
 		sort.Slice(bundles, func(i, j int) bool {
 			return bundles[i].Name < bundles[j].Name
 		})
+	}
+
+	printBundles := func(bundles []BundleToPack) {
 		for _, b := range bundles {
 			fmt.Printf("  %s %d -> %d\n", b.Name, b.FromVersion, b.ToVersion)
 		}
@@ -158,14 +161,17 @@ func TestFindBundlesToPack(t *testing.T) {
 			bundles = append(bundles, *b)
 		}
 
-		fmt.Printf("== CASE: %s\n", tt.Name)
-		fmt.Printf("ACTUAL OUTPUT (%d bundles):\n", len(bundles))
-		sortAndPrintBundles(bundles)
-
-		fmt.Printf("EXPECTED OUTPUT (%d bundles):\n", len(tt.Expected))
-		sortAndPrintBundles(tt.Expected)
+		sortBundles(bundles)
+		sortBundles(tt.Expected)
 
 		if !reflect.DeepEqual(bundles, tt.Expected) {
+			fmt.Printf("== CASE: %s\n", tt.Name)
+			fmt.Printf("ACTUAL OUTPUT (%d bundles):\n", len(bundles))
+			printBundles(bundles)
+
+			fmt.Printf("EXPECTED OUTPUT (%d bundles):\n", len(tt.Expected))
+			printBundles(tt.Expected)
+
 			t.Fatalf("mismatch between returned bundles to pack and expected bundles to pack in case %q", tt.Name)
 			continue
 		}
@@ -248,6 +254,63 @@ func TestCreatePackZeroPacks(t *testing.T) {
 	mustValidateZeroPack(t, ts.path("www/20/Manifest.shells"), ts.path("www/20/pack-shells-from-0.tar"))
 }
 
+func TestCreatePackWithDelta(t *testing.T) {
+	fs := newTestFileSystem(t, "create-pack-")
+	defer fs.cleanup()
+
+	const (
+		format = 1
+		minVer = 0
+	)
+
+	//
+	// In version 10, create a bundle with files of different sizes.
+	//
+	emptyContents := ""
+	smallContents := "small"
+	largeContents := strings.Repeat("large", 1000)
+	if len(emptyContents) >= minimumSizeToMakeDeltaInBytes || len(smallContents) >= minimumSizeToMakeDeltaInBytes || len(largeContents) < minimumSizeToMakeDeltaInBytes {
+		t.Fatal("test contents sizes are invalid")
+	}
+
+	mustInitStandardTest(t, fs.Dir, "0", "10", []string{"contents"})
+	fs.write("image/10/contents/small1", emptyContents)
+	fs.write("image/10/contents/small2", smallContents)
+	fs.write("image/10/contents/large1", largeContents)
+	fs.write("image/10/contents/large2", largeContents)
+	mustCreateManifests(t, 10, minVer, format, fs.Dir)
+
+	//
+	// In version 20, swap the content of small files, and modify the large files
+	// changing one byte or all bytes.
+	//
+	mustInitStandardTest(t, fs.Dir, "10", "20", []string{"contents"})
+	fs.write("image/20/contents/small1", smallContents)
+	fs.write("image/20/contents/small2", smallContents)
+	fs.write("image/20/contents/large1", strings.ToUpper(largeContents[:1])+largeContents[1:])
+	fs.write("image/20/contents/large2", largeContents[:1]+strings.ToUpper(largeContents[1:]))
+	mustCreateManifests(t, 20, minVer, format, fs.Dir)
+
+	info := mustCreatePack(t, "contents", 10, 20, fs.path("www"), fs.path("image"))
+	mustHaveDeltaCount(t, info, 2)
+
+	//
+	// In version 30, make a change to one large files from 20.
+	//
+	mustInitStandardTest(t, fs.Dir, "20", "30", []string{"contents"})
+	fs.cp("image/20/contents", "image/30")
+	fs.write("image/30/contents/large1", strings.ToUpper(largeContents[:2])+largeContents[2:])
+	mustCreateManifests(t, 30, minVer, format, fs.Dir)
+
+	// Pack between 20 and 30 has only a delta for large1.
+	info = mustCreatePack(t, "contents", 20, 30, fs.path("www"), fs.path("image"))
+	mustHaveDeltaCount(t, info, 1)
+
+	// Pack between 10 and 30 has both deltas.
+	info = mustCreatePack(t, "contents", 10, 30, fs.path("www"), fs.path("image"))
+	mustHaveDeltaCount(t, info, 2)
+}
+
 func TestCreatePackWithIncompleteChrootDir(t *testing.T) {
 	fs := newTestFileSystem(t, "create-pack-")
 	defer fs.cleanup()
@@ -452,6 +515,7 @@ func mustCreatePack(t *testing.T, name string, fromVersion, toVersion uint32, ou
 func mustHaveFullfileCount(t *testing.T, info *PackInfo, expected uint64) {
 	t.Helper()
 	if info.FullfileCount != expected {
+		printPackInfo(info)
 		t.Fatalf("pack has %d fullfiles but expected %d", info.FullfileCount, expected)
 	}
 }
@@ -459,6 +523,7 @@ func mustHaveFullfileCount(t *testing.T, info *PackInfo, expected uint64) {
 func mustHaveDeltaCount(t *testing.T, info *PackInfo, expected uint64) {
 	t.Helper()
 	if info.DeltaCount != expected {
+		printPackInfo(info)
 		t.Fatalf("pack has %d deltas but expected %d", info.DeltaCount, expected)
 	}
 }
@@ -466,6 +531,7 @@ func mustHaveDeltaCount(t *testing.T, info *PackInfo, expected uint64) {
 func mustHaveNoWarnings(t *testing.T, info *PackInfo) {
 	t.Helper()
 	if len(info.Warnings) > 0 {
+		printPackInfo(info)
 		t.Fatalf("unexpected warnings in pack: %s", strings.Join(info.Warnings, "\n"))
 	}
 }
@@ -477,3 +543,16 @@ func mustCreateFullfiles(t *testing.T, m *Manifest, chrootDir, outputDir string)
 		t.Fatalf("couldn't create fullfiles: %s", err)
 	}
 }
+
+func printPackInfo(info *PackInfo) {
+	fmt.Printf("WARNINGS (%d)\n", len(info.Warnings))
+	for _, w := range info.Warnings {
+		fmt.Println(w)
+	}
+	fmt.Println()
+	fmt.Printf("ENTRIES (%d)\n", len(info.Entries))
+	for _, e := range info.Entries {
+		fmt.Printf("  %-40s %s (%s)\n", e.File.Name, e.State, e.Reason)
+	}
+	fmt.Println()
+}