From d7961f4caa26b030e7aba76153188a1aad74437e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 26 Feb 2020 20:41:51 -0800 Subject: [PATCH 1/3] travis.yml: rm unsupported go releases, add 1.14 Signed-off-by: Kir Kolyshkin --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6248f031..fcfab637 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,9 +2,8 @@ language: go sudo: required go: - - 1.11.x - - 1.12.x - 1.13.x + - 1.14.x - tip go_import_path: github.com/containerd/continuity From da42a3033a39c971c1336a8a9f70fbf323857374 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 26 Feb 2020 20:36:43 -0800 Subject: [PATCH 2/3] driver: fail to build on Windows with go < 1.13 The next commit requires go 1.13 for Windows, so let's guard against building this package with earlier golang releases. Note that since go 1.14 is out, go 1.12 is no longer supported. Signed-off-by: Kir Kolyshkin --- driver/driver_windows.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/driver/driver_windows.go b/driver/driver_windows.go index f1dcea32..80fc9b1e 100644 --- a/driver/driver_windows.go +++ b/driver/driver_windows.go @@ -14,6 +14,12 @@ limitations under the License. */ +// +build go1.13 + +// Go 1.13 is the minimally supported version for Windows. +// Earlier golang releases have bug in os.Readlink +// (see https://github.com/golang/go/issues/30463). + package driver import ( From 643e66e4bb3e30dda0a35b6468fb5f35cde7d856 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 26 Feb 2020 20:19:30 -0800 Subject: [PATCH 3/3] Remove Windows' Readlink fork This reverts the part of commit 8100e750d (PR #113) that added the fork of Readlink() for Windows. At that time the fork was done to work around the bug in golang's implementation of os.Readlink() for Windows. The above bug was never reported upstream, but fortunately it was independently found, reported [1], and fixed [2]. The fix made its way into go-1.13 (there is no mention of that in release notes, so I checked it in git history). [1] https://github.com/golang/go/issues/30463 [2] https://go-review.googlesource.com/c/go/+/164201 Signed-off-by: Kir Kolyshkin --- driver/driver.go | 4 ++ driver/driver_unix.go | 5 -- driver/driver_windows.go | 8 --- syscallx/syscall_unix.go | 26 -------- syscallx/syscall_windows.go | 112 ------------------------------- sysx/file_posix.go | 128 ------------------------------------ 6 files changed, 4 insertions(+), 279 deletions(-) delete mode 100644 syscallx/syscall_unix.go delete mode 100644 syscallx/syscall_windows.go delete mode 100644 sysx/file_posix.go diff --git a/driver/driver.go b/driver/driver.go index 327e96af..e5d9d0f8 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -138,6 +138,10 @@ func (d *driver) Lstat(p string) (os.FileInfo, error) { return os.Lstat(p) } +func (d *driver) Readlink(p string) (string, error) { + return os.Readlink(p) +} + func (d *driver) Mkdir(p string, mode os.FileMode) error { return os.Mkdir(p, mode) } diff --git a/driver/driver_unix.go b/driver/driver_unix.go index 6cb5d10f..3e58d10a 100644 --- a/driver/driver_unix.go +++ b/driver/driver_unix.go @@ -131,8 +131,3 @@ func (d *driver) LSetxattr(path string, attrMap map[string][]byte) error { func (d *driver) DeviceInfo(fi os.FileInfo) (maj uint64, min uint64, err error) { return devices.DeviceInfo(fi) } - -// Readlink was forked on Windows to fix a Golang bug, use the "os" package here -func (d *driver) Readlink(p string) (string, error) { - return os.Readlink(p) -} diff --git a/driver/driver_windows.go b/driver/driver_windows.go index 80fc9b1e..c2a9a3b8 100644 --- a/driver/driver_windows.go +++ b/driver/driver_windows.go @@ -24,8 +24,6 @@ package driver import ( "os" - - "github.com/containerd/continuity/sysx" ) func (d *driver) Mknod(path string, mode os.FileMode, major, minor int) error { @@ -41,9 +39,3 @@ func (d *driver) Lchmod(path string, mode os.FileMode) (err error) { // TODO: Use Window's equivalent return os.Chmod(path, mode) } - -// Readlink is forked in order to support Volume paths which are used -// in container layers. -func (d *driver) Readlink(p string) (string, error) { - return sysx.Readlink(p) -} diff --git a/syscallx/syscall_unix.go b/syscallx/syscall_unix.go deleted file mode 100644 index 0bfa6a04..00000000 --- a/syscallx/syscall_unix.go +++ /dev/null @@ -1,26 +0,0 @@ -// +build !windows - -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package syscallx - -import "syscall" - -// Readlink returns the destination of the named symbolic link. -func Readlink(path string, buf []byte) (n int, err error) { - return syscall.Readlink(path, buf) -} diff --git a/syscallx/syscall_windows.go b/syscallx/syscall_windows.go deleted file mode 100644 index 2ba81499..00000000 --- a/syscallx/syscall_windows.go +++ /dev/null @@ -1,112 +0,0 @@ -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package syscallx - -import ( - "syscall" - "unsafe" -) - -type reparseDataBuffer struct { - ReparseTag uint32 - ReparseDataLength uint16 - Reserved uint16 - - // GenericReparseBuffer - reparseBuffer byte -} - -type mountPointReparseBuffer struct { - SubstituteNameOffset uint16 - SubstituteNameLength uint16 - PrintNameOffset uint16 - PrintNameLength uint16 - PathBuffer [1]uint16 -} - -type symbolicLinkReparseBuffer struct { - SubstituteNameOffset uint16 - SubstituteNameLength uint16 - PrintNameOffset uint16 - PrintNameLength uint16 - Flags uint32 - PathBuffer [1]uint16 -} - -const ( - _IO_REPARSE_TAG_MOUNT_POINT = 0xA0000003 - _SYMLINK_FLAG_RELATIVE = 1 -) - -// Readlink returns the destination of the named symbolic link. -func Readlink(path string, buf []byte) (n int, err error) { - fd, err := syscall.CreateFile(syscall.StringToUTF16Ptr(path), syscall.GENERIC_READ, 0, nil, syscall.OPEN_EXISTING, - syscall.FILE_FLAG_OPEN_REPARSE_POINT|syscall.FILE_FLAG_BACKUP_SEMANTICS, 0) - if err != nil { - return -1, err - } - defer syscall.CloseHandle(fd) - - rdbbuf := make([]byte, syscall.MAXIMUM_REPARSE_DATA_BUFFER_SIZE) - var bytesReturned uint32 - err = syscall.DeviceIoControl(fd, syscall.FSCTL_GET_REPARSE_POINT, nil, 0, &rdbbuf[0], uint32(len(rdbbuf)), &bytesReturned, nil) - if err != nil { - return -1, err - } - - rdb := (*reparseDataBuffer)(unsafe.Pointer(&rdbbuf[0])) - var s string - switch rdb.ReparseTag { - case syscall.IO_REPARSE_TAG_SYMLINK: - data := (*symbolicLinkReparseBuffer)(unsafe.Pointer(&rdb.reparseBuffer)) - p := (*[0xffff]uint16)(unsafe.Pointer(&data.PathBuffer[0])) - s = syscall.UTF16ToString(p[data.SubstituteNameOffset/2 : (data.SubstituteNameOffset+data.SubstituteNameLength)/2]) - if data.Flags&_SYMLINK_FLAG_RELATIVE == 0 { - if len(s) >= 4 && s[:4] == `\??\` { - s = s[4:] - switch { - case len(s) >= 2 && s[1] == ':': // \??\C:\foo\bar - // do nothing - case len(s) >= 4 && s[:4] == `UNC\`: // \??\UNC\foo\bar - s = `\\` + s[4:] - default: - // unexpected; do nothing - } - } else { - // unexpected; do nothing - } - } - case _IO_REPARSE_TAG_MOUNT_POINT: - data := (*mountPointReparseBuffer)(unsafe.Pointer(&rdb.reparseBuffer)) - p := (*[0xffff]uint16)(unsafe.Pointer(&data.PathBuffer[0])) - s = syscall.UTF16ToString(p[data.SubstituteNameOffset/2 : (data.SubstituteNameOffset+data.SubstituteNameLength)/2]) - if len(s) >= 4 && s[:4] == `\??\` { // \??\C:\foo\bar - if len(s) < 48 || s[:11] != `\??\Volume{` { - s = s[4:] - } - } else { - // unexpected; do nothing - } - default: - // the path is not a symlink or junction but another type of reparse - // point - return -1, syscall.ENOENT - } - n = copy(buf, []byte(s)) - - return n, nil -} diff --git a/sysx/file_posix.go b/sysx/file_posix.go deleted file mode 100644 index e28f3a1b..00000000 --- a/sysx/file_posix.go +++ /dev/null @@ -1,128 +0,0 @@ -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package sysx - -import ( - "os" - "path/filepath" - - "github.com/containerd/continuity/syscallx" -) - -// Readlink returns the destination of the named symbolic link. -// If there is an error, it will be of type *PathError. -func Readlink(name string) (string, error) { - for len := 128; ; len *= 2 { - b := make([]byte, len) - n, e := fixCount(syscallx.Readlink(fixLongPath(name), b)) - if e != nil { - return "", &os.PathError{Op: "readlink", Path: name, Err: e} - } - if n < len { - return string(b[0:n]), nil - } - } -} - -// Many functions in package syscall return a count of -1 instead of 0. -// Using fixCount(call()) instead of call() corrects the count. -func fixCount(n int, err error) (int, error) { - if n < 0 { - n = 0 - } - return n, err -} - -// fixLongPath returns the extended-length (\\?\-prefixed) form of -// path when needed, in order to avoid the default 260 character file -// path limit imposed by Windows. If path is not easily converted to -// the extended-length form (for example, if path is a relative path -// or contains .. elements), or is short enough, fixLongPath returns -// path unmodified. -// -// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath -func fixLongPath(path string) string { - // Do nothing (and don't allocate) if the path is "short". - // Empirically (at least on the Windows Server 2013 builder), - // the kernel is arbitrarily okay with < 248 bytes. That - // matches what the docs above say: - // "When using an API to create a directory, the specified - // path cannot be so long that you cannot append an 8.3 file - // name (that is, the directory name cannot exceed MAX_PATH - // minus 12)." Since MAX_PATH is 260, 260 - 12 = 248. - // - // The MSDN docs appear to say that a normal path that is 248 bytes long - // will work; empirically the path must be less then 248 bytes long. - if len(path) < 248 { - // Don't fix. (This is how Go 1.7 and earlier worked, - // not automatically generating the \\?\ form) - return path - } - - // The extended form begins with \\?\, as in - // \\?\c:\windows\foo.txt or \\?\UNC\server\share\foo.txt. - // The extended form disables evaluation of . and .. path - // elements and disables the interpretation of / as equivalent - // to \. The conversion here rewrites / to \ and elides - // . elements as well as trailing or duplicate separators. For - // simplicity it avoids the conversion entirely for relative - // paths or paths containing .. elements. For now, - // \\server\share paths are not converted to - // \\?\UNC\server\share paths because the rules for doing so - // are less well-specified. - if len(path) >= 2 && path[:2] == `\\` { - // Don't canonicalize UNC paths. - return path - } - if !filepath.IsAbs(path) { - // Relative path - return path - } - - const prefix = `\\?` - - pathbuf := make([]byte, len(prefix)+len(path)+len(`\`)) - copy(pathbuf, prefix) - n := len(path) - r, w := 0, len(prefix) - for r < n { - switch { - case os.IsPathSeparator(path[r]): - // empty block - r++ - case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])): - // /./ - r++ - case r+1 < n && path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])): - // /../ is currently unhandled - return path - default: - pathbuf[w] = '\\' - w++ - for ; r < n && !os.IsPathSeparator(path[r]); r++ { - pathbuf[w] = path[r] - w++ - } - } - } - // A drive's root directory needs a trailing \ - if w == len(`\\?\c:`) { - pathbuf[w] = '\\' - w++ - } - return string(pathbuf[:w]) -}