Refactor pkg/archive with a platform-independent stat struct

pkg/archive contains code both invoked from cli (cross platform) and
daemon (linux only) and Unix-specific dependencies break compilation on
Windows. We extracted those stat-related funcs into platform specific
implementations at pkg/system and added unit tests.

Signed-off-by: Ahmet Alp Balkan <ahmetb@microsoft.com>

archive/archive.go

@@ -192,20 +192,11 @@ func (ta *tarAppender) addTarFile(path, name string) error {
 
 	hdr.Name = name
 
-	var (
+	nlink, inode, err := setHeaderForSpecialDevice(hdr, ta, name, fi.Sys())
-		nlink uint32
+	if err != nil {
-		inode uint64
+		return err
-	)
-	if stat, ok := fi.Sys().(*syscall.Stat_t); ok {
-		nlink = uint32(stat.Nlink)
-		inode = uint64(stat.Ino)
-		// Currently go does not fill in the major/minors
-		if stat.Mode&syscall.S_IFBLK == syscall.S_IFBLK ||
-			stat.Mode&syscall.S_IFCHR == syscall.S_IFCHR {
-			hdr.Devmajor = int64(major(uint64(stat.Rdev)))
-			hdr.Devminor = int64(minor(uint64(stat.Rdev)))
-		}
 	}
+
 	// if it's a regular file and has more than 1 link,
 	// it's hardlinked, so set the type flag accordingly
 	if fi.Mode().IsRegular() && nlink > 1 {
@@ -291,7 +282,7 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L
 			mode |= syscall.S_IFIFO
 		}
 
-		if err := syscall.Mknod(path, mode, int(system.Mkdev(hdr.Devmajor, hdr.Devminor))); err != nil {
+		if err := system.Mknod(path, mode, int(system.Mkdev(hdr.Devmajor, hdr.Devminor))); err != nil {
 			return err
 		}
 

archive/archive_unix.go

@@ -0,0 +1,39 @@
+// +build !windows
+
+package archive
+
+import (
+	"errors"
+	"syscall"
+
+	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
+)
+
+func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
+	s, ok := stat.(*syscall.Stat_t)
+
+	if !ok {
+		err = errors.New("cannot convert stat value to syscall.Stat_t")
+		return
+	}
+
+	nlink = uint32(s.Nlink)
+	inode = uint64(s.Ino)
+
+	// Currently go does not fil in the major/minors
+	if s.Mode&syscall.S_IFBLK == syscall.S_IFBLK ||
+		s.Mode&syscall.S_IFCHR == syscall.S_IFCHR {
+		hdr.Devmajor = int64(major(uint64(s.Rdev)))
+		hdr.Devminor = int64(minor(uint64(s.Rdev)))
+	}
+
+	return
+}
+
+func major(device uint64) uint64 {
+	return (device >> 8) & 0xfff
+}
+
+func minor(device uint64) uint64 {
+	return (device & 0xff) | ((device >> 12) & 0xfff00)
+}

archive/archive_windows.go

@@ -0,0 +1,12 @@
+// +build windows
+
+package archive
+
+import (
+	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
+)
+
+func setHeaderForSpecialDevice(hdr *tar.Header, ta *tarAppender, name string, stat interface{}) (nlink uint32, inode uint64, err error) {
+	// do nothing. no notion of Rdev, Inode, Nlink in stat on Windows
+	return
+}

archive/changes.go

@@ -135,7 +135,7 @@ func Changes(layers []string, rw string) ([]Change, error) {
 type FileInfo struct {
 	parent     *FileInfo
 	name       string
-	stat       syscall.Stat_t
+	stat       *system.Stat
 	children   map[string]*FileInfo
 	capability []byte
 	added      bool
@@ -168,7 +168,7 @@ func (info *FileInfo) path() string {
 }
 
 func (info *FileInfo) isDir() bool {
-	return info.parent == nil || info.stat.Mode&syscall.S_IFDIR == syscall.S_IFDIR
+	return info.parent == nil || info.stat.Mode()&syscall.S_IFDIR == syscall.S_IFDIR
 }
 
 func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
@@ -199,21 +199,21 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) {
 		oldChild, _ := oldChildren[name]
 		if oldChild != nil {
 			// change?
-			oldStat := &oldChild.stat
+			oldStat := oldChild.stat
-			newStat := &newChild.stat
+			newStat := newChild.stat
 			// Note: We can't compare inode or ctime or blocksize here, because these change
 			// when copying a file into a container. However, that is not generally a problem
 			// because any content change will change mtime, and any status change should
 			// be visible when actually comparing the stat fields. The only time this
 			// breaks down is if some code intentionally hides a change by setting
 			// back mtime
-			if oldStat.Mode != newStat.Mode ||
+			if oldStat.Mode() != newStat.Mode() ||
-				oldStat.Uid != newStat.Uid ||
+				oldStat.Uid() != newStat.Uid() ||
-				oldStat.Gid != newStat.Gid ||
+				oldStat.Gid() != newStat.Gid() ||
-				oldStat.Rdev != newStat.Rdev ||
+				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) ||
+				(oldStat.Size() != newStat.Size() && oldStat.Mode()&syscall.S_IFDIR != syscall.S_IFDIR) ||
-				!sameFsTimeSpec(system.GetLastModification(oldStat), system.GetLastModification(newStat)) ||
+				!sameFsTimeSpec(oldStat.Mtim(), newStat.Mtim()) ||
 				bytes.Compare(oldChild.capability, newChild.capability) != 0 {
 				change := Change{
 					Path: newChild.path(),
@@ -269,14 +269,6 @@ func newRootFileInfo() *FileInfo {
 	return root
 }
 
-func lstat(path string) (*stat, error) {
-	s, err := system.Lstat(path)
-	if err != nil {
-		return nil, err
-	}
-	return fromStatT(s), nil
-}
-
 func collectFileInfo(sourceDir string) (*FileInfo, error) {
 	root := newRootFileInfo()
 
@@ -307,7 +299,7 @@ func collectFileInfo(sourceDir string) (*FileInfo, error) {
 			parent:   parent,
 		}
 
-		s, err := lstat(path)
+		s, err := system.Lstat(path)
 		if err != nil {
 			return err
 		}
@@ -369,14 +361,6 @@ func ChangesSize(newDir string, changes []Change) int64 {
 	return size
 }
 
-func major(device uint64) uint64 {
-	return (device >> 8) & 0xfff
-}
-
-func minor(device uint64) uint64 {
-	return (device & 0xff) | ((device >> 12) & 0xfff00)
-}
-
 // ExportChanges produces an Archive from the provided changes, relative to dir.
 func ExportChanges(dir string, changes []Change) (Archive, error) {
 	reader, writer := io.Pipe()

archive/diff.go

@@ -12,16 +12,21 @@ import (
 	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
 
 	"github.com/docker/docker/pkg/pools"
+	"github.com/docker/docker/pkg/system"
 )
 
 // ApplyLayer parses a diff in the standard layer format from `layer`, and
 // applies it to the directory `dest`.
 func ApplyLayer(dest string, layer ArchiveReader) error {
 	// We need to be able to set any perms
-	oldmask := syscall.Umask(0)
+	oldmask, err := system.Umask(0)
-	defer syscall.Umask(oldmask)
+	if err != nil {
+		return err
+	}
 
-	layer, err := DecompressStream(layer)
+	defer system.Umask(oldmask) // ignore err, ErrNotSupportedPlatform
+
+	layer, err = DecompressStream(layer)
 	if err != nil {
 		return err
 	}

system/lstat.go

@@ -6,11 +6,11 @@ import (
 	"syscall"
 )
 
-func Lstat(path string) (*syscall.Stat_t, error) {
+func Lstat(path string) (*Stat, error) {
 	s := &syscall.Stat_t{}
 	err := syscall.Lstat(path, s)
 	if err != nil {
 		return nil, err
 	}
-	return s, nil
+	return fromStatT(s)
 }

system/lstat_test.go

@@ -0,0 +1,25 @@
+package system
+
+import (
+	"testing"
+)
+
+func TestLstat(t *testing.T) {
+	file, invalid, _ := prepareFiles(t)
+
+	statFile, err := Lstat(file)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if statFile == nil {
+		t.Fatal("returned empty stat for existing file")
+	}
+
+	statInvalid, err := Lstat(invalid)
+	if err == nil {
+		t.Fatal("did not return error for non-existing file")
+	}
+	if statInvalid != nil {
+		t.Fatal("returned non-nil stat for non-existing file")
+	}
+}

system/lstat_windows.go

@@ -2,11 +2,7 @@
 
 package system
 
-import (
+func Lstat(path string) (*Stat, error) {
-	"syscall"
-)
-
-func Lstat(path string) (*syscall.Win32FileAttributeData, error) {
 	// should not be called on cli code path
 	return nil, ErrNotSupportedPlatform
 }

system/stat.go

@@ -0,0 +1,42 @@
+package system
+
+import (
+	"syscall"
+)
+
+type Stat struct {
+	mode uint32
+	uid  uint32
+	gid  uint32
+	rdev uint64
+	size int64
+	mtim syscall.Timespec
+}
+
+func (s Stat) Mode() uint32 {
+	return s.mode
+}
+
+func (s Stat) Uid() uint32 {
+	return s.uid
+}
+
+func (s Stat) Gid() uint32 {
+	return s.gid
+}
+
+func (s Stat) Rdev() uint64 {
+	return s.rdev
+}
+
+func (s Stat) Size() int64 {
+	return s.size
+}
+
+func (s Stat) Mtim() syscall.Timespec {
+	return s.mtim
+}
+
+func (s Stat) GetLastModification() syscall.Timespec {
+	return s.Mtim()
+}

system/stat_linux.go

@@ -4,10 +4,11 @@ import (
 	"syscall"
 )
 
-func GetLastAccess(stat *syscall.Stat_t) syscall.Timespec {
+func fromStatT(s *syscall.Stat_t) (*Stat, error) {
-	return stat.Atim
+	return &Stat{size: s.Size,
-}
+		mode: s.Mode,
-
+		uid:  s.Uid,
-func GetLastModification(stat *syscall.Stat_t) syscall.Timespec {
+		gid:  s.Gid,
-	return stat.Mtim
+		rdev: s.Rdev,
+		mtim: s.Mtim}, nil
 }

system/stat_test.go

@@ -0,0 +1,34 @@
+package system
+
+import (
+	"syscall"
+	"testing"
+)
+
+func TestFromStatT(t *testing.T) {
+	file, _, _ := prepareFiles(t)
+
+	stat := &syscall.Stat_t{}
+	err := syscall.Lstat(file, stat)
+
+	s, err := fromStatT(stat)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if stat.Mode != s.Mode() {
+		t.Fatal("got invalid mode")
+	}
+	if stat.Uid != s.Uid() {
+		t.Fatal("got invalid uid")
+	}
+	if stat.Gid != s.Gid() {
+		t.Fatal("got invalid gid")
+	}
+	if stat.Rdev != s.Rdev() {
+		t.Fatal("got invalid rdev")
+	}
+	if stat.Mtim != s.Mtim() {
+		t.Fatal("got invalid mtim")
+	}
+}

system/stat_unsupported.go

@@ -1,13 +1,16 @@
-// +build !linux
+// +build !linux,!windows
 
 package system
 
-import "syscall"
+import (
+	"syscall"
+)
 
-func GetLastAccess(stat *syscall.Stat_t) syscall.Timespec {
+func fromStatT(s *syscall.Stat_t) (*Stat, error) {
-	return stat.Atimespec
+	return &Stat{size: s.Size,
-}
+		mode: uint32(s.Mode),
-
+		uid:  s.Uid,
-func GetLastModification(stat *syscall.Stat_t) syscall.Timespec {
+		gid:  s.Gid,
-	return stat.Mtimespec
+		rdev: uint64(s.Rdev),
+		mtim: s.Mtimespec}, nil
 }

system/stat_windows.go

@@ -0,0 +1,12 @@
+// +build windows
+
+package system
+
+import (
+	"errors"
+	"syscall"
+)
+
+func fromStatT(s *syscall.Win32FileAttributeData) (*Stat, error) {
+	return nil, errors.New("fromStatT should not be called on windows path")
+}