Skip to content

Commit

Permalink
go/packages/packagestest: make Export skip tests involving unsupporte…
Browse files Browse the repository at this point in the history
…d 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>
  • Loading branch information
Bryan C. Mills committed Jun 2, 2021

Verified

This commit was signed with the committer’s verified signature. The key has expired.
ulyssa Ulyssa
1 parent 726034e commit 7922491
Showing 3 changed files with 156 additions and 24 deletions.
167 changes: 155 additions & 12 deletions go/packages/packagestest/export.go
Original file line number Diff line number Diff line change
@@ -65,26 +65,34 @@ 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"

"golang.org/x/tools/go/expect"
"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.
2 changes: 1 addition & 1 deletion go/packages/packagestest/export_test.go
Original file line number Diff line number Diff line change
@@ -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{
11 changes: 0 additions & 11 deletions internal/imports/fix_test.go
Original file line number Diff line number Diff line change
@@ -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 (

0 comments on commit 7922491

Please sign in to comment.