From cddaa8477786011227677a8a220de0a380387d02 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 30 Aug 2022 15:52:32 +0200 Subject: [PATCH] pkg/archive: remove backward compat hack for go < 1.9 The fillGo18FileTypeBits func was added in 1a451d9a7bb9cd7d437b42d4e73b0560fbf84348 to keep the tar headers consistent with headers created with go1.8 and older. go1.8 and older incorrectly preserved all file-mode bits, including file-type, instead of stripping those bits and only preserving the _permission_ bits, as defined in; - the GNU tar spec: https://www.gnu.org/software/tar/manual/html_node/Standard.html - and POSIX: http://pubs.opengroup.org/onlinepubs/009695399/basedefs/tar.h.html We decided at the time to copy the "wrong" behavior to prevent a cache-bust and to keep the archives identical, however: - It's not matching the standards, which causes differences between our tar implementation and the standard tar implementations, as well as implementations in other languages, such as Python (see docker/compose#883). - BuildKit does not implement this hack. - We don't _need_ this extra information (as it's already preserved in the type header; https://pkg.go.dev/archive/tar#pkg-constants In short; let's remove this hack. This reverts commit 1a451d9a7bb9cd7d437b42d4e73b0560fbf84348. This reverts commit 41eb61d5c2a44c745b11ada948034fd3bcc42db5. Signed-off-by: Sebastiaan van Stijn --- builder/remotecontext/tarsum_test.go | 4 +-- pkg/archive/archive.go | 41 ++-------------------------- pkg/archive/archive_windows.go | 10 ++----- pkg/archive/archive_windows_test.go | 4 +-- 4 files changed, 7 insertions(+), 52 deletions(-) diff --git a/builder/remotecontext/tarsum_test.go b/builder/remotecontext/tarsum_test.go index fd5d9edc2f..64b7f1d5f2 100644 --- a/builder/remotecontext/tarsum_test.go +++ b/builder/remotecontext/tarsum_test.go @@ -60,7 +60,7 @@ func TestHashFile(t *testing.T) { t.Fatalf("Hash returned empty sum") } - expected := "1149ab94af7be6cc1da1335e398f24ee1cf4926b720044d229969dfc248ae7ec" + expected := "55dfeb344351ab27f59aa60ebb0ed12025a2f2f4677bf77d26ea7a671274a9ca" if actual := sum; expected != actual { t.Fatalf("invalid checksum. expected %s, got %s", expected, actual) @@ -97,7 +97,7 @@ func TestHashSubdir(t *testing.T) { t.Fatalf("Hash returned empty sum") } - expected := "d7f8d6353dee4816f9134f4156bf6a9d470fdadfb5d89213721f7e86744a4e69" + expected := "74a3326b8e766ce63a8e5232f22e9dd895be647fb3ca7d337e5e0a9b3da8ef28" if actual := sum; expected != actual { t.Fatalf("invalid checksum. expected %s, got %s", expected, actual) diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 6d8464b60b..bd8ce64a0b 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -99,16 +99,6 @@ const ( OverlayWhiteoutFormat ) -const ( - modeISDIR = 040000 // Directory - modeISFIFO = 010000 // FIFO - modeISREG = 0100000 // Regular file - modeISLNK = 0120000 // Symbolic link - modeISBLK = 060000 // Block special file - modeISCHR = 020000 // Character special file - modeISSOCK = 0140000 // Socket -) - // IsArchivePath checks if the (possibly compressed) file at the given path // starts with a tar file header. func IsArchivePath(path string) bool { @@ -458,9 +448,7 @@ func FileInfoHeaderNoLookups(fi os.FileInfo, link string) (*tar.Header, error) { // but is safe to call from a chrooted process. The AccessTime and ChangeTime // fields are not set in the returned header, ModTime is truncated to one-second // precision, and the Uname and Gname fields are only set when fi is a FileInfo -// value returned from tar.Header.FileInfo(). Also, regardless of Go version, -// this function fills file type bits (e.g. hdr.Mode |= modeISDIR), which have -// been deleted since Go 1.9 archive/tar. +// value returned from tar.Header.FileInfo(). func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, error) { hdr, err := FileInfoHeaderNoLookups(fi, link) if err != nil { @@ -470,36 +458,11 @@ func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, erro hdr.ModTime = hdr.ModTime.Truncate(time.Second) hdr.AccessTime = time.Time{} hdr.ChangeTime = time.Time{} - hdr.Mode = fillGo18FileTypeBits(int64(chmodTarEntry(os.FileMode(hdr.Mode))), fi) + hdr.Mode = int64(chmodTarEntry(os.FileMode(hdr.Mode))) hdr.Name = canonicalTarName(name, fi.IsDir()) return hdr, nil } -// fillGo18FileTypeBits fills type bits which have been removed on Go 1.9 archive/tar -// https://github.com/golang/go/commit/66b5a2f -func fillGo18FileTypeBits(mode int64, fi os.FileInfo) int64 { - fm := fi.Mode() - switch { - case fm.IsRegular(): - mode |= modeISREG - case fi.IsDir(): - mode |= modeISDIR - case fm&os.ModeSymlink != 0: - mode |= modeISLNK - case fm&os.ModeDevice != 0: - if fm&os.ModeCharDevice != 0 { - mode |= modeISCHR - } else { - mode |= modeISBLK - } - case fm&os.ModeNamedPipe != 0: - mode |= modeISFIFO - case fm&os.ModeSocket != 0: - mode |= modeISSOCK - } - return mode -} - // ReadSecurityXattrToTarHeader reads security.capability xattr from filesystem // to a tar header func ReadSecurityXattrToTarHeader(path string, hdr *tar.Header) error { diff --git a/pkg/archive/archive_windows.go b/pkg/archive/archive_windows.go index 7260174bfb..ef0c7ddcd7 100644 --- a/pkg/archive/archive_windows.go +++ b/pkg/archive/archive_windows.go @@ -31,14 +31,8 @@ func CanonicalTarNameForPath(p string) string { // 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 { - // perm &= 0755 // this 0-ed out tar flags (like link, regular file, directory marker etc.) - permPart := perm & os.ModePerm - noPermPart := perm &^ os.ModePerm - // Add the x bit: make everything +x from windows - permPart |= 0111 - permPart &= 0755 - - return noPermPart | permPart + // Add the x bit: make everything +x on Windows + return perm | 0111 } func setHeaderForSpecialDevice(hdr *tar.Header, name string, stat interface{}) (err error) { diff --git a/pkg/archive/archive_windows_test.go b/pkg/archive/archive_windows_test.go index f03b1e2d82..a34e648804 100644 --- a/pkg/archive/archive_windows_test.go +++ b/pkg/archive/archive_windows_test.go @@ -71,12 +71,10 @@ func TestChmodTarEntry(t *testing.T) { in, expected os.FileMode }{ {0000, 0111}, - {0777, 0755}, + {0777, 0777}, {0644, 0755}, {0755, 0755}, {0444, 0555}, - {0755 | os.ModeDir, 0755 | os.ModeDir}, - {0755 | os.ModeSymlink, 0755 | os.ModeSymlink}, } for _, v := range cases { if out := chmodTarEntry(v.in); out != v.expected {