Merge pull request #37356 from Microsoft/jjh/new37316

LCOW: lazycontext: Use correct lstat, fix archive check

builder/remotecontext/lazycontext.go

@@ -45,7 +45,7 @@ func (c *lazySource) Hash(path string) (string, error) {
 		return "", errors.WithStack(convertPathError(err, cleanPath))
 	}
 
-	fi, err := os.Lstat(fullPath)
+	fi, err := c.root.Lstat(fullPath)
 	if err != nil {
 		// Backwards compatibility: a missing file returns a path as hash.
 		// This is reached in the case of a broken symlink.

pkg/archive/archive.go

@@ -367,11 +367,7 @@ func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, erro
 	hdr.AccessTime = time.Time{}
 	hdr.ChangeTime = time.Time{}
 	hdr.Mode = fillGo18FileTypeBits(int64(chmodTarEntry(os.FileMode(hdr.Mode))), fi)
-	name, err = canonicalTarName(name, fi.IsDir())
-	if err != nil {
-		return nil, fmt.Errorf("tar: cannot canonicalize path: %v", err)
-	}
-	hdr.Name = name
+	hdr.Name = canonicalTarName(name, fi.IsDir())
 	if err := setHeaderForSpecialDevice(hdr, name, fi.Sys()); err != nil {
 		return nil, err
 	}
@@ -447,17 +443,14 @@ func newTarAppender(idMapping *idtools.IDMappings, writer io.Writer, chownOpts *
 
 // canonicalTarName provides a platform-independent and consistent posix-style
 //path for files and directories to be archived regardless of the platform.
-func canonicalTarName(name string, isDir bool) (string, error) {
-	name, err := CanonicalTarNameForPath(name)
-	if err != nil {
-		return "", err
-	}
+func canonicalTarName(name string, isDir bool) string {
+	name = CanonicalTarNameForPath(name)
 
 	// suffix with '/' for directories
 	if isDir && !strings.HasSuffix(name, "/") {
 		name += "/"
 	}
-	return name, nil
+	return name
 }
 
 // addTarFile adds to the tar archive a file from `path` as `name`

pkg/archive/archive_unix.go

@@ -32,8 +32,8 @@ func getWalkRoot(srcPath string, include string) string {
 // CanonicalTarNameForPath returns platform-specific filepath
 // to canonical posix-style path for tar archival. p is relative
 // path.
-func CanonicalTarNameForPath(p string) (string, error) {
-	return p, nil // already unix-style
+func CanonicalTarNameForPath(p string) string {
+	return p // already unix-style
 }
 
 // chmodTarEntry is used to adjust the file permissions used in tar header based

pkg/archive/archive_unix_test.go

@@ -26,10 +26,8 @@ func TestCanonicalTarNameForPath(t *testing.T) {
 		{"foo/dir/", "foo/dir/"},
 	}
 	for _, v := range cases {
-		if out, err := CanonicalTarNameForPath(v.in); err != nil {
-			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
-		} else if out != v.expected {
-			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		if CanonicalTarNameForPath(v.in) != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, CanonicalTarNameForPath(v.in))
 		}
 	}
 }
@@ -46,10 +44,8 @@ func TestCanonicalTarName(t *testing.T) {
 		{"foo/bar", true, "foo/bar/"},
 	}
 	for _, v := range cases {
-		if out, err := canonicalTarName(v.in, v.isDir); err != nil {
-			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
-		} else if out != v.expected {
-			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		if canonicalTarName(v.in, v.isDir) != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, canonicalTarName(v.in, v.isDir))
 		}
 	}
 }

pkg/archive/archive_windows.go

@@ -2,10 +2,8 @@ package archive // import "github.com/docker/docker/pkg/archive"
 
 import (
 	"archive/tar"
-	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/longpath"
@@ -26,16 +24,8 @@ func getWalkRoot(srcPath string, include string) string {
 // CanonicalTarNameForPath returns platform-specific filepath
 // to canonical posix-style path for tar archival. p is relative
 // path.
-func CanonicalTarNameForPath(p string) (string, error) {
-	// windows: convert windows style relative path with backslashes
-	// into forward slashes. Since windows does not allow '/' or '\'
-	// in file names, it is mostly safe to replace however we must
-	// check just in case
-	if strings.Contains(p, "/") {
-		return "", fmt.Errorf("Windows path contains forward slash: %s", p)
-	}
-	return strings.Replace(p, string(os.PathSeparator), "/", -1), nil
-
+func CanonicalTarNameForPath(p string) string {
+	return filepath.ToSlash(p)
 }
 
 // chmodTarEntry is used to adjust the file permissions used in tar header based

pkg/archive/archive_windows_test.go

@@ -36,19 +36,14 @@ func TestCopyFileWithInvalidDest(t *testing.T) {
 func TestCanonicalTarNameForPath(t *testing.T) {
 	cases := []struct {
 		in, expected string
-		shouldFail   bool
 	}{
-		{"foo", "foo", false},
-		{"foo/bar", "___", true}, // unix-styled windows path must fail
-		{`foo\bar`, "foo/bar", false},
+		{"foo", "foo"},
+		{"foo/bar", "foo/bar"},
+		{`foo\bar`, "foo/bar"},
 	}
 	for _, v := range cases {
-		if out, err := CanonicalTarNameForPath(v.in); err != nil && !v.shouldFail {
-			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
-		} else if v.shouldFail && err == nil {
-			t.Fatalf("canonical path call should have failed with error. in=%s out=%s", v.in, out)
-		} else if !v.shouldFail && out != v.expected {
-			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		if CanonicalTarNameForPath(v.in) != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, CanonicalTarNameForPath(v.in))
 		}
 	}
 }
@@ -65,10 +60,8 @@ func TestCanonicalTarName(t *testing.T) {
 		{`foo\bar`, true, "foo/bar/"},
 	}
 	for _, v := range cases {
-		if out, err := canonicalTarName(v.in, v.isDir); err != nil {
-			t.Fatalf("cannot get canonical name for path: %s: %v", v.in, err)
-		} else if out != v.expected {
-			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, out)
+		if canonicalTarName(v.in, v.isDir) != v.expected {
+			t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, canonicalTarName(v.in, v.isDir))
 		}
 	}
 }