diff --git a/archive/archive.go b/archive/archive.go index 68e5c1d..d9fcead 100644 --- a/archive/archive.go +++ b/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 }) diff --git a/archive/changes.go b/archive/changes.go index 85217f6..9f16b76 100644 --- a/archive/changes.go +++ b/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 diff --git a/archive/changes_posix_test.go b/archive/changes_posix_test.go new file mode 100644 index 0000000..9d528e6 --- /dev/null +++ b/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 +} diff --git a/archive/changes_test.go b/archive/changes_test.go index 6b8f235..8f32d7b 100644 --- a/archive/changes_test.go +++ b/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},