diff --git a/go/packages/packagestest/export.go b/go/packages/packagestest/export.go index 6b03926d923..2b93d2c2b95 100644 --- a/go/packages/packagestest/export.go +++ b/go/packages/packagestest/export.go @@ -65,13 +65,16 @@ Running the test with verbose output will print: package packagestest import ( + "errors" "flag" "fmt" "go/token" + "io" "io/ioutil" "log" "os" "path/filepath" + "runtime" "strings" "testing" @@ -79,12 +82,17 @@ import ( "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 @@ func BenchmarkAll(b *testing.B, f func(*testing.B, Exporter)) { // 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 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported { } }() 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 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported { 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,26 +283,132 @@ func Script(contents string) Writer { // 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 @@ -297,12 +425,27 @@ func Copy(source string) Writer { // 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. diff --git a/go/packages/packagestest/export_test.go b/go/packages/packagestest/export_test.go index 36f63975059..356dd4b524d 100644 --- a/go/packages/packagestest/export_test.go +++ b/go/packages/packagestest/export_test.go @@ -15,7 +15,7 @@ import ( 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 005bf96e7e6..bfd3cfa1776 100644 --- a/internal/imports/fix_test.go +++ b/internal/imports/fix_test.go @@ -13,7 +13,6 @@ import ( "log" "path/filepath" "reflect" - "runtime" "sort" "strings" "sync" @@ -1307,11 +1306,6 @@ func bar() { // 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 @@ var ( } 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 (