Merge pull request #674 from dmcgowan/fs-hardlink-unmodified

Fix hardlinks in tars for unmodified files

archive/tar.go

@@ -260,18 +260,20 @@ func Apply(ctx context.Context, root string, r io.Reader) (int64, error) {
 }
 
 type changeWriter struct {
-	tw         *tar.Writer
-	source     string
-	whiteoutT  time.Time
-	inodeCache map[uint64]string
+	tw        *tar.Writer
+	source    string
+	whiteoutT time.Time
+	inodeSrc  map[uint64]string
+	inodeRefs map[uint64][]string
 }
 
 func newChangeWriter(w io.Writer, source string) *changeWriter {
 	return &changeWriter{
-		tw:         tar.NewWriter(w),
-		source:     source,
-		whiteoutT:  time.Now(),
-		inodeCache: map[uint64]string{},
+		tw:        tar.NewWriter(w),
+		source:    source,
+		whiteoutT: time.Now(),
+		inodeSrc:  map[uint64]string{},
+		inodeRefs: map[uint64][]string{},
 	}
 }
 
@@ -334,15 +336,28 @@ func (cw *changeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e
 			return errors.Wrap(err, "failed to set device headers")
 		}
 
-		linkname, err := fs.GetLinkSource(name, f, cw.inodeCache)
-		if err != nil {
-			return errors.Wrap(err, "failed to get hardlink")
-		}
-
-		if linkname != "" {
-			hdr.Typeflag = tar.TypeLink
-			hdr.Linkname = linkname
-			hdr.Size = 0
+		// additionalLinks stores file names which must be linked to
+		// this file when this file is added
+		var additionalLinks []string
+		inode, isHardlink := fs.GetLinkInfo(f)
+		if isHardlink {
+			// If the inode has a source, always link to it
+			if source, ok := cw.inodeSrc[inode]; ok {
+				hdr.Typeflag = tar.TypeLink
+				hdr.Linkname = source
+				hdr.Size = 0
+			} else {
+				if k == fs.ChangeKindUnmodified {
+					cw.inodeRefs[inode] = append(cw.inodeRefs[inode], name)
+					return nil
+				}
+				cw.inodeSrc[inode] = name
+				additionalLinks = cw.inodeRefs[inode]
+				delete(cw.inodeRefs, inode)
+			}
+		} else if k == fs.ChangeKindUnmodified {
+			// Nothing to write to diff
+			return nil
 		}
 
 		if capability, err := getxattr(source, "security.capability"); err != nil {
@@ -374,6 +389,19 @@ func (cw *changeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e
 				return errors.New("short write copying file")
 			}
 		}
+
+		if additionalLinks != nil {
+			source = hdr.Name
+			for _, extra := range additionalLinks {
+				hdr.Name = extra
+				hdr.Typeflag = tar.TypeLink
+				hdr.Linkname = source
+				hdr.Size = 0
+				if err := cw.tw.WriteHeader(hdr); err != nil {
+					return errors.Wrap(err, "failed to write file header")
+				}
+			}
+		}
 	}
 	return nil
 }

archive/tar_test.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"os/exec"
 	"testing"
-	"time"
 
 	_ "crypto/sha256"
 
@@ -67,8 +66,6 @@ func TestDiffApply(t *testing.T) {
 			fstest.RemoveAll("/home"),
 			fstest.CreateDir("/home/derek", 0700),
 			fstest.CreateFile("/home/derek/.bashrc", []byte("#not going away\n"), 0640),
-			// "/etc/hosts" must be touched to be hardlinked in same layer
-			fstest.Chtime("/etc/hosts", time.Now()),
 			fstest.Link("/etc/hosts", "/etc/hosts.allow"),
 		),
 	}

fs/copy.go

@@ -65,7 +65,7 @@ func copyDirectory(dst, src string, inodes map[uint64]string) error {
 			}
 			continue
 		case (fi.Mode() & os.ModeType) == 0:
-			link, err := GetLinkSource(target, fi, inodes)
+			link, err := getLinkSource(target, fi, inodes)
 			if err != nil {
 				return errors.Wrap(err, "failed to get hardlink")
 			}

fs/diff.go

@@ -16,9 +16,13 @@ import (
 type ChangeKind int
 
 const (
+	// ChangeKindUnmodified represents an unmodified
+	// file
+	ChangeKindUnmodified = iota
+
 	// ChangeKindAdd represents an addition of
 	// a file
-	ChangeKindAdd = iota
+	ChangeKindAdd
 
 	// ChangeKindModify represents a change to
 	// an existing file
@@ -31,6 +35,8 @@ const (
 
 func (k ChangeKind) String() string {
 	switch k {
+	case ChangeKindUnmodified:
+		return "unmodified"
 	case ChangeKindAdd:
 		return "add"
 	case ChangeKindModify:
@@ -287,7 +293,10 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b string) (err
 				f1 = nil
 				f2 = nil
 				if same {
-					continue
+					if !isLinked(f) {
+						continue
+					}
+					k = ChangeKindUnmodified
 				}
 			}
 			if err := changeFn(k, p, f, nil); err != nil {

fs/diff_linux.go

@@ -90,3 +90,11 @@ func compareCapabilities(p1, p2 string) (bool, error) {
 	}
 	return bytes.Equal(c1, c2), nil
 }
+
+func isLinked(f os.FileInfo) bool {
+	s, ok := f.Sys().(*syscall.Stat_t)
+	if !ok {
+		return false
+	}
+	return !f.IsDir() && s.Nlink > 1
+}

fs/diff_windows.go

@@ -1,5 +1,7 @@
 package fs
 
+import "os"
+
 func detectDirDiff(upper, lower string) *diffDirOptions {
 	return nil
 }
@@ -13,3 +15,7 @@ func compareCapabilities(p1, p2 string) (bool, error) {
 	// TODO: Use windows equivalent
 	return true, nil
 }
+
+func isLinked(os.FileInfo) bool {
+	return false
+}

fs/hardlink.go

@@ -2,11 +2,26 @@ package fs
 
 import "os"
 
-// GetLinkSource returns a path for the given name and
+// GetLinkID returns an identifier representing the node a hardlink is pointing
+// to. If the file is not hard linked then 0 will be returned.
+func GetLinkInfo(fi os.FileInfo) (uint64, bool) {
+	return getLinkInfo(fi)
+}
+
+// getLinkSource returns a path for the given name and
 // file info to its link source in the provided inode
 // map. If the given file name is not in the map and
 // has other links, it is added to the inode map
 // to be a source for other link locations.
-func GetLinkSource(name string, fi os.FileInfo, inodes map[uint64]string) (string, error) {
-	return getHardLink(name, fi, inodes)
+func getLinkSource(name string, fi os.FileInfo, inodes map[uint64]string) (string, error) {
+	inode, isHardlink := getLinkInfo(fi)
+	if !isHardlink {
+		return "", nil
+	}
+
+	path, ok := inodes[inode]
+	if !ok {
+		inodes[inode] = name
+	}
+	return path, nil
 }

fs/hardlink_unix.go

@@ -3,31 +3,15 @@
 package fs
 
 import (
-	"errors"
 	"os"
 	"syscall"
 )
 
-func getHardLink(name string, fi os.FileInfo, inodes map[uint64]string) (string, error) {
-	if fi.IsDir() {
-		return "", nil
-	}
-
+func getLinkInfo(fi os.FileInfo) (uint64, bool) {
 	s, ok := fi.Sys().(*syscall.Stat_t)
 	if !ok {
-		return "", errors.New("unsupported stat type")
+		return 0, false
 	}
 
-	// If inode is not hardlinked, no reason to lookup or save inode
-	if s.Nlink == 1 {
-		return "", nil
-	}
-
-	inode := uint64(s.Ino)
-
-	path, ok := inodes[inode]
-	if !ok {
-		inodes[inode] = name
-	}
-	return path, nil
+	return uint64(s.Ino), !fi.IsDir() && s.Nlink > 1
 }

fs/hardlink_windows.go

@@ -2,6 +2,6 @@ package fs
 
 import "os"
 
-func getHardLink(string, os.FileInfo, map[uint64]string) (string, error) {
-	return "", nil
+func getLinkInfo(fi os.FileInfo) (uint64, bool) {
+	return 0, false
 }