From 40b77af234319f02029368732249c2de0babb380 Mon Sep 17 00:00:00 2001 From: Darren Stahl Date: Thu, 1 Oct 2015 10:45:32 -0700 Subject: [PATCH] Fixed file modified time not changing on Windows Signed-off-by: Darren Stahl --- builder/internals.go | 15 +-- daemon/graphdriver/overlay/copy.go | 9 +- pkg/archive/archive.go | 12 +- pkg/archive/diff.go | 4 +- pkg/system/chtimes.go | 31 +++++ pkg/system/chtimes_test.go | 120 +++++++++++++++++ pkg/system/chtimes_unix_test.go | 121 ++++++++++++++++++ pkg/system/chtimes_windows_test.go | 114 +++++++++++++++++ .../{lstat_test.go => lstat_unix_test.go} | 2 + ...nfo_linux_test.go => meminfo_unix_test.go} | 2 + .../{stat_test.go => stat_unix_test.go} | 2 + pkg/system/utimes_darwin.go | 6 - pkg/system/utimes_freebsd.go | 6 - pkg/system/utimes_linux.go | 6 - .../{utimes_test.go => utimes_unix_test.go} | 2 + pkg/system/utimes_unsupported.go | 5 - 16 files changed, 412 insertions(+), 45 deletions(-) create mode 100644 pkg/system/chtimes.go create mode 100644 pkg/system/chtimes_test.go create mode 100644 pkg/system/chtimes_unix_test.go create mode 100644 pkg/system/chtimes_windows_test.go rename pkg/system/{lstat_test.go => lstat_unix_test.go} (95%) rename pkg/system/{meminfo_linux_test.go => meminfo_unix_test.go} (97%) rename pkg/system/{stat_test.go => stat_unix_test.go} (96%) rename pkg/system/{utimes_test.go => utimes_unix_test.go} (98%) diff --git a/builder/internals.go b/builder/internals.go index 5d8a448a42..217b77ded4 100644 --- a/builder/internals.go +++ b/builder/internals.go @@ -16,7 +16,6 @@ import ( "runtime" "sort" "strings" - "syscall" "time" "github.com/Sirupsen/logrus" @@ -337,23 +336,19 @@ func calcCopyInfo(b *builder, cmdName string, cInfos *[]*copyInfo, origPath stri // Set the mtime to the Last-Modified header value if present // Otherwise just remove atime and mtime - times := make([]syscall.Timespec, 2) + mTime := time.Time{} lastMod := resp.Header.Get("Last-Modified") if lastMod != "" { - mTime, err := http.ParseTime(lastMod) // If we can't parse it then just let it default to 'zero' // otherwise use the parsed time value - if err == nil { - times[1] = syscall.NsecToTimespec(mTime.UnixNano()) + if parsedMTime, err := http.ParseTime(lastMod); err == nil { + mTime = parsedMTime } } - // Windows does not support UtimesNano. - if runtime.GOOS != "windows" { - if err := system.UtimesNano(tmpFileName, times); err != nil { - return err - } + if err := system.Chtimes(tmpFileName, time.Time{}, mTime); err != nil { + return err } ci.origPath = filepath.Join(filepath.Base(tmpDirName), filepath.Base(tmpFileName)) diff --git a/daemon/graphdriver/overlay/copy.go b/daemon/graphdriver/overlay/copy.go index 835737dc79..def4d439c7 100644 --- a/daemon/graphdriver/overlay/copy.go +++ b/daemon/graphdriver/overlay/copy.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "syscall" + "time" "github.com/docker/docker/pkg/system" ) @@ -149,13 +150,15 @@ func copyDir(srcDir, dstDir string, flags copyFlags) error { } } - ts := []syscall.Timespec{stat.Atim, stat.Mtim} - // syscall.UtimesNano doesn't support a NOFOLLOW flag atm, and + // system.Chtimes doesn't support a NOFOLLOW flag atm if !isSymlink { - if err := system.UtimesNano(dstPath, ts); err != nil { + aTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + mTime := time.Unix(int64(stat.Mtim.Sec), int64(stat.Mtim.Nsec)) + if err := system.Chtimes(dstPath, aTime, mTime); err != nil { return err } } else { + ts := []syscall.Timespec{stat.Atim, stat.Mtim} if err := system.LUtimesNano(dstPath, ts); err != nil { return err } diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index ef7e3318b5..40bb3cba3a 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -375,19 +375,19 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L return err } - ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)} - // syscall.UtimesNano doesn't support a NOFOLLOW flag atm + // system.Chtimes doesn't support a NOFOLLOW flag atm if hdr.Typeflag == tar.TypeLink { if fi, err := os.Lstat(hdr.Linkname); err == nil && (fi.Mode()&os.ModeSymlink == 0) { - if err := system.UtimesNano(path, ts); err != nil && err != system.ErrNotSupportedPlatform { + if err := system.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil { return err } } } else if hdr.Typeflag != tar.TypeSymlink { - if err := system.UtimesNano(path, ts); err != nil && err != system.ErrNotSupportedPlatform { + if err := system.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil { return err } } else { + ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)} if err := system.LUtimesNano(path, ts); err != nil && err != system.ErrNotSupportedPlatform { return err } @@ -644,8 +644,8 @@ loop: 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 { + + if err := system.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil { return err } } diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go index 50656cb5fd..515fb1d358 100644 --- a/pkg/archive/diff.go +++ b/pkg/archive/diff.go @@ -9,7 +9,6 @@ import ( "path/filepath" "runtime" "strings" - "syscall" "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/pools" @@ -167,8 +166,7 @@ func UnpackLayer(dest string, layer Reader) (size int64, err error) { 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 { + if err := system.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil { return 0, err } } diff --git a/pkg/system/chtimes.go b/pkg/system/chtimes.go new file mode 100644 index 0000000000..31ed9ff100 --- /dev/null +++ b/pkg/system/chtimes.go @@ -0,0 +1,31 @@ +package system + +import ( + "os" + "time" +) + +// Chtimes changes the access time and modified time of a file at the given path +func Chtimes(name string, atime time.Time, mtime time.Time) error { + unixMinTime := time.Unix(0, 0) + // The max Unix time is 33 bits set + unixMaxTime := unixMinTime.Add((1<<33 - 1) * time.Second) + + // If the modified time is prior to the Unix Epoch, or after the + // end of Unix Time, os.Chtimes has undefined behavior + // default to Unix Epoch in this case, just in case + + if atime.Before(unixMinTime) || atime.After(unixMaxTime) { + atime = unixMinTime + } + + if mtime.Before(unixMinTime) || mtime.After(unixMaxTime) { + mtime = unixMinTime + } + + if err := os.Chtimes(name, atime, mtime); err != nil { + return err + } + + return nil +} diff --git a/pkg/system/chtimes_test.go b/pkg/system/chtimes_test.go new file mode 100644 index 0000000000..f65a4b8073 --- /dev/null +++ b/pkg/system/chtimes_test.go @@ -0,0 +1,120 @@ +package system + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + "time" +) + +// prepareTempFile creates a temporary file in a temporary directory. +func prepareTempFile(t *testing.T) (string, string) { + dir, err := ioutil.TempDir("", "docker-system-test") + if err != nil { + t.Fatal(err) + } + + file := filepath.Join(dir, "exist") + if err := ioutil.WriteFile(file, []byte("hello"), 0644); err != nil { + t.Fatal(err) + } + return file, dir +} + +// TestChtimes tests Chtimes on a tempfile. Test only mTime, because aTime is OS dependent +func TestChtimes(t *testing.T) { + file, dir := prepareTempFile(t) + defer os.RemoveAll(dir) + + beforeUnixEpochTime := time.Unix(0, 0).Add(-100 * time.Second) + unixEpochTime := time.Unix(0, 0) + afterUnixEpochTime := time.Unix(100, 0) + // The max Unix time is 33 bits set + unixMaxTime := unixEpochTime.Add((1<<33 - 1) * time.Second) + afterUnixMaxTime := unixMaxTime.Add(100 * time.Second) + + // Test both aTime and mTime set to Unix Epoch + Chtimes(file, unixEpochTime, unixEpochTime) + + f, err := os.Stat(file) + if err != nil { + t.Fatal(err) + } + + if f.ModTime() != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, f.ModTime()) + } + + // Test aTime before Unix Epoch and mTime set to Unix Epoch + Chtimes(file, beforeUnixEpochTime, unixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + if f.ModTime() != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, f.ModTime()) + } + + // Test aTime set to Unix Epoch and mTime before Unix Epoch + Chtimes(file, unixEpochTime, beforeUnixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + if f.ModTime() != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, f.ModTime()) + } + + // Test both aTime and mTime set to after Unix Epoch (valid time) + Chtimes(file, afterUnixEpochTime, afterUnixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + if f.ModTime() != afterUnixEpochTime { + t.Fatalf("Expected: %s, got: %s", afterUnixEpochTime, f.ModTime()) + } + + // Test both aTime and mTime set to Unix max time + Chtimes(file, unixMaxTime, unixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + if f.ModTime() != unixMaxTime { + t.Fatalf("Expected: %s, got: %s", unixMaxTime, f.ModTime()) + } + + // Test aTime after Unix max time and mTime set to Unix max time + Chtimes(file, afterUnixMaxTime, unixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + if f.ModTime() != unixMaxTime { + t.Fatalf("Expected: %s, got: %s", unixMaxTime, f.ModTime()) + } + + // Test aTime set to Unix Epoch and mTime before Unix Epoch + Chtimes(file, unixMaxTime, afterUnixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + if f.ModTime() != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, f.ModTime()) + } +} diff --git a/pkg/system/chtimes_unix_test.go b/pkg/system/chtimes_unix_test.go new file mode 100644 index 0000000000..6998bbef5d --- /dev/null +++ b/pkg/system/chtimes_unix_test.go @@ -0,0 +1,121 @@ +// +build linux freebsd + +package system + +import ( + "os" + "syscall" + "testing" + "time" +) + +// TestChtimes tests Chtimes access time on a tempfile on Linux +func TestChtimesLinux(t *testing.T) { + file, dir := prepareTempFile(t) + defer os.RemoveAll(dir) + + beforeUnixEpochTime := time.Unix(0, 0).Add(-100 * time.Second) + unixEpochTime := time.Unix(0, 0) + afterUnixEpochTime := time.Unix(100, 0) + // The max Unix time is 33 bits set + unixMaxTime := unixEpochTime.Add((1<<33 - 1) * time.Second) + afterUnixMaxTime := unixMaxTime.Add(100 * time.Second) + + // Test both aTime and mTime set to Unix Epoch + Chtimes(file, unixEpochTime, unixEpochTime) + + f, err := os.Stat(file) + if err != nil { + t.Fatal(err) + } + + stat := f.Sys().(*syscall.Stat_t) + aTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test aTime before Unix Epoch and mTime set to Unix Epoch + Chtimes(file, beforeUnixEpochTime, unixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + stat = f.Sys().(*syscall.Stat_t) + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test aTime set to Unix Epoch and mTime before Unix Epoch + Chtimes(file, unixEpochTime, beforeUnixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + stat = f.Sys().(*syscall.Stat_t) + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test both aTime and mTime set to after Unix Epoch (valid time) + Chtimes(file, afterUnixEpochTime, afterUnixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + stat = f.Sys().(*syscall.Stat_t) + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if aTime != afterUnixEpochTime { + t.Fatalf("Expected: %s, got: %s", afterUnixEpochTime, aTime) + } + + // Test both aTime and mTime set to Unix max time + Chtimes(file, unixMaxTime, unixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + stat = f.Sys().(*syscall.Stat_t) + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if aTime != unixMaxTime { + t.Fatalf("Expected: %s, got: %s", unixMaxTime, aTime) + } + + // Test aTime after Unix max time and mTime set to Unix max time + Chtimes(file, afterUnixMaxTime, unixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + stat = f.Sys().(*syscall.Stat_t) + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test aTime set to Unix Epoch and mTime before Unix Epoch + Chtimes(file, unixMaxTime, afterUnixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + stat = f.Sys().(*syscall.Stat_t) + aTime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if aTime != unixMaxTime { + t.Fatalf("Expected: %s, got: %s", unixMaxTime, aTime) + } +} diff --git a/pkg/system/chtimes_windows_test.go b/pkg/system/chtimes_windows_test.go new file mode 100644 index 0000000000..f09c402847 --- /dev/null +++ b/pkg/system/chtimes_windows_test.go @@ -0,0 +1,114 @@ +// +build windows + +package system + +import ( + "os" + "syscall" + "testing" + "time" +) + +// TestChtimes tests Chtimes access time on a tempfile on Windows +func TestChtimesWindows(t *testing.T) { + file, dir := prepareTempFile(t) + defer os.RemoveAll(dir) + + beforeUnixEpochTime := time.Unix(0, 0).Add(-100 * time.Second) + unixEpochTime := time.Unix(0, 0) + afterUnixEpochTime := time.Unix(100, 0) + // The max Unix time is 33 bits set + unixMaxTime := unixEpochTime.Add((1<<33 - 1) * time.Second) + afterUnixMaxTime := unixMaxTime.Add(100 * time.Second) + + // Test both aTime and mTime set to Unix Epoch + Chtimes(file, unixEpochTime, unixEpochTime) + + f, err := os.Stat(file) + if err != nil { + t.Fatal(err) + } + + aTime := time.Unix(0, f.Sys().(*syscall.Win32FileAttributeData).LastAccessTime.Nanoseconds()) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test aTime before Unix Epoch and mTime set to Unix Epoch + Chtimes(file, beforeUnixEpochTime, unixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + aTime = time.Unix(0, f.Sys().(*syscall.Win32FileAttributeData).LastAccessTime.Nanoseconds()) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test aTime set to Unix Epoch and mTime before Unix Epoch + Chtimes(file, unixEpochTime, beforeUnixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + aTime = time.Unix(0, f.Sys().(*syscall.Win32FileAttributeData).LastAccessTime.Nanoseconds()) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test both aTime and mTime set to after Unix Epoch (valid time) + Chtimes(file, afterUnixEpochTime, afterUnixEpochTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + aTime = time.Unix(0, f.Sys().(*syscall.Win32FileAttributeData).LastAccessTime.Nanoseconds()) + if aTime != afterUnixEpochTime { + t.Fatalf("Expected: %s, got: %s", afterUnixEpochTime, aTime) + } + + // Test both aTime and mTime set to Unix max time + Chtimes(file, unixMaxTime, unixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + aTime = time.Unix(0, f.Sys().(*syscall.Win32FileAttributeData).LastAccessTime.Nanoseconds()) + if aTime != unixMaxTime { + t.Fatalf("Expected: %s, got: %s", unixMaxTime, aTime) + } + + // Test aTime after Unix max time and mTime set to Unix max time + Chtimes(file, afterUnixMaxTime, unixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + aTime = time.Unix(0, f.Sys().(*syscall.Win32FileAttributeData).LastAccessTime.Nanoseconds()) + if aTime != unixEpochTime { + t.Fatalf("Expected: %s, got: %s", unixEpochTime, aTime) + } + + // Test aTime set to Unix Epoch and mTime before Unix Epoch + Chtimes(file, unixMaxTime, afterUnixMaxTime) + + f, err = os.Stat(file) + if err != nil { + t.Fatal(err) + } + + aTime = time.Unix(0, f.Sys().(*syscall.Win32FileAttributeData).LastAccessTime.Nanoseconds()) + if aTime != unixMaxTime { + t.Fatalf("Expected: %s, got: %s", unixMaxTime, aTime) + } +} diff --git a/pkg/system/lstat_test.go b/pkg/system/lstat_unix_test.go similarity index 95% rename from pkg/system/lstat_test.go rename to pkg/system/lstat_unix_test.go index 6bac492eb1..062cf53bfe 100644 --- a/pkg/system/lstat_test.go +++ b/pkg/system/lstat_unix_test.go @@ -1,3 +1,5 @@ +// +build linux freebsd + package system import ( diff --git a/pkg/system/meminfo_linux_test.go b/pkg/system/meminfo_unix_test.go similarity index 97% rename from pkg/system/meminfo_linux_test.go rename to pkg/system/meminfo_unix_test.go index 10ddf796c6..c8fec6298a 100644 --- a/pkg/system/meminfo_linux_test.go +++ b/pkg/system/meminfo_unix_test.go @@ -1,3 +1,5 @@ +// +build linux freebsd + package system import ( diff --git a/pkg/system/stat_test.go b/pkg/system/stat_unix_test.go similarity index 96% rename from pkg/system/stat_test.go rename to pkg/system/stat_unix_test.go index 57121f1579..8b3c42b733 100644 --- a/pkg/system/stat_test.go +++ b/pkg/system/stat_unix_test.go @@ -1,3 +1,5 @@ +// +build linux freebsd + package system import ( diff --git a/pkg/system/utimes_darwin.go b/pkg/system/utimes_darwin.go index 9e3dcddbc1..0a16197544 100644 --- a/pkg/system/utimes_darwin.go +++ b/pkg/system/utimes_darwin.go @@ -6,9 +6,3 @@ import "syscall" func LUtimesNano(path string, ts []syscall.Timespec) error { return ErrNotSupportedPlatform } - -// UtimesNano is used to change access and modification time of path. -// it can't be used for symbol link file. -func UtimesNano(path string, ts []syscall.Timespec) error { - return syscall.UtimesNano(path, ts) -} diff --git a/pkg/system/utimes_freebsd.go b/pkg/system/utimes_freebsd.go index 15ce26f063..e2eac3b553 100644 --- a/pkg/system/utimes_freebsd.go +++ b/pkg/system/utimes_freebsd.go @@ -20,9 +20,3 @@ func LUtimesNano(path string, ts []syscall.Timespec) error { return nil } - -// UtimesNano is used to change access and modification time of the specified path. -// It can't be used for symbol link file. -func UtimesNano(path string, ts []syscall.Timespec) error { - return syscall.UtimesNano(path, ts) -} diff --git a/pkg/system/utimes_linux.go b/pkg/system/utimes_linux.go index 7909801f9e..007bfa8c03 100644 --- a/pkg/system/utimes_linux.go +++ b/pkg/system/utimes_linux.go @@ -24,9 +24,3 @@ func LUtimesNano(path string, ts []syscall.Timespec) error { return nil } - -// UtimesNano is used to change access and modification time of the specified path. -// It can't be used for symbol link file. -func UtimesNano(path string, ts []syscall.Timespec) error { - return syscall.UtimesNano(path, ts) -} diff --git a/pkg/system/utimes_test.go b/pkg/system/utimes_unix_test.go similarity index 98% rename from pkg/system/utimes_test.go rename to pkg/system/utimes_unix_test.go index 350cce1ead..1ee0d099f9 100644 --- a/pkg/system/utimes_test.go +++ b/pkg/system/utimes_unix_test.go @@ -1,3 +1,5 @@ +// +build linux freebsd + package system import ( diff --git a/pkg/system/utimes_unsupported.go b/pkg/system/utimes_unsupported.go index cb614a12a3..50c3a04364 100644 --- a/pkg/system/utimes_unsupported.go +++ b/pkg/system/utimes_unsupported.go @@ -8,8 +8,3 @@ import "syscall" func LUtimesNano(path string, ts []syscall.Timespec) error { return ErrNotSupportedPlatform } - -// UtimesNano is not supported on platforms other than linux, freebsd and darwin. -func UtimesNano(path string, ts []syscall.Timespec) error { - return ErrNotSupportedPlatform -}