Skip to content

Commit 7922491

Browse files
author
Bryan C. Mills
committed
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 <[email protected]> gopls-CI: kokoro <[email protected]> Run-TryBot: Bryan C. Mills <[email protected]> Reviewed-by: Ian Cottrell <[email protected]>
1 parent 726034e commit 7922491

File tree

3 files changed

+156
-24
lines changed

3 files changed

+156
-24
lines changed

go/packages/packagestest/export.go

+155-12
Original file line numberDiff line numberDiff line change
@@ -65,26 +65,34 @@ Running the test with verbose output will print:
6565
package packagestest
6666

6767
import (
68+
"errors"
6869
"flag"
6970
"fmt"
7071
"go/token"
72+
"io"
7173
"io/ioutil"
7274
"log"
7375
"os"
7476
"path/filepath"
77+
"runtime"
7578
"strings"
7679
"testing"
7780

7881
"golang.org/x/tools/go/expect"
7982
"golang.org/x/tools/go/packages"
8083
"golang.org/x/tools/internal/span"
8184
"golang.org/x/tools/internal/testenv"
85+
"golang.org/x/xerrors"
8286
)
8387

8488
var (
8589
skipCleanup = flag.Bool("skip-cleanup", false, "Do not delete the temporary export folders") // for debugging
8690
)
8791

92+
// ErrUnsupported indicates an error due to an operation not supported on the
93+
// current platform.
94+
var ErrUnsupported = errors.New("operation is not supported")
95+
8896
// Module is a representation of a go module.
8997
type Module struct {
9098
// 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)) {
180188
// The file deletion in the cleanup can be skipped by setting the skip-cleanup
181189
// flag when invoking the test, allowing the temporary directory to be left for
182190
// debugging tests.
191+
//
192+
// If the Writer for any file within any module returns an error equivalent to
193+
// ErrUnspported, Export skips the test.
183194
func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
184195
t.Helper()
185196
if exporter == Modules {
@@ -213,6 +224,17 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
213224
}
214225
}()
215226
for _, module := range modules {
227+
// Create all parent directories before individual files. If any file is a
228+
// symlink to a directory, that directory must exist before the symlink is
229+
// created or else it may be created with the wrong type on Windows.
230+
// (See https://golang.org/issue/39183.)
231+
for fragment := range module.Files {
232+
fullpath := exporter.Filename(exported, module.Name, filepath.FromSlash(fragment))
233+
if err := os.MkdirAll(filepath.Dir(fullpath), 0755); err != nil {
234+
t.Fatal(err)
235+
}
236+
}
237+
216238
for fragment, value := range module.Files {
217239
fullpath := exporter.Filename(exported, module.Name, filepath.FromSlash(fragment))
218240
written, ok := exported.written[module.Name]
@@ -221,12 +243,12 @@ func Export(t testing.TB, exporter Exporter, modules []Module) *Exported {
221243
exported.written[module.Name] = written
222244
}
223245
written[fragment] = fullpath
224-
if err := os.MkdirAll(filepath.Dir(fullpath), 0755); err != nil {
225-
t.Fatal(err)
226-
}
227246
switch value := value.(type) {
228247
case Writer:
229248
if err := value(fullpath); err != nil {
249+
if xerrors.Is(err, ErrUnsupported) {
250+
t.Skip(err)
251+
}
230252
t.Fatal(err)
231253
}
232254
case string:
@@ -261,26 +283,132 @@ func Script(contents string) Writer {
261283
// Link returns a Writer that creates a hard link from the specified source to
262284
// the required file.
263285
// This is used to link testdata files into the generated testing tree.
286+
//
287+
// If hard links to source are not supported on the destination filesystem, the
288+
// returned Writer returns an error for which errors.Is(_, ErrUnsupported)
289+
// returns true.
264290
func Link(source string) Writer {
265291
return func(filename string) error {
266-
return os.Link(source, filename)
292+
linkErr := os.Link(source, filename)
293+
294+
if linkErr != nil && !builderMustSupportLinks() {
295+
// Probe to figure out whether Link failed because the Link operation
296+
// isn't supported.
297+
if stat, err := openAndStat(source); err == nil {
298+
if err := createEmpty(filename, stat.Mode()); err == nil {
299+
// Successfully opened the source and created the destination,
300+
// but the result is empty and not a hard-link.
301+
return &os.PathError{Op: "Link", Path: filename, Err: ErrUnsupported}
302+
}
303+
}
304+
}
305+
306+
return linkErr
267307
}
268308
}
269309

270310
// Symlink returns a Writer that creates a symlink from the specified source to the
271311
// required file.
272312
// This is used to link testdata files into the generated testing tree.
313+
//
314+
// If symlinks to source are not supported on the destination filesystem, the
315+
// returned Writer returns an error for which errors.Is(_, ErrUnsupported)
316+
// returns true.
273317
func Symlink(source string) Writer {
274318
if !strings.HasPrefix(source, ".") {
275-
if abspath, err := filepath.Abs(source); err == nil {
319+
if absSource, err := filepath.Abs(source); err == nil {
276320
if _, err := os.Stat(source); !os.IsNotExist(err) {
277-
source = abspath
321+
source = absSource
278322
}
279323
}
280324
}
281325
return func(filename string) error {
282-
return os.Symlink(source, filename)
326+
symlinkErr := os.Symlink(source, filename)
327+
328+
if symlinkErr != nil && !builderMustSupportLinks() {
329+
// Probe to figure out whether Symlink failed because the Symlink
330+
// operation isn't supported.
331+
fullSource := source
332+
if !filepath.IsAbs(source) {
333+
// Compute the target path relative to the parent of filename, not the
334+
// current working directory.
335+
fullSource = filepath.Join(filename, "..", source)
336+
}
337+
stat, err := openAndStat(fullSource)
338+
mode := os.ModePerm
339+
if err == nil {
340+
mode = stat.Mode()
341+
} else if !xerrors.Is(err, os.ErrNotExist) {
342+
// We couldn't open the source, but it might exist. We don't expect to be
343+
// able to portably create a symlink to a file we can't see.
344+
return symlinkErr
345+
}
346+
347+
if err := createEmpty(filename, mode|0644); err == nil {
348+
// Successfully opened the source (or verified that it does not exist) and
349+
// created the destination, but we couldn't create it as a symlink.
350+
// Probably the OS just doesn't support symlinks in this context.
351+
return &os.PathError{Op: "Symlink", Path: filename, Err: ErrUnsupported}
352+
}
353+
}
354+
355+
return symlinkErr
356+
}
357+
}
358+
359+
// builderMustSupportLinks reports whether we are running on a Go builder
360+
// that is known to support hard and symbolic links.
361+
func builderMustSupportLinks() bool {
362+
if os.Getenv("GO_BUILDER_NAME") == "" {
363+
// Any OS can be configured to mount an exotic filesystem.
364+
// Don't make assumptions about what users are running.
365+
return false
366+
}
367+
368+
switch runtime.GOOS {
369+
case "windows", "plan9":
370+
// Some versions of Windows and all versions of plan9 do not support
371+
// symlinks by default.
372+
return false
373+
374+
default:
375+
// All other platforms should support symlinks by default, and our builders
376+
// should not do anything unusual that would violate that.
377+
return true
378+
}
379+
}
380+
381+
// openAndStat attempts to open source for reading.
382+
func openAndStat(source string) (os.FileInfo, error) {
383+
src, err := os.Open(source)
384+
if err != nil {
385+
return nil, err
386+
}
387+
stat, err := src.Stat()
388+
src.Close()
389+
if err != nil {
390+
return nil, err
283391
}
392+
return stat, nil
393+
}
394+
395+
// createEmpty creates an empty file or directory (depending on mode)
396+
// at dst, with the same permissions as mode.
397+
func createEmpty(dst string, mode os.FileMode) error {
398+
if mode.IsDir() {
399+
return os.Mkdir(dst, mode.Perm())
400+
}
401+
402+
f, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode.Perm())
403+
if err != nil {
404+
return err
405+
}
406+
if err := f.Close(); err != nil {
407+
os.Remove(dst) // best-effort
408+
return err
409+
}
410+
411+
return nil
284412
}
285413

286414
// Copy returns a Writer that copies a file from the specified source to the
@@ -297,12 +425,27 @@ func Copy(source string) Writer {
297425
// symlinks, devices, etc.)
298426
return fmt.Errorf("cannot copy non regular file %s", source)
299427
}
300-
contents, err := ioutil.ReadFile(source)
301-
if err != nil {
302-
return err
303-
}
304-
return ioutil.WriteFile(filename, contents, stat.Mode())
428+
return copyFile(filename, source, stat.Mode().Perm())
429+
}
430+
}
431+
432+
func copyFile(dest, source string, perm os.FileMode) error {
433+
src, err := os.Open(source)
434+
if err != nil {
435+
return err
436+
}
437+
defer src.Close()
438+
439+
dst, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
440+
if err != nil {
441+
return err
442+
}
443+
444+
_, err = io.Copy(dst, src)
445+
if closeErr := dst.Close(); err == nil {
446+
err = closeErr
305447
}
448+
return err
306449
}
307450

308451
// GroupFilesByModules attempts to map directories to the modules within each directory.

go/packages/packagestest/export_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
var testdata = []packagestest.Module{{
1616
Name: "golang.org/fake1",
1717
Files: map[string]interface{}{
18-
"a.go": packagestest.Symlink("testdata/a.go"),
18+
"a.go": packagestest.Symlink("testdata/a.go"), // broken symlink
1919
"b.go": "invalid file contents",
2020
},
2121
Overlay: map[string][]byte{

internal/imports/fix_test.go

-11
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"log"
1414
"path/filepath"
1515
"reflect"
16-
"runtime"
1716
"sort"
1817
"strings"
1918
"sync"
@@ -1307,11 +1306,6 @@ func bar() {
13071306
// Test support for packages in GOPATH that are actually symlinks.
13081307
// Also test that a symlink loop does not block the process.
13091308
func TestImportSymlinks(t *testing.T) {
1310-
switch runtime.GOOS {
1311-
case "windows", "plan9":
1312-
t.Skipf("skipping test on %q as there are no symlinks", runtime.GOOS)
1313-
}
1314-
13151309
const input = `package p
13161310
13171311
var (
@@ -1347,11 +1341,6 @@ var (
13471341
}
13481342

13491343
func TestImportSymlinksWithIgnore(t *testing.T) {
1350-
switch runtime.GOOS {
1351-
case "windows", "plan9":
1352-
t.Skipf("skipping test on %q as there are no symlinks", runtime.GOOS)
1353-
}
1354-
13551344
const input = `package p
13561345
13571346
var (

0 commit comments

Comments
 (0)