Fix hardlinks with unmodified files

Previously hardlinking to an unmodified file or linking to a file
which was touched by not detected as modified caused a new file
to be created on unpack. This new file and the original source file
were not linked since no link record was created in the tar. This
change addresses this by adding links for all hardlinks to a file
when it is detected as changed. These links will be written after
the source file is written and may occur out of order in regard to
file name.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>

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
 }

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
 }