From 2180aa4f6f2ad4d8f284d63ee29e93547263976e Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Thu, 13 Nov 2014 12:36:05 -0800 Subject: [PATCH] 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 --- pkg/archive/archive.go | 19 ++++----------- pkg/archive/archive_unix.go | 39 +++++++++++++++++++++++++++++++ pkg/archive/archive_windows.go | 12 ++++++++++ pkg/archive/changes.go | 38 +++++++++--------------------- pkg/archive/diff.go | 11 ++++++--- pkg/system/lstat.go | 4 ++-- pkg/system/lstat_test.go | 25 ++++++++++++++++++++ pkg/system/lstat_windows.go | 6 +---- pkg/system/stat.go | 42 ++++++++++++++++++++++++++++++++++ pkg/system/stat_linux.go | 13 ++++++----- pkg/system/stat_test.go | 34 +++++++++++++++++++++++++++ pkg/system/stat_unsupported.go | 19 ++++++++------- pkg/system/stat_windows.go | 12 ++++++++++ 13 files changed, 209 insertions(+), 65 deletions(-) create mode 100644 pkg/archive/archive_unix.go create mode 100644 pkg/archive/archive_windows.go create mode 100644 pkg/system/lstat_test.go create mode 100644 pkg/system/stat.go create mode 100644 pkg/system/stat_test.go create mode 100644 pkg/system/stat_windows.go diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 85d23190d0..5a81223dbd 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -192,20 +192,11 @@ func (ta *tarAppender) addTarFile(path, name string) error { hdr.Name = name - var ( - nlink uint32 - inode uint64 - ) - 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))) - } + nlink, inode, err := setHeaderForSpecialDevice(hdr, ta, name, fi.Sys()) + if err != nil { + return err } + // 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 } diff --git a/pkg/archive/archive_unix.go b/pkg/archive/archive_unix.go new file mode 100644 index 0000000000..c0e8aee93c --- /dev/null +++ b/pkg/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) +} diff --git a/pkg/archive/archive_windows.go b/pkg/archive/archive_windows.go new file mode 100644 index 0000000000..3cc2493f6f --- /dev/null +++ b/pkg/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 +} diff --git a/pkg/archive/changes.go b/pkg/archive/changes.go index 720d549758..85217f6e08 100644 --- a/pkg/archive/changes.go +++ b/pkg/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 - newStat := &newChild.stat + oldStat := oldChild.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 || - oldStat.Uid != newStat.Uid || - oldStat.Gid != newStat.Gid || - oldStat.Rdev != newStat.Rdev || + if oldStat.Mode() != newStat.Mode() || + oldStat.Uid() != newStat.Uid() || + oldStat.Gid() != newStat.Gid() || + 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) || - !sameFsTimeSpec(system.GetLastModification(oldStat), system.GetLastModification(newStat)) || + (oldStat.Size() != newStat.Size() && oldStat.Mode()&syscall.S_IFDIR != syscall.S_IFDIR) || + !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() diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go index c208336ab3..eabb7c48ff 100644 --- a/pkg/archive/diff.go +++ b/pkg/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) - defer syscall.Umask(oldmask) + oldmask, err := system.Umask(0) + 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 } diff --git a/pkg/system/lstat.go b/pkg/system/lstat.go index d7e06b3efb..9ef82d5523 100644 --- a/pkg/system/lstat.go +++ b/pkg/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) } diff --git a/pkg/system/lstat_test.go b/pkg/system/lstat_test.go new file mode 100644 index 0000000000..7e271efea5 --- /dev/null +++ b/pkg/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") + } +} diff --git a/pkg/system/lstat_windows.go b/pkg/system/lstat_windows.go index f4c7e6d0e9..213a7c7ade 100644 --- a/pkg/system/lstat_windows.go +++ b/pkg/system/lstat_windows.go @@ -2,11 +2,7 @@ package system -import ( - "syscall" -) - -func Lstat(path string) (*syscall.Win32FileAttributeData, error) { +func Lstat(path string) (*Stat, error) { // should not be called on cli code path return nil, ErrNotSupportedPlatform } diff --git a/pkg/system/stat.go b/pkg/system/stat.go new file mode 100644 index 0000000000..5d47494d21 --- /dev/null +++ b/pkg/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() +} diff --git a/pkg/system/stat_linux.go b/pkg/system/stat_linux.go index e702200360..47cebef5cf 100644 --- a/pkg/system/stat_linux.go +++ b/pkg/system/stat_linux.go @@ -4,10 +4,11 @@ import ( "syscall" ) -func GetLastAccess(stat *syscall.Stat_t) syscall.Timespec { - return stat.Atim -} - -func GetLastModification(stat *syscall.Stat_t) syscall.Timespec { - return stat.Mtim +func fromStatT(s *syscall.Stat_t) (*Stat, error) { + return &Stat{size: s.Size, + mode: s.Mode, + uid: s.Uid, + gid: s.Gid, + rdev: s.Rdev, + mtim: s.Mtim}, nil } diff --git a/pkg/system/stat_test.go b/pkg/system/stat_test.go new file mode 100644 index 0000000000..0dcb239ece --- /dev/null +++ b/pkg/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") + } +} diff --git a/pkg/system/stat_unsupported.go b/pkg/system/stat_unsupported.go index 4686a4c346..c4d53e6cd6 100644 --- a/pkg/system/stat_unsupported.go +++ b/pkg/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 { - return stat.Atimespec -} - -func GetLastModification(stat *syscall.Stat_t) syscall.Timespec { - return stat.Mtimespec +func fromStatT(s *syscall.Stat_t) (*Stat, error) { + return &Stat{size: s.Size, + mode: uint32(s.Mode), + uid: s.Uid, + gid: s.Gid, + rdev: uint64(s.Rdev), + mtim: s.Mtimespec}, nil } diff --git a/pkg/system/stat_windows.go b/pkg/system/stat_windows.go new file mode 100644 index 0000000000..584e8940cc --- /dev/null +++ b/pkg/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") +}