Merge pull request #10612 from vbatts/vbatts-archive_sort

pkg/archive: sort files

archive/archive.go

@@ -458,7 +458,7 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 				}
 
 				if err := ta.addTarFile(filePath, relFilePath); err != nil {
-					log.Debugf("Can't add file %s to tar: %s", srcPath, err)
+					log.Debugf("Can't add file %s to tar: %s", filePath, err)
 				}
 				return nil
 			})

archive/changes.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"syscall"
 	"time"
@@ -43,6 +44,13 @@ func (change *Change) String() string {
 	return fmt.Sprintf("%s %s", kind, change.Path)
 }
 
+// for sort.Sort
+type changesByPath []Change
+
+func (c changesByPath) Less(i, j int) bool { return c[i].Path < c[j].Path }
+func (c changesByPath) Len() int           { return len(c) }
+func (c changesByPath) Swap(i, j int)      { c[j], c[i] = c[i], c[j] }
+
 // Gnu tar and the go tar writer don't have sub-second mtime
 // precision, which is problematic when we apply changes via tar
 // files, we handle this by comparing for exact times, *or* same
@@ -373,6 +381,8 @@ func ExportChanges(dir string, changes []Change) (Archive, error) {
 		// this buffer is needed for the duration of this piped stream
 		defer pools.BufioWriter32KPool.Put(ta.Buffer)
 
+		sort.Sort(changesByPath(changes))
+
 		// In general we log errors here but ignore them because
 		// during e.g. a diff operation the container can continue
 		// mutating the filesystem and we can see transient errors

archive/changes_posix_test.go

@@ -0,0 +1,127 @@
+package archive
+
+import (
+	"archive/tar"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path"
+	"sort"
+	"testing"
+)
+
+func TestHardLinkOrder(t *testing.T) {
+	names := []string{"file1.txt", "file2.txt", "file3.txt"}
+	msg := []byte("Hey y'all")
+
+	// Create dir
+	src, err := ioutil.TempDir("", "docker-hardlink-test-src-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	//defer os.RemoveAll(src)
+	for _, name := range names {
+		func() {
+			fh, err := os.Create(path.Join(src, name))
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer fh.Close()
+			if _, err = fh.Write(msg); err != nil {
+				t.Fatal(err)
+			}
+		}()
+	}
+	// Create dest, with changes that includes hardlinks
+	dest, err := ioutil.TempDir("", "docker-hardlink-test-dest-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	os.RemoveAll(dest) // we just want the name, at first
+	if err := copyDir(src, dest); err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dest)
+	for _, name := range names {
+		for i := 0; i < 5; i++ {
+			if err := os.Link(path.Join(dest, name), path.Join(dest, fmt.Sprintf("%s.link%d", name, i))); err != nil {
+				t.Fatal(err)
+			}
+		}
+	}
+
+	// get changes
+	changes, err := ChangesDirs(dest, src)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// sort
+	sort.Sort(changesByPath(changes))
+
+	// ExportChanges
+	ar, err := ExportChanges(dest, changes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	hdrs, err := walkHeaders(ar)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// reverse sort
+	sort.Sort(sort.Reverse(changesByPath(changes)))
+	// ExportChanges
+	arRev, err := ExportChanges(dest, changes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	hdrsRev, err := walkHeaders(arRev)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// line up the two sets
+	sort.Sort(tarHeaders(hdrs))
+	sort.Sort(tarHeaders(hdrsRev))
+
+	// compare Size and LinkName
+	for i := range hdrs {
+		if hdrs[i].Name != hdrsRev[i].Name {
+			t.Errorf("headers - expected name %q; but got %q", hdrs[i].Name, hdrsRev[i].Name)
+		}
+		if hdrs[i].Size != hdrsRev[i].Size {
+			t.Errorf("headers - %q expected size %d; but got %d", hdrs[i].Name, hdrs[i].Size, hdrsRev[i].Size)
+		}
+		if hdrs[i].Typeflag != hdrsRev[i].Typeflag {
+			t.Errorf("headers - %q expected type %d; but got %d", hdrs[i].Name, hdrs[i].Typeflag, hdrsRev[i].Typeflag)
+		}
+		if hdrs[i].Linkname != hdrsRev[i].Linkname {
+			t.Errorf("headers - %q expected linkname %q; but got %q", hdrs[i].Name, hdrs[i].Linkname, hdrsRev[i].Linkname)
+		}
+	}
+
+}
+
+type tarHeaders []tar.Header
+
+func (th tarHeaders) Len() int           { return len(th) }
+func (th tarHeaders) Swap(i, j int)      { th[j], th[i] = th[i], th[j] }
+func (th tarHeaders) Less(i, j int) bool { return th[i].Name < th[j].Name }
+
+func walkHeaders(r io.Reader) ([]tar.Header, error) {
+	t := tar.NewReader(r)
+	headers := []tar.Header{}
+	for {
+		hdr, err := t.Next()
+		if err != nil {
+			if err == io.EOF {
+				break
+			}
+			return headers, err
+		}
+		headers = append(headers, *hdr)
+	}
+	return headers, nil
+}

archive/changes_test.go

@@ -25,13 +25,6 @@ func copyDir(src, dst string) error {
 	return nil
 }
 
-// Helper to sort []Change by path
-type byPath struct{ changes []Change }
-
-func (b byPath) Less(i, j int) bool { return b.changes[i].Path < b.changes[j].Path }
-func (b byPath) Len() int           { return len(b.changes) }
-func (b byPath) Swap(i, j int)      { b.changes[i], b.changes[j] = b.changes[j], b.changes[i] }
-
 type FileType uint32
 
 const (
@@ -220,7 +213,7 @@ func TestChangesDirsMutated(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	sort.Sort(byPath{changes})
+	sort.Sort(changesByPath(changes))
 
 	expectedChanges := []Change{
 		{"/dir1", ChangeDelete},