From d59758450b6bb876867beadc5cd7be2d1805687c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 30 Aug 2022 10:40:27 +0200 Subject: [PATCH 1/2] pkg/archive: make CanonicalTarNameForPath and alias for filepath.ToSlash filepath.ToSlash is already a no-op on non-Windows platforms, so there's no need to provide multiple implementations. We could consider deprecating this function, but it's used in the CLI, and perhaps it's still useful to have a canonical location to perform this normalization. Signed-off-by: Sebastiaan van Stijn --- pkg/archive/archive.go | 11 +++++++++-- pkg/archive/archive_unix.go | 8 -------- pkg/archive/archive_windows.go | 7 ------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index fe87093054..3e56a53ade 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -555,10 +555,17 @@ func newTarAppender(idMapping idtools.IdentityMapping, writer io.Writer, chownOp } } -// canonicalTarName provides a platform-independent and consistent posix-style +// CanonicalTarNameForPath canonicalizes relativePath to a POSIX-style path using +// forward slashes. It is an alias for filepath.ToSlash, which is a no-op on +// Linux and Unix. +func CanonicalTarNameForPath(relativePath string) string { + return filepath.ToSlash(relativePath) +} + +// 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 { - name = CanonicalTarNameForPath(name) + name = filepath.ToSlash(name) // suffix with '/' for directories if isDir && !strings.HasSuffix(name, "/") { diff --git a/pkg/archive/archive_unix.go b/pkg/archive/archive_unix.go index 1a2aea2e65..92d8e23dd0 100644 --- a/pkg/archive/archive_unix.go +++ b/pkg/archive/archive_unix.go @@ -35,16 +35,8 @@ func getWalkRoot(srcPath string, include string) string { return strings.TrimSuffix(srcPath, string(filepath.Separator)) + string(filepath.Separator) + include } -// CanonicalTarNameForPath returns platform-specific filepath -// to canonical posix-style path for tar archival. p is relative -// path. -func CanonicalTarNameForPath(p string) string { - return p // already unix-style -} - // chmodTarEntry is used to adjust the file permissions used in tar header based // on the platform the archival is done. - func chmodTarEntry(perm os.FileMode) os.FileMode { return perm // noop for unix as golang APIs provide perm bits correctly } diff --git a/pkg/archive/archive_windows.go b/pkg/archive/archive_windows.go index 7260174bfb..c68acdb6ee 100644 --- a/pkg/archive/archive_windows.go +++ b/pkg/archive/archive_windows.go @@ -21,13 +21,6 @@ func getWalkRoot(srcPath string, include string) string { return filepath.Join(srcPath, include) } -// CanonicalTarNameForPath returns platform-specific filepath -// to canonical posix-style path for tar archival. p is relative -// path. -func CanonicalTarNameForPath(p string) string { - return filepath.ToSlash(p) -} - // chmodTarEntry is used to adjust the file permissions used in tar header based // on the platform the archival is done. func chmodTarEntry(perm os.FileMode) os.FileMode { From 8b36298d7fa8d906c3528df4cbdee3a8bfa0493a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 30 Aug 2022 10:41:53 +0200 Subject: [PATCH 2/2] pkg/archive: remove tests for CanonicalTarNameForPath Now that CanonicalTarNameForPath is an alias for filepath.ToSlash, they were mostly redundant, and only testing Go's stdlib. Coverage for filepath.ToSlash is provided through TestCanonicalTarName, which does a superset of CanonicalTarNameForPath, Signed-off-by: Sebastiaan van Stijn --- pkg/archive/archive_unix_test.go | 13 ------------- pkg/archive/archive_windows_test.go | 15 --------------- 2 files changed, 28 deletions(-) diff --git a/pkg/archive/archive_unix_test.go b/pkg/archive/archive_unix_test.go index ac3833fd55..2e9726b729 100644 --- a/pkg/archive/archive_unix_test.go +++ b/pkg/archive/archive_unix_test.go @@ -23,19 +23,6 @@ import ( "gotest.tools/v3/skip" ) -func TestCanonicalTarNameForPath(t *testing.T) { - cases := []struct{ in, expected string }{ - {"foo", "foo"}, - {"foo/bar", "foo/bar"}, - {"foo/dir/", "foo/dir/"}, - } - for _, v := range cases { - if CanonicalTarNameForPath(v.in) != v.expected { - t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, CanonicalTarNameForPath(v.in)) - } - } -} - func TestCanonicalTarName(t *testing.T) { cases := []struct { in string diff --git a/pkg/archive/archive_windows_test.go b/pkg/archive/archive_windows_test.go index f03b1e2d82..92b2c49c55 100644 --- a/pkg/archive/archive_windows_test.go +++ b/pkg/archive/archive_windows_test.go @@ -33,21 +33,6 @@ func TestCopyFileWithInvalidDest(t *testing.T) { } } -func TestCanonicalTarNameForPath(t *testing.T) { - cases := []struct { - in, expected string - }{ - {"foo", "foo"}, - {"foo/bar", "foo/bar"}, - {`foo\bar`, "foo/bar"}, - } - for _, v := range cases { - if CanonicalTarNameForPath(v.in) != v.expected { - t.Fatalf("wrong canonical tar name. expected:%s got:%s", v.expected, CanonicalTarNameForPath(v.in)) - } - } -} - func TestCanonicalTarName(t *testing.T) { cases := []struct { in string