From 316ba0b74098495f0b460c7b8bb56b8487c1c9c3 Mon Sep 17 00:00:00 2001 From: Hana Date: Fri, 22 Oct 2021 12:52:37 -0400 Subject: [PATCH] go/packages/packagestest: skip nested module directories in copying MustCopyFileTree is used to construct a module representation based on a real directory tree and its output consumers (e.g. Export) assume the output contains only files that belong to the module. If the real directory contains nested module directories, they need to be trimmed to satisfy the assumption. This CL implements this trimming logic - a directory containing go.mod is excluded from copying if the directory is not the top-level directory. Change-Id: Ib09d4d74e120b502e338a021042d4d5e00719f23 Reviewed-on: https://go-review.googlesource.com/c/tools/+/358037 Trust: Hyang-Ah Hana Kim Run-TryBot: Hyang-Ah Hana Kim gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Robert Findley --- go/packages/packagestest/export.go | 8 +++- go/packages/packagestest/export_test.go | 53 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/go/packages/packagestest/export.go b/go/packages/packagestest/export.go index 5dea613f0b6..d792c3c3d1f 100644 --- a/go/packages/packagestest/export.go +++ b/go/packages/packagestest/export.go @@ -582,7 +582,7 @@ func GroupFilesByModules(root string) ([]Module, error) { // MustCopyFileTree returns a file set for a module based on a real directory tree. // It scans the directory tree anchored at root and adds a Copy writer to the -// map for every file found. +// map for every file found. It skips copying files in nested modules. // This is to enable the common case in tests where you have a full copy of the // package in your testdata. // This will panic if there is any kind of error trying to walk the file tree. @@ -593,6 +593,12 @@ func MustCopyFileTree(root string) map[string]interface{} { return err } if info.IsDir() { + // skip nested modules. + if path != root { + if fi, err := os.Stat(filepath.Join(path, "go.mod")); err == nil && !fi.IsDir() { + return filepath.SkipDir + } + } return nil } fragment, err := filepath.Rel(root, path) diff --git a/go/packages/packagestest/export_test.go b/go/packages/packagestest/export_test.go index 356dd4b524d..1172f7c150a 100644 --- a/go/packages/packagestest/export_test.go +++ b/go/packages/packagestest/export_test.go @@ -5,8 +5,11 @@ package packagestest_test import ( + "io/ioutil" "os" "path/filepath" + "reflect" + "sort" "testing" "golang.org/x/tools/go/packages/packagestest" @@ -180,3 +183,53 @@ func TestGroupFilesByModules(t *testing.T) { }) } } + +func TestMustCopyFiles(t *testing.T) { + // Create the following test directory structure in a temporary directory. + src := map[string]string{ + // copies all files under the specified directory. + "go.mod": "module example.com", + "m.go": "package m", + "a/a.go": "package a", + // contents from a nested module shouldn't be copied. + "nested/go.mod": "module example.com/nested", + "nested/m.go": "package nested", + "nested/b/b.go": "package b", + } + + tmpDir, err := ioutil.TempDir("", t.Name()) + if err != nil { + t.Fatalf("failed to create a temporary directory: %v", err) + } + defer os.RemoveAll(tmpDir) + + for fragment, contents := range src { + fullpath := filepath.Join(tmpDir, filepath.FromSlash(fragment)) + if err := os.MkdirAll(filepath.Dir(fullpath), 0755); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(fullpath, []byte(contents), 0644); err != nil { + t.Fatal(err) + } + } + + copied := packagestest.MustCopyFileTree(tmpDir) + var got []string + for fragment := range copied { + got = append(got, filepath.ToSlash(fragment)) + } + want := []string{"go.mod", "m.go", "a/a.go"} + + sort.Strings(got) + sort.Strings(want) + if !reflect.DeepEqual(got, want) { + t.Errorf("packagestest.MustCopyFileTree = %v, want %v", got, want) + } + + // packagestest.Export is happy. + exported := packagestest.Export(t, packagestest.Modules, []packagestest.Module{{ + Name: "example.com", + Files: packagestest.MustCopyFileTree(tmpDir), + }}) + defer exported.Cleanup() +}