diff --git a/archive/tar.go b/archive/tar.go index fa9601c69..99cfc35fc 100644 --- a/archive/tar.go +++ b/archive/tar.go @@ -395,6 +395,7 @@ type changeWriter struct { whiteoutT time.Time inodeSrc map[uint64]string inodeRefs map[uint64][]string + addedDirs map[string]struct{} } func newChangeWriter(w io.Writer, source string) *changeWriter { @@ -404,6 +405,7 @@ func newChangeWriter(w io.Writer, source string) *changeWriter { whiteoutT: time.Now(), inodeSrc: map[uint64]string{}, inodeRefs: map[uint64][]string{}, + addedDirs: map[string]struct{}{}, } } @@ -416,6 +418,7 @@ func (cw *changeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e whiteOutBase := filepath.Base(p) whiteOut := filepath.Join(whiteOutDir, whiteoutPrefix+whiteOutBase) hdr := &tar.Header{ + Typeflag: tar.TypeReg, Name: whiteOut[1:], Size: 0, ModTime: cw.whiteoutT, @@ -485,10 +488,8 @@ func (cw *changeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e additionalLinks = cw.inodeRefs[inode] delete(cw.inodeRefs, inode) } - } else if k == fs.ChangeKindUnmodified && !f.IsDir() { + } else if k == fs.ChangeKindUnmodified { // Nothing to write to diff - // Unmodified directories should still be written to keep - // directory permissions correct on direct unpack return nil } @@ -501,6 +502,9 @@ func (cw *changeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e hdr.PAXRecords[paxSchilyXattr+"security.capability"] = string(capability) } + if err := cw.includeParents(hdr); err != nil { + return err + } if err := cw.tw.WriteHeader(hdr); err != nil { return errors.Wrap(err, "failed to write file header") } @@ -530,6 +534,10 @@ func (cw *changeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e hdr.Typeflag = tar.TypeLink hdr.Linkname = source hdr.Size = 0 + + if err := cw.includeParents(hdr); err != nil { + return err + } if err := cw.tw.WriteHeader(hdr); err != nil { return errors.Wrap(err, "failed to write file header") } @@ -546,6 +554,29 @@ func (cw *changeWriter) Close() error { return nil } +func (cw *changeWriter) includeParents(hdr *tar.Header) error { + name := strings.TrimRight(hdr.Name, "/") + fname := filepath.Join(cw.source, name) + parent := filepath.Dir(name) + pname := filepath.Join(cw.source, parent) + + // Do not include root directory as parent + if fname != cw.source && pname != cw.source { + _, ok := cw.addedDirs[parent] + if !ok { + cw.addedDirs[parent] = struct{}{} + fi, err := os.Stat(pname) + if err != nil { + return err + } + if err := cw.HandleChange(fs.ChangeKindModify, parent, fi, nil); err != nil { + return err + } + } + } + return nil +} + func copyBuffered(ctx context.Context, dst io.Writer, src io.Reader) (written int64, err error) { buf := bufferPool.Get().(*[]byte) defer bufferPool.Put(buf) diff --git a/archive/tar_test.go b/archive/tar_test.go index 276a47d00..d5e10a6c6 100644 --- a/archive/tar_test.go +++ b/archive/tar_test.go @@ -698,6 +698,44 @@ func TestDiffTar(t *testing.T) { fstest.CreateFile("/d2/f", []byte("ok"), 0644), ), }, + { + name: "HardlinkParentInclusion", + validators: []tarEntryValidator{ + dirEntry("d2/", 0755), + fileEntry("d2/l1", []byte("link me"), 0644), + // d1/f1 and its parent is included after the new link, + // before the new link was included, these files would + // not habe needed + dirEntry("d1/", 0755), + linkEntry("d1/f1", "d2/l1"), + dirEntry("d3/", 0755), + fileEntry("d3/l1", []byte("link me"), 0644), + dirEntry("d4/", 0755), + linkEntry("d4/f1", "d3/l1"), + whiteoutEntry("d6/l1"), + whiteoutEntry("d6/l2"), + }, + a: fstest.Apply( + fstest.CreateDir("/d1/", 0755), + fstest.CreateFile("/d1/f1", []byte("link me"), 0644), + fstest.CreateDir("/d2/", 0755), + fstest.CreateFile("/d2/f1", []byte("link me"), 0644), + fstest.CreateDir("/d3/", 0755), + fstest.CreateDir("/d4/", 0755), + fstest.CreateFile("/d4/f1", []byte("link me"), 0644), + fstest.CreateDir("/d5/", 0755), + fstest.CreateFile("/d5/f1", []byte("link me"), 0644), + fstest.CreateDir("/d6/", 0755), + fstest.Link("/d1/f1", "/d6/l1"), + fstest.Link("/d5/f1", "/d6/l2"), + ), + b: fstest.Apply( + fstest.Link("/d1/f1", "/d2/l1"), + fstest.Link("/d4/f1", "/d3/l1"), + fstest.Remove("/d6/l1"), + fstest.Remove("/d6/l2"), + ), + }, } for _, at := range tests { @@ -740,6 +778,37 @@ func fileEntry(name string, expected []byte, mode int) tarEntryValidator { } } +func linkEntry(name, link string) tarEntryValidator { + return func(hdr *tar.Header, b []byte) error { + if hdr.Typeflag != tar.TypeLink { + return errors.New("not link type") + } + if hdr.Name != name { + return errors.Errorf("wrong name %q, expected %q", hdr.Name, name) + } + if hdr.Linkname != link { + return errors.Errorf("wrong link %q, expected %q", hdr.Linkname, link) + } + return nil + } +} + +func whiteoutEntry(name string) tarEntryValidator { + whiteOutDir := filepath.Dir(name) + whiteOutBase := filepath.Base(name) + whiteOut := filepath.Join(whiteOutDir, whiteoutPrefix+whiteOutBase) + + return func(hdr *tar.Header, b []byte) error { + if hdr.Typeflag != tar.TypeReg { + return errors.Errorf("not file type: %q", hdr.Typeflag) + } + if hdr.Name != whiteOut { + return errors.Errorf("wrong name %q, expected whiteout %q", hdr.Name, name) + } + return nil + } +} + func makeDiffTarTest(validators []tarEntryValidator, a, b fstest.Applier) func(*testing.T) { return func(t *testing.T) { ad, err := ioutil.TempDir("", "test-make-diff-tar-") diff --git a/fs/diff.go b/fs/diff.go index 3a53f4215..9073d0d92 100644 --- a/fs/diff.go +++ b/fs/diff.go @@ -222,10 +222,8 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err c1 = make(chan *currentPath) c2 = make(chan *currentPath) - f1, f2 *currentPath - rmdir string - lastEmittedDir = string(filepath.Separator) - parents []os.FileInfo + f1, f2 *currentPath + rmdir string ) g.Go(func() error { defer close(c1) @@ -260,10 +258,7 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err continue } - var ( - f os.FileInfo - emit = true - ) + var f os.FileInfo k, p := pathChange(f1, f2) switch k { case ChangeKindAdd: @@ -299,35 +294,13 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err f2 = nil if same { if !isLinked(f) { - emit = false + continue } k = ChangeKindUnmodified } } - if emit { - emittedDir, emitParents := commonParents(lastEmittedDir, p, parents) - for _, pf := range emitParents { - p := filepath.Join(emittedDir, pf.Name()) - if err := changeFn(ChangeKindUnmodified, p, pf, nil); err != nil { - return err - } - emittedDir = p - } - - if err := changeFn(k, p, f, nil); err != nil { - return err - } - - if f != nil && f.IsDir() { - lastEmittedDir = p - } else { - lastEmittedDir = emittedDir - } - - parents = parents[:0] - } else if f.IsDir() { - lastEmittedDir, parents = commonParents(lastEmittedDir, p, parents) - parents = append(parents, f) + if err := changeFn(k, p, f, nil); err != nil { + return err } } return nil @@ -335,47 +308,3 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err return g.Wait() } - -func commonParents(base, updated string, dirs []os.FileInfo) (string, []os.FileInfo) { - if basePrefix := makePrefix(base); strings.HasPrefix(updated, basePrefix) { - var ( - parents []os.FileInfo - last = base - ) - for _, d := range dirs { - next := filepath.Join(last, d.Name()) - if strings.HasPrefix(updated, makePrefix(last)) { - parents = append(parents, d) - last = next - } else { - break - } - } - return base, parents - } - - baseS := strings.Split(base, string(filepath.Separator)) - updatedS := strings.Split(updated, string(filepath.Separator)) - commonS := []string{string(filepath.Separator)} - - min := len(baseS) - if len(updatedS) < min { - min = len(updatedS) - } - for i := 0; i < min; i++ { - if baseS[i] == updatedS[i] { - commonS = append(commonS, baseS[i]) - } else { - break - } - } - - return filepath.Join(commonS...), []os.FileInfo{} -} - -func makePrefix(d string) string { - if d == "" || d[len(d)-1] != filepath.Separator { - return d + string(filepath.Separator) - } - return d -} diff --git a/fs/diff_test.go b/fs/diff_test.go index 004caac7e..f3b2d9763 100644 --- a/fs/diff_test.go +++ b/fs/diff_test.go @@ -44,7 +44,6 @@ func TestSimpleDiff(t *testing.T) { fstest.Remove("/etc/unexpected"), ) diff := []TestChange{ - Unchanged("/etc"), Modify("/etc/hosts"), Modify("/etc/profile"), Delete("/etc/unexpected"), @@ -71,7 +70,6 @@ func TestDirectoryReplace(t *testing.T) { fstest.CreateFile("/dir1/f2", []byte("Now file"), 0666), ) diff := []TestChange{ - Unchanged("/dir1"), Add("/dir1/f11"), Modify("/dir1/f2"), } @@ -134,13 +132,10 @@ func TestParentDirectoryPermission(t *testing.T) { fstest.CreateFile("/dir3/f", []byte("irrelevant"), 0644), ) diff := []TestChange{ - Unchanged("/dir1"), Add("/dir1/d"), Add("/dir1/d/f"), Add("/dir1/f"), - Unchanged("/dir2"), Add("/dir2/f"), - Unchanged("/dir3"), Add("/dir3/f"), } @@ -198,56 +193,6 @@ func TestUpdateWithSameTime(t *testing.T) { } } -func TestUnchangedParent(t *testing.T) { - skipDiffTestOnWindows(t) - l1 := fstest.Apply( - fstest.CreateDir("/dir1", 0755), - fstest.CreateDir("/dir3", 0755), - fstest.CreateDir("/dir3/a", 0755), - fstest.CreateDir("/dir3/a/e", 0755), - fstest.CreateDir("/dir3/z", 0755), - ) - l2 := fstest.Apply( - fstest.CreateDir("/dir1/a", 0755), - fstest.CreateFile("/dir1/a/b", []byte("irrelevant"), 0644), - fstest.CreateDir("/dir1/a/e", 0755), - fstest.CreateFile("/dir1/a/e/f", []byte("irrelevant"), 0644), - fstest.CreateFile("/dir1/f", []byte("irrelevant"), 0644), - fstest.CreateDir("/dir2", 0755), - fstest.CreateFile("/dir2/f", []byte("irrelevant"), 0644), - fstest.CreateFile("/dir3/a/b", []byte("irrelevant"), 0644), - fstest.CreateDir("/dir3/a/c", 0755), - fstest.CreateDir("/dir3/a/e/i", 0755), - fstest.CreateFile("/dir3/a/e/i/f", []byte("irrelevant"), 0644), - fstest.CreateFile("/dir3/f", []byte("irrelevant"), 0644), - fstest.CreateFile("/dir3/z/f", []byte("irrelevant"), 0644), - ) - diff := []TestChange{ - Unchanged("/dir1"), - Add("/dir1/a"), - Add("/dir1/a/b"), - Add("/dir1/a/e"), - Add("/dir1/a/e/f"), - Add("/dir1/f"), - Add("/dir2"), - Add("/dir2/f"), - Unchanged("/dir3"), - Unchanged("/dir3/a"), - Add("/dir3/a/b"), - Add("/dir3/a/c"), - Unchanged("/dir3/a/e"), - Add("/dir3/a/e/i"), - Add("/dir3/a/e/i/f"), - Add("/dir3/f"), - Unchanged("/dir3/z"), - Add("/dir3/z/f"), - } - - if err := testDiffWithBase(l1, l2, diff); err != nil { - t.Fatalf("Failed diff with base: %+v", err) - } -} - // buildkit#172 func TestLchtimes(t *testing.T) { skipDiffTestOnWindows(t) @@ -411,7 +356,7 @@ func diffString(c1, c2 []TestChange) string { func changesString(c []TestChange) string { strs := make([]string, len(c)) for i := range c { - strs[i] = fmt.Sprintf("\t%-10s\t%s", c[i].Kind, c[i].Path) + strs[i] = fmt.Sprintf("\t%s\t%s", c[i].Kind, c[i].Path) } return strings.Join(strs, "\n") } @@ -436,10 +381,3 @@ func Modify(p string) TestChange { Path: filepath.FromSlash(p), } } - -func Unchanged(p string) TestChange { - return TestChange{ - Kind: ChangeKindUnmodified, - Path: filepath.FromSlash(p), - } -}