go/packages/packagestest: make Export skip tests involving unsupported links
Also make Export create all parent directories before all files.
If the files are symlinks to directories, the target directory must
exist before the symlink is created on Windows. Otherwise, the symlink
will be created with the wrong type (and thus broken).
Fixes golang/go#46503
Updates golang/go#38772
Updates golang/go#39183
Change-Id: I17864ed66e0464e0ed1f56c55751b384b8c59484
Reviewed-on: https://go-review.googlesource.com/c/tools/+/234112
Trust: Bryan C. Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/go/packages/packagestest/export.go b/go/packages/packagestest/export.go
index 6b03926..2b93d2c 100644
--- a/go/packages/packagestest/export.go
+++ b/go/packages/packagestest/export.go
@@ -65,13 +65,16 @@
package packagestest
import (
+ "errors"
"flag"
"fmt"
"go/token"
+ "io"
"io/ioutil"
"log"
"os"
"path/filepath"
+ "runtime"
"strings"
"testing"
@@ -79,12 +82,17 @@
"golang.org/x/tools/go/packages"
"golang.org/x/tools/internal/span"
"golang.org/x/tools/internal/testenv"
+ "golang.org/x/xerrors"
)
var (
skipCleanup = flag.Bool("skip-cleanup", false, "Do not delete the temporary export folders") // for debugging
)
+// ErrUnsupported indicates an error due to an operation not supported on the
+// current platform.
+var ErrUnsupported = errors.New("operation is not supported")
+
// Module is a representation of a go module.
type Module struct {
// Name is the base name of the module as it would be in the go.mod file.
@@ -180,6 +188,9 @@
// The file deletion in the cleanup can be skipped by setting the skip-cleanup
// flag when invoking the test, allowing the temporary directory to be left for
// debugging tests.
+//
+// If the Writer for any file within any module returns an error equivalent to
+// ErrUnspported, Export skips the test.
func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
t.Helper()
if exporter == Modules {
@@ -213,6 +224,17 @@
}
}()
for _, module := range modules {
+ // Create all parent directories before individual files. If any file is a
+ // symlink to a directory, that directory must exist before the symlink is
+ // created or else it may be created with the wrong type on Windows.
+ // (See https://golang.org/issue/39183.)
+ for fragment := range module.Files {
+ fullpath := exporter.Filename(exported, module.Name, filepath.FromSlash(fragment))
+ if err := os.MkdirAll(filepath.Dir(fullpath), 0755); err != nil {
+ t.Fatal(err)
+ }
+ }
+
for fragment, value := range module.Files {
fullpath := exporter.Filename(exported, module.Name, filepath.FromSlash(fragment))
written, ok := exported.written[module.Name]
@@ -221,12 +243,12 @@
exported.written[module.Name] = written
}
written[fragment] = fullpath
- if err := os.MkdirAll(filepath.Dir(fullpath), 0755); err != nil {
- t.Fatal(err)
- }
switch value := value.(type) {
case Writer:
if err := value(fullpath); err != nil {
+ if xerrors.Is(err, ErrUnsupported) {
+ t.Skip(err)
+ }
t.Fatal(err)
}
case string:
@@ -261,28 +283,134 @@
// Link returns a Writer that creates a hard link from the specified source to
// the required file.
// This is used to link testdata files into the generated testing tree.
+//
+// If hard links to source are not supported on the destination filesystem, the
+// returned Writer returns an error for which errors.Is(_, ErrUnsupported)
+// returns true.
func Link(source string) Writer {
return func(filename string) error {
- return os.Link(source, filename)
+ linkErr := os.Link(source, filename)
+
+ if linkErr != nil && !builderMustSupportLinks() {
+ // Probe to figure out whether Link failed because the Link operation
+ // isn't supported.
+ if stat, err := openAndStat(source); err == nil {
+ if err := createEmpty(filename, stat.Mode()); err == nil {
+ // Successfully opened the source and created the destination,
+ // but the result is empty and not a hard-link.
+ return &os.PathError{Op: "Link", Path: filename, Err: ErrUnsupported}
+ }
+ }
+ }
+
+ return linkErr
}
}
// Symlink returns a Writer that creates a symlink from the specified source to the
// required file.
// This is used to link testdata files into the generated testing tree.
+//
+// If symlinks to source are not supported on the destination filesystem, the
+// returned Writer returns an error for which errors.Is(_, ErrUnsupported)
+// returns true.
func Symlink(source string) Writer {
if !strings.HasPrefix(source, ".") {
- if abspath, err := filepath.Abs(source); err == nil {
+ if absSource, err := filepath.Abs(source); err == nil {
if _, err := os.Stat(source); !os.IsNotExist(err) {
- source = abspath
+ source = absSource
}
}
}
return func(filename string) error {
- return os.Symlink(source, filename)
+ symlinkErr := os.Symlink(source, filename)
+
+ if symlinkErr != nil && !builderMustSupportLinks() {
+ // Probe to figure out whether Symlink failed because the Symlink
+ // operation isn't supported.
+ fullSource := source
+ if !filepath.IsAbs(source) {
+ // Compute the target path relative to the parent of filename, not the
+ // current working directory.
+ fullSource = filepath.Join(filename, "..", source)
+ }
+ stat, err := openAndStat(fullSource)
+ mode := os.ModePerm
+ if err == nil {
+ mode = stat.Mode()
+ } else if !xerrors.Is(err, os.ErrNotExist) {
+ // We couldn't open the source, but it might exist. We don't expect to be
+ // able to portably create a symlink to a file we can't see.
+ return symlinkErr
+ }
+
+ if err := createEmpty(filename, mode|0644); err == nil {
+ // Successfully opened the source (or verified that it does not exist) and
+ // created the destination, but we couldn't create it as a symlink.
+ // Probably the OS just doesn't support symlinks in this context.
+ return &os.PathError{Op: "Symlink", Path: filename, Err: ErrUnsupported}
+ }
+ }
+
+ return symlinkErr
}
}
+// builderMustSupportLinks reports whether we are running on a Go builder
+// that is known to support hard and symbolic links.
+func builderMustSupportLinks() bool {
+ if os.Getenv("GO_BUILDER_NAME") == "" {
+ // Any OS can be configured to mount an exotic filesystem.
+ // Don't make assumptions about what users are running.
+ return false
+ }
+
+ switch runtime.GOOS {
+ case "windows", "plan9":
+ // Some versions of Windows and all versions of plan9 do not support
+ // symlinks by default.
+ return false
+
+ default:
+ // All other platforms should support symlinks by default, and our builders
+ // should not do anything unusual that would violate that.
+ return true
+ }
+}
+
+// openAndStat attempts to open source for reading.
+func openAndStat(source string) (os.FileInfo, error) {
+ src, err := os.Open(source)
+ if err != nil {
+ return nil, err
+ }
+ stat, err := src.Stat()
+ src.Close()
+ if err != nil {
+ return nil, err
+ }
+ return stat, nil
+}
+
+// createEmpty creates an empty file or directory (depending on mode)
+// at dst, with the same permissions as mode.
+func createEmpty(dst string, mode os.FileMode) error {
+ if mode.IsDir() {
+ return os.Mkdir(dst, mode.Perm())
+ }
+
+ f, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode.Perm())
+ if err != nil {
+ return err
+ }
+ if err := f.Close(); err != nil {
+ os.Remove(dst) // best-effort
+ return err
+ }
+
+ return nil
+}
+
// Copy returns a Writer that copies a file from the specified source to the
// required file.
// This is used to copy testdata files into the generated testing tree.
@@ -297,14 +425,29 @@
// symlinks, devices, etc.)
return fmt.Errorf("cannot copy non regular file %s", source)
}
- contents, err := ioutil.ReadFile(source)
- if err != nil {
- return err
- }
- return ioutil.WriteFile(filename, contents, stat.Mode())
+ return copyFile(filename, source, stat.Mode().Perm())
}
}
+func copyFile(dest, source string, perm os.FileMode) error {
+ src, err := os.Open(source)
+ if err != nil {
+ return err
+ }
+ defer src.Close()
+
+ dst, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
+ if err != nil {
+ return err
+ }
+
+ _, err = io.Copy(dst, src)
+ if closeErr := dst.Close(); err == nil {
+ err = closeErr
+ }
+ return err
+}
+
// GroupFilesByModules attempts to map directories to the modules within each directory.
// This function assumes that the folder is structured in the following way:
// - dir
diff --git a/go/packages/packagestest/export_test.go b/go/packages/packagestest/export_test.go
index 36f6397..356dd4b 100644
--- a/go/packages/packagestest/export_test.go
+++ b/go/packages/packagestest/export_test.go
@@ -15,7 +15,7 @@
var testdata = []packagestest.Module{{
Name: "golang.org/fake1",
Files: map[string]interface{}{
- "a.go": packagestest.Symlink("testdata/a.go"),
+ "a.go": packagestest.Symlink("testdata/a.go"), // broken symlink
"b.go": "invalid file contents",
},
Overlay: map[string][]byte{
diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go
index 005bf96..bfd3cfa 100644
--- a/internal/imports/fix_test.go
+++ b/internal/imports/fix_test.go
@@ -13,7 +13,6 @@
"log"
"path/filepath"
"reflect"
- "runtime"
"sort"
"strings"
"sync"
@@ -1307,11 +1306,6 @@
// Test support for packages in GOPATH that are actually symlinks.
// Also test that a symlink loop does not block the process.
func TestImportSymlinks(t *testing.T) {
- switch runtime.GOOS {
- case "windows", "plan9":
- t.Skipf("skipping test on %q as there are no symlinks", runtime.GOOS)
- }
-
const input = `package p
var (
@@ -1347,11 +1341,6 @@
}
func TestImportSymlinksWithIgnore(t *testing.T) {
- switch runtime.GOOS {
- case "windows", "plan9":
- t.Skipf("skipping test on %q as there are no symlinks", runtime.GOOS)
- }
-
const input = `package p
var (
