Skip to content

Commit

Permalink
pkg/archive: remove backward compat hack for go < 1.9
Browse files Browse the repository at this point in the history
The fillGo18FileTypeBits func was added in 1a451d9
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 1a451d9.
This reverts commit 41eb61d.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Sep 1, 2022
1 parent c9c55df commit 6c5056a
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 50 deletions.
41 changes: 2 additions & 39 deletions pkg/archive/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
10 changes: 2 additions & 8 deletions pkg/archive/archive_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions pkg/archive/archive_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 6c5056a

Please sign in to comment.