Merge pull request #16490 from Microsoft/10662-mtimefix

Fixed file modified time not changing on windows

builder/internals.go

@@ -16,7 +16,6 @@ import (
 	"runtime"
 	"sort"
 	"strings"
-	"syscall"
 	"time"
 
 	"github.com/Sirupsen/logrus"
@@ -342,23 +341,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))

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
 			}

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
 		}
 	}

pkg/archive/diff.go

@@ -9,7 +9,6 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
-	"syscall"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/pools"
@@ -184,8 +183,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
 		}
 	}

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
+}

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())
+	}
+}

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)
+	}
+}

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)
+	}
+}

pkg/system/lstat_test.go → pkg/system/lstat_unix_test.go

@@ -1,3 +1,5 @@
+// +build linux freebsd
+
 package system
 
 import (

pkg/system/meminfo_linux_test.go → pkg/system/meminfo_unix_test.go

@@ -1,3 +1,5 @@
+// +build linux freebsd
+
 package system
 
 import (

pkg/system/stat_test.go → pkg/system/stat_unix_test.go

@@ -1,3 +1,5 @@
+// +build linux freebsd
+
 package system
 
 import (

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)
-}

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)
-}

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)
-}

pkg/system/utimes_test.go → pkg/system/utimes_unix_test.go

@@ -1,3 +1,5 @@
+// +build linux freebsd
+
 package system
 
 import (

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
-}