Merge pull request #3207 from alexlarsson/fix-applylayer

Re-enable TestApplyLayer and make it work

archive/changes.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"strings"
 	"syscall"
+	"time"
 )
 
 type ChangeType int
@@ -34,6 +35,21 @@ func (change *Change) String() string {
 	return fmt.Sprintf("%s %s", kind, change.Path)
 }
 
+// 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
+// second count and either a or b having exactly 0 nanoseconds
+func sameFsTime(a, b time.Time) bool {
+	return a == b ||
+		(a.Unix() == b.Unix() &&
+			(a.Nanosecond() == 0 || b.Nanosecond() == 0))
+}
+
+func sameFsTimeSpec(a, b syscall.Timespec) bool {
+	return a.Sec == b.Sec &&
+		(a.Nsec == b.Nsec || a.Nsec == 0 || b.Nsec == 0)
+}
+
 func Changes(layers []string, rw string) ([]Change, error) {
 	var changes []Change
 	err := filepath.Walk(rw, func(path string, f os.FileInfo, err error) error {
@@ -85,7 +101,7 @@ func Changes(layers []string, rw string) ([]Change, error) {
 					// However, if it's a directory, maybe it wasn't actually modified.
 					// If you modify /foo/bar/baz, then /foo will be part of the changed files only because it's the parent of bar
 					if stat.IsDir() && f.IsDir() {
-						if f.Size() == stat.Size() && f.Mode() == stat.Mode() && f.ModTime() == stat.ModTime() {
+						if f.Size() == stat.Size() && f.Mode() == stat.Mode() && sameFsTime(f.ModTime(), stat.ModTime()) {
 							// Both directories are the same, don't record the change
 							return nil
 						}
@@ -181,7 +197,7 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
 				oldStat.Rdev != newStat.Rdev ||
 				// Don't look at size for dirs, its not a good measure of change
 				(oldStat.Size != newStat.Size && oldStat.Mode&syscall.S_IFDIR != syscall.S_IFDIR) ||
-				getLastModification(oldStat) != getLastModification(newStat) {
+				!sameFsTimeSpec(getLastModification(oldStat), getLastModification(newStat)) {
 				change := Change{
 					Path: newChild.path(),
 					Kind: ChangeModify,

archive/changes_test.go

@@ -258,48 +258,44 @@ func TestChangesDirsMutated(t *testing.T) {
 }
 
 func TestApplyLayer(t *testing.T) {
-	t.Skip("Skipping TestApplyLayer due to known failures") // Disable this for now as it is broken
-	return
+	src, err := ioutil.TempDir("", "docker-changes-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	createSampleDir(t, src)
+	defer os.RemoveAll(src)
+	dst := src + "-copy"
+	if err := copyDir(src, dst); err != nil {
+		t.Fatal(err)
+	}
+	mutateSampleDir(t, dst)
+	defer os.RemoveAll(dst)
 
-	// src, err := ioutil.TempDir("", "docker-changes-test")
-	// if err != nil {
-	// 	t.Fatal(err)
-	// }
-	// createSampleDir(t, src)
-	// dst := src + "-copy"
-	// if err := copyDir(src, dst); err != nil {
-	// 	t.Fatal(err)
-	// }
-	// mutateSampleDir(t, dst)
+	changes, err := ChangesDirs(dst, src)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	// changes, err := ChangesDirs(dst, src)
-	// if err != nil {
-	// 	t.Fatal(err)
-	// }
+	layer, err := ExportChanges(dst, changes)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	// layer, err := ExportChanges(dst, changes)
-	// if err != nil {
-	// 	t.Fatal(err)
-	// }
+	layerCopy, err := NewTempArchive(layer, "")
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	// layerCopy, err := NewTempArchive(layer, "")
-	// if err != nil {
-	// 	t.Fatal(err)
-	// }
+	if err := ApplyLayer(src, layerCopy); err != nil {
+		t.Fatal(err)
+	}
 
-	// if err := ApplyLayer(src, layerCopy); err != nil {
-	// 	t.Fatal(err)
-	// }
+	changes2, err := ChangesDirs(src, dst)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	// changes2, err := ChangesDirs(src, dst)
-	// if err != nil {
-	// 	t.Fatal(err)
-	// }
-
-	// if len(changes2) != 0 {
-	// 	t.Fatalf("Unexpected differences after re applying mutation: %v", changes)
-	// }
-
-	// os.RemoveAll(src)
-	// os.RemoveAll(dst)
+	if len(changes2) != 0 {
+		t.Fatalf("Unexpected differences after re applying mutation: %v", changes2)
+	}
 }

archive/diff.go

@@ -1,6 +1,9 @@
 package archive
 
 import (
+	"archive/tar"
+	"github.com/dotcloud/docker/utils"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -8,87 +11,176 @@ import (
 	"time"
 )
 
+// Linux device nodes are a bit weird due to backwards compat with 16 bit device nodes
+// The lower 8 bit is the lower 8 bit in the minor, the following 12 bits are the major,
+// and then there is the top 12 bits of then minor
+func mkdev(major int64, minor int64) uint32 {
+	return uint32(((minor & 0xfff00) << 12) | ((major & 0xfff) << 8) | (minor & 0xff))
+}
+func timeToTimespec(time time.Time) (ts syscall.Timespec) {
+	if time.IsZero() {
+		// Return UTIME_OMIT special value
+		ts.Sec = 0
+		ts.Nsec = ((1 << 30) - 2)
+		return
+	}
+	return syscall.NsecToTimespec(time.UnixNano())
+}
+
 // ApplyLayer parses a diff in the standard layer format from `layer`, and
 // applies it to the directory `dest`.
 func ApplyLayer(dest string, layer Archive) error {
-	// Poor man's diff applyer in 2 steps:
+	// We need to be able to set any perms
+	oldmask := syscall.Umask(0)
+	defer syscall.Umask(oldmask)
 
-	// Step 1: untar everything in place
-	if err := Untar(layer, dest, nil); err != nil {
-		return err
-	}
+	tr := tar.NewReader(layer)
 
-	modifiedDirs := make(map[string]*syscall.Stat_t)
-	addDir := func(file string) {
-		d := filepath.Dir(file)
-		if _, exists := modifiedDirs[d]; !exists {
-			if s, err := os.Lstat(d); err == nil {
-				if sys := s.Sys(); sys != nil {
-					if stat, ok := sys.(*syscall.Stat_t); ok {
-						modifiedDirs[d] = stat
+	var dirs []*tar.Header
+
+	// Iterate through the files in the archive.
+	for {
+		hdr, err := tr.Next()
+		if err == io.EOF {
+			// end of tar archive
+			break
+		}
+		if err != nil {
+			return err
+		}
+
+		// Normalize name, for safety and for a simple is-root check
+		hdr.Name = filepath.Clean(hdr.Name)
+
+		if !strings.HasSuffix(hdr.Name, "/") {
+			// Not the root directory, ensure that the parent directory exists
+			// This happened in some tests where an image had a tarfile without any
+			// parent directories
+			parent := filepath.Dir(hdr.Name)
+			parentPath := filepath.Join(dest, parent)
+			if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) {
+				err = os.MkdirAll(parentPath, 600)
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+		// Skip AUFS metadata dirs
+		if strings.HasPrefix(hdr.Name, ".wh..wh.") {
+			continue
+		}
+
+		path := filepath.Join(dest, hdr.Name)
+		base := filepath.Base(path)
+		if strings.HasPrefix(base, ".wh.") {
+			originalBase := base[len(".wh."):]
+			originalPath := filepath.Join(filepath.Dir(path), originalBase)
+			if err := os.RemoveAll(originalPath); err != nil {
+				return err
+			}
+		} else {
+			// If path exits we almost always just want to remove and replace it
+			// The only exception is when it is a directory *and* the file from
+			// the layer is also a directory. Then we want to merge them (i.e.
+			// just apply the metadata from the layer).
+			hasDir := false
+			if fi, err := os.Lstat(path); err == nil {
+				if fi.IsDir() && hdr.Typeflag == tar.TypeDir {
+					hasDir = true
+				} else {
+					if err := os.RemoveAll(path); err != nil {
+						return err
+					}
+				}
+			}
+
+			switch hdr.Typeflag {
+			case tar.TypeDir:
+				if !hasDir {
+					err = os.Mkdir(path, os.FileMode(hdr.Mode))
+					if err != nil {
+						return err
+					}
+				}
+				dirs = append(dirs, hdr)
+
+			case tar.TypeReg, tar.TypeRegA:
+				// Source is regular file
+				file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, os.FileMode(hdr.Mode))
+				if err != nil {
+					return err
+				}
+				if _, err := io.Copy(file, tr); err != nil {
+					file.Close()
+					return err
+				}
+				file.Close()
+
+			case tar.TypeBlock, tar.TypeChar, tar.TypeFifo:
+				mode := uint32(hdr.Mode & 07777)
+				switch hdr.Typeflag {
+				case tar.TypeBlock:
+					mode |= syscall.S_IFBLK
+				case tar.TypeChar:
+					mode |= syscall.S_IFCHR
+				case tar.TypeFifo:
+					mode |= syscall.S_IFIFO
+				}
+
+				if err := syscall.Mknod(path, mode, int(mkdev(hdr.Devmajor, hdr.Devminor))); err != nil {
+					return err
+				}
+
+			case tar.TypeLink:
+				if err := os.Link(filepath.Join(dest, hdr.Linkname), path); err != nil {
+					return err
+				}
+
+			case tar.TypeSymlink:
+				if err := os.Symlink(hdr.Linkname, path); err != nil {
+					return err
+				}
+
+			default:
+				utils.Debugf("unhandled type %d\n", hdr.Typeflag)
+			}
+
+			if err = syscall.Lchown(path, hdr.Uid, hdr.Gid); err != nil {
+				return err
+			}
+
+			// There is no LChmod, so ignore mode for symlink.  Also, this
+			// must happen after chown, as that can modify the file mode
+			if hdr.Typeflag != tar.TypeSymlink {
+				err = syscall.Chmod(path, uint32(hdr.Mode&07777))
+				if err != nil {
+					return err
+				}
+			}
+
+			// Directories must be handled at the end to avoid further
+			// file creation in them to modify the mtime
+			if hdr.Typeflag != tar.TypeDir {
+				ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)}
+				// syscall.UtimesNano doesn't support a NOFOLLOW flag atm, and
+				if hdr.Typeflag != tar.TypeSymlink {
+					if err := syscall.UtimesNano(path, ts); err != nil {
+						return err
+					}
+				} else {
+					if err := LUtimesNano(path, ts); err != nil {
+						return err
 					}
 				}
 			}
 		}
 	}
 
-	// Step 2: walk for whiteouts and apply them, removing them in the process
-	err := filepath.Walk(dest, func(fullPath string, f os.FileInfo, err error) error {
-		if err != nil {
-			if os.IsNotExist(err) {
-				// This happens in the case of whiteouts in parent dir removing a directory
-				// We just ignore it
-				return filepath.SkipDir
-			}
-			return err
-		}
-
-		// Rebase path
-		path, err := filepath.Rel(dest, fullPath)
-		if err != nil {
-			return err
-		}
-		path = filepath.Join("/", path)
-
-		// Skip AUFS metadata
-		if matched, err := filepath.Match("/.wh..wh.*", path); err != nil {
-			return err
-		} else if matched {
-			addDir(fullPath)
-			if err := os.RemoveAll(fullPath); err != nil {
-				return err
-			}
-		}
-
-		filename := filepath.Base(path)
-		if strings.HasPrefix(filename, ".wh.") {
-			rmTargetName := filename[len(".wh."):]
-			rmTargetPath := filepath.Join(filepath.Dir(fullPath), rmTargetName)
-
-			// Remove the file targeted by the whiteout
-			addDir(rmTargetPath)
-			if err := os.RemoveAll(rmTargetPath); err != nil {
-				return err
-			}
-			// Remove the whiteout itself
-			addDir(fullPath)
-			if err := os.RemoveAll(fullPath); err != nil {
-				return err
-			}
-		}
-		return nil
-	})
-	if err != nil {
-		return err
-	}
-
-	for k, v := range modifiedDirs {
-		lastAccess := getLastAccess(v)
-		lastModification := getLastModification(v)
-		aTime := time.Unix(lastAccess.Unix())
-		mTime := time.Unix(lastModification.Unix())
-
-		if err := os.Chtimes(k, aTime, mTime); err != nil {
+	for _, hdr := range dirs {
+		path := filepath.Join(dest, hdr.Name)
+		ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)}
+		if err := syscall.UtimesNano(path, ts); err != nil {
 			return err
 		}
 	}

archive/stat_darwin.go

@@ -9,3 +9,7 @@ func getLastAccess(stat *syscall.Stat_t) syscall.Timespec {
 func getLastModification(stat *syscall.Stat_t) syscall.Timespec {
 	return stat.Mtimespec
 }
+
+func LUtimesNano(path string, ts []syscall.Timespec) error {
+	return nil
+}

archive/stat_linux.go

@@ -1,6 +1,9 @@
 package archive
 
-import "syscall"
+import (
+	"syscall"
+	"unsafe"
+)
 
 func getLastAccess(stat *syscall.Stat_t) syscall.Timespec {
 	return stat.Atim
@@ -9,3 +12,21 @@ func getLastAccess(stat *syscall.Stat_t) syscall.Timespec {
 func getLastModification(stat *syscall.Stat_t) syscall.Timespec {
 	return stat.Mtim
 }
+
+func LUtimesNano(path string, ts []syscall.Timespec) error {
+	// These are not currently availible in syscall
+	AT_FDCWD := -100
+	AT_SYMLINK_NOFOLLOW := 0x100
+
+	var _path *byte
+	_path, err := syscall.BytePtrFromString(path)
+	if err != nil {
+		return err
+	}
+
+	if _, _, err := syscall.Syscall6(syscall.SYS_UTIMENSAT, uintptr(AT_FDCWD), uintptr(unsafe.Pointer(_path)), uintptr(unsafe.Pointer(&ts[0])), uintptr(AT_SYMLINK_NOFOLLOW), 0, 0); err != 0 && err != syscall.ENOSYS {
+		return err
+	}
+
+	return nil
+}