diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ef764f4ae..a4160161d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ IMPROVEMENTS: * Don't exclude `Godeps` folder ([#1822](https://github.com/golang/dep/issues/1822)). * Add project-package relationship graph support in graphviz ([#1588](https://github.com/golang/dep/pull/1588)). * Limit concurrency of `dep status` to avoid hitting open file limits ([#1923](https://github.com/golang/dep/issue/1923)). +* Export the content of git submodules to the vendor directory ([#1909](https://github.com/golang/dep/pull/1909)). WIP: * Enable importing external configuration from dependencies during init (#1277). This is feature flagged and disabled by default. diff --git a/appveyor.yml b/appveyor.yml index 26d1728eec..008f80352e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -31,4 +31,5 @@ deploy: false test_script: - go build github.com/golang/dep/cmd/dep + - go test ./cmd/dep -args -logs - for /f "" %%G in ('go list github.com/golang/dep/...') do ( go test %%G & IF ERRORLEVEL == 1 EXIT 1) diff --git a/cmd/dep/integration_test.go b/cmd/dep/integration_test.go index 0a4667cff2..3c449aaac4 100644 --- a/cmd/dep/integration_test.go +++ b/cmd/dep/integration_test.go @@ -16,6 +16,7 @@ import ( "testing" "github.com/golang/dep" + "github.com/golang/dep/internal/fs" "github.com/golang/dep/internal/test" "github.com/golang/dep/internal/test/integration" ) @@ -44,7 +45,12 @@ func TestIntegration(t *testing.T) { parse := strings.Split(path, string(filepath.Separator)) testName := strings.Join(parse[2:len(parse)-1], "/") t.Run(testName, func(t *testing.T) { - t.Parallel() + // When updating test case files we can't run in parallel, because then + // we would have a race condition (each test case is used twice, once + // for "internal" and again for "external"). + if !*test.UpdateGolden { + t.Parallel() + } t.Run("external", testIntegration(testName, relPath, wd, execCmd)) t.Run("internal", testIntegration(testName, relPath, wd, runMain)) @@ -210,18 +216,66 @@ func testIntegration(name, relPath, wd string, run integration.RunFunc) func(t * testCase.CompareOutput(testProj.GetStdout()) } + // Determine how the test case specifies the expected + // content: either it lists just some projects, or it + // provides a complete reference directory. + reference := len(testCase.VendorFinal) == 1 && testCase.VendorFinal[0] == "compare" + // Check vendor paths testProj.CompareImportPaths() - testCase.CompareVendorPaths(testProj.GetVendorPaths()) + if !reference { + testCase.CompareVendorPaths(testProj.GetVendorPaths()) + } - if *test.UpdateGolden { - // Update manifest and lock - testCase.UpdateFile(dep.ManifestName, testProj.ProjPath(dep.ManifestName)) - testCase.UpdateFile(dep.LockName, testProj.ProjPath(dep.LockName)) + if reference { + // Check all files. + if *test.UpdateGolden { + // Update all files in the 'final' directory, removing those which + // no longer should exist. + if err := os.RemoveAll(testCase.FinalPath()); err != nil { + t.Fatalf("error removing 'final' directory: %s", err) + } + if err := fs.CopyDir(testProj.ProjPath(), testCase.FinalPath()); err != nil { + t.Fatalf("error copying into 'final' directory: %s", err) + } + } else { + // Compare all files from either of the two trees. + files := make(map[string]bool) + findFiles := func(dir string) error { + return filepath.Walk(dir, + func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if dir == path { + return nil + } + if !info.IsDir() { + localpath := path[len(dir)+1:] + files[localpath] = true + } + return nil + }) + } + findFiles(testCase.FinalPath()) + findFiles(testProj.ProjPath()) + + t.Logf("checking against %s", testCase.FinalPath()) + for localpath := range files { + t.Logf("comparing %s", localpath) + testCase.CompareFile(localpath, testProj.ProjPath(localpath)) + } + } } else { - // Check final manifest and lock - testCase.CompareFile(dep.ManifestName, testProj.ProjPath(dep.ManifestName)) - testCase.CompareFile(dep.LockName, testProj.ProjPath(dep.LockName)) + if *test.UpdateGolden { + // Update manifest and lock + testCase.UpdateFile(dep.ManifestName, testProj.ProjPath(dep.ManifestName)) + testCase.UpdateFile(dep.LockName, testProj.ProjPath(dep.LockName)) + } else { + // Check final manifest and lock + testCase.CompareFile(dep.ManifestName, testProj.ProjPath(dep.ManifestName)) + testCase.CompareFile(dep.LockName, testProj.ProjPath(dep.LockName)) + } } } } diff --git a/cmd/dep/testdata/harness_tests/README.md b/cmd/dep/testdata/harness_tests/README.md index 379caaa2db..60029d7cfc 100644 --- a/cmd/dep/testdata/harness_tests/README.md +++ b/cmd/dep/testdata/harness_tests/README.md @@ -30,6 +30,8 @@ the test case directory itself. In the above example, the test name would be `category1/subcategory1/case1`, and could be singled out with the `-run` option of `go test` (i.e. `go test github.com/golang/dep/cmd/dep -run Integration/category1/subcategory1/case1`). +`dlv` can also be used when explicitly setting the right directory +(`dlv test --wd cmd/dep ./cmd/dep -test.run Integration/category1/subcategory1/case1`). New tests can be added simply by adding a new directory with the json file to the `testdata` tree. There is no need for code modification - the new test will be included automatically. diff --git a/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/Gopkg.lock new file mode 100644 index 0000000000..e134cde91e --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/Gopkg.lock @@ -0,0 +1,17 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + branch = "master" + digest = "1:62b5039d111b756e045e4d0f74545bfa9db8b49137f00bfe87e7f403023f500e" + name = "github.com/pohly/deptestmodules" + packages = ["pkg/foo"] + pruneopts = "UT" + revision = "797a8d2c23e060c7d84f3e3afcda9128d4853648" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + input-imports = ["github.com/pohly/deptestmodules/pkg/foo"] + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/Gopkg.toml new file mode 100644 index 0000000000..a550140ec6 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/Gopkg.toml @@ -0,0 +1,34 @@ +# Gopkg.toml example +# +# Refer to https://golang.github.io/dep/docs/Gopkg.toml.html +# for detailed Gopkg.toml documentation. +# +# required = ["github.com/user/thing/cmd/thing"] +# ignored = ["github.com/user/project/pkgX", "bitbucket.org/user/project/pkgA/pkgY"] +# +# [[constraint]] +# name = "github.com/user/project" +# version = "1.0.0" +# +# [[constraint]] +# name = "github.com/user/project2" +# branch = "dev" +# source = "github.com/myfork/project2" +# +# [[override]] +# name = "github.com/x/y" +# version = "2.4.0" +# +# [prune] +# non-go = false +# go-tests = true +# unused-packages = true + + +[[constraint]] + branch = "master" + name = "github.com/pohly/deptestmodules" + +[prune] + go-tests = true + unused-packages = true diff --git a/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/main.go b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/main.go new file mode 100644 index 0000000000..0f125e8351 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/main.go @@ -0,0 +1,8 @@ +package main + +import ( + _ "github.com/pohly/deptestmodules/pkg/foo" +) + +func main() { +} diff --git a/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/vendor/github.com/pohly/deptestmodules/pkg/foo/foo.go b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/vendor/github.com/pohly/deptestmodules/pkg/foo/foo.go new file mode 100644 index 0000000000..b549aef878 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/final/vendor/github.com/pohly/deptestmodules/pkg/foo/foo.go @@ -0,0 +1,7 @@ +/* +Copyright 2018 Intel Corporation. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package foo diff --git a/cmd/dep/testdata/harness_tests/ensure/submodules/case1/initial/main.go b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/initial/main.go new file mode 100644 index 0000000000..0f125e8351 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/initial/main.go @@ -0,0 +1,8 @@ +package main + +import ( + _ "github.com/pohly/deptestmodules/pkg/foo" +) + +func main() { +} diff --git a/cmd/dep/testdata/harness_tests/ensure/submodules/case1/testcase.json b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/testcase.json new file mode 100644 index 0000000000..0789716aef --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/submodules/case1/testcase.json @@ -0,0 +1,9 @@ +{ + "commands": [ + ["init"], + ["ensure"] + ], + "vendor-final": [ + "compare" + ] +} diff --git a/gps/vcs_source.go b/gps/vcs_source.go index 1e04b7b8c4..1e286840fd 100644 --- a/gps/vcs_source.go +++ b/gps/vcs_source.go @@ -159,28 +159,94 @@ type gitSource struct { baseVCSSource } +// gitSubmoduleStatusRE matches the output of "git submodule status" which has lines like this: +// f273d8faee1af72e63c0c949287a9265af6635f9 pkg (heads/master) +var gitSubmoduleStatusRE = regexp.MustCompile(`^.([a-fA-F0-9]+) (.*) \(.*?\)$`) + func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to string) error { r := s.repo + if err := s.exportTreeTo(ctx, filepath.Join(r.LocalPath(), ".git"), r.LocalPath(), rev.String(), to); err != nil { + return errors.Wrap(err, "exporting main repo content") + } + + // Now do the same for any submodule, using a shell command that "git submodule" + // iterates over. + { + cmd := commandContext(ctx, "git", "submodule", "status", "--recursive") + cmd.SetDir(r.LocalPath()) + out, err := cmd.CombinedOutput() + if err != nil { + return errors.Wrapf(err, "git submodule status in %s: %s", r.LocalPath(), string(out)) + } + + // The output is something like this: + // f273d8faee1af72e63c0c949287a9265af6635f9 pkg (heads/new-master2) + // 4782e9497d6177ebd53bf080198a7fb8320847f8 pkg/foo (heads/new-master2) + // + // We only need the paths. There's no good way to reliably distinguish + // paths (which may contain brackets) from the "git describe" output at the end + // (which might contain tags with spaces), but those pathological cases + // hopefully get avoided by developers. + for _, line := range strings.Split(string(out), "\n") { + if line == "" { + continue + } + match := gitSubmoduleStatusRE.FindStringSubmatch(line) + if match == nil { + return errors.Errorf("unexpected 'git submodule status' output line: %q", line) + } + hash := match[1] + relPath := match[2] + absPath := filepath.Join(r.LocalPath(), relPath) + // Each submodule path has .git text + // file which contains a relative link to the + // git directory. We need to follow that and + // there do the same "git read-tree + git + // checkout-index" with a temporary index as + // for the main repo. + // + // Instead of parsing that .git text file + // ourselves, we use "git rev-parse --git-dir". + var gitPath string + { + cmd := commandContext(ctx, "git", "rev-parse", "--git-dir") + cmd.SetDir(absPath) + out, err := cmd.CombinedOutput() + if err != nil { + return errors.Wrapf(err, "git rev-parse --git-dir in %s: %s", absPath, string(out)) + } + gitPath = strings.TrimRight(string(out), "\n") + } + if err := s.exportTreeTo(ctx, gitPath, absPath, hash, filepath.Join(to, relPath)); err != nil { + return errors.Wrapf(err, "exporting submodule %s repo content", relPath) + } + } + } + + return nil +} + +func (s *gitSource) exportTreeTo(ctx context.Context, gitDir, workDir, hash, to string) error { if err := os.MkdirAll(to, 0777); err != nil { return err } // Back up original index - idx, bak := filepath.Join(r.LocalPath(), ".git", "index"), filepath.Join(r.LocalPath(), ".git", "origindex") + idx, bak := filepath.Join(gitDir, "index"), filepath.Join(gitDir, "origindex") err := fs.RenameWithFallback(idx, bak) if err != nil { - return err + return errors.Wrapf(err, "rename index in %s", gitDir) } // could have an err here...but it's hard to imagine how? defer fs.RenameWithFallback(bak, idx) { - cmd := commandContext(ctx, "git", "read-tree", rev.String()) - cmd.SetDir(r.LocalPath()) + cmd := commandContext(ctx, "git", "read-tree", hash) + cmd.SetDir(workDir) if out, err := cmd.CombinedOutput(); err != nil { - return errors.Wrap(err, string(out)) + return errors.Wrapf(err, "git read-tree %s in %s: %s", workDir, hash, string(out)) } } @@ -197,9 +263,9 @@ func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to strin // index and HEAD. { cmd := commandContext(ctx, "git", "checkout-index", "-a", "--prefix="+to) - cmd.SetDir(r.LocalPath()) + cmd.SetDir(workDir) if out, err := cmd.CombinedOutput(); err != nil { - return errors.Wrap(err, string(out)) + return errors.Wrapf(err, "git checkout-index in %s to %q: %s", workDir, to, string(out)) } } diff --git a/internal/test/integration/testcase.go b/internal/test/integration/testcase.go index cb7f60fed0..29409ea98a 100644 --- a/internal/test/integration/testcase.go +++ b/internal/test/integration/testcase.go @@ -65,6 +65,11 @@ func (tc *TestCase) InitialPath() string { return tc.initialPath } +// FinalPath represents the final set of files in a project. +func (tc *TestCase) FinalPath() string { + return tc.finalPath +} + // UpdateFile updates the golden file with the working result. func (tc *TestCase) UpdateFile(goldenPath, workingPath string) { exists, working, err := getFile(workingPath) diff --git a/internal/test/integration/testproj.go b/internal/test/integration/testproj.go index 1d8cc729d7..c47d26a1d6 100644 --- a/internal/test/integration/testproj.go +++ b/internal/test/integration/testproj.go @@ -16,6 +16,7 @@ import ( "strings" "testing" + "github.com/golang/dep/internal/fs" "github.com/golang/dep/internal/test" "github.com/pkg/errors" ) @@ -56,8 +57,22 @@ func NewTestProject(t *testing.T, initPath, wd string, run RunFunc) *TestProject run: run, } new.makeRootTempDir() + new.TempDir(projectRoot) + dst := filepath.Join(new.tempdir, projectRoot) + // Not all test projects have an "initial" directory. + if _, err := os.Lstat(initPath); !os.IsNotExist(err) { + // Everything except for the inner destination + // directory must exist when calling CopyDir. It was + // created earlier by new.TempDir, so we have to + // remove it again here. + if err := os.Remove(dst); err != nil { + t.Fatalf("Remove(%s): %s", dst, err) + } + if err := fs.CopyDir(initPath, dst); err != nil { + t.Fatalf("CopyDir(%s, %s): %s", initPath, dst, err) + } + } new.TempDir(projectRoot, "vendor") - new.CopyTree(initPath) new.Setenv("GOPATH", new.tempdir) @@ -172,7 +187,7 @@ func (p *TestProject) GetVendorGit(ip string) { // DoRun executes the integration test command against the test project. func (p *TestProject) DoRun(args []string) error { if *test.PrintLogs { - p.t.Logf("running testdep %v", args) + p.t.Logf("running testdep %v in %s", args, p.ProjPath("")) } prog := filepath.Join(p.origWd, "testdep"+test.ExeSuffix) @@ -197,39 +212,6 @@ func (p *TestProject) DoRun(args []string) error { return status } -// CopyTree recursively copies a source directory into the test project's directory. -func (p *TestProject) CopyTree(src string) { - filepath.Walk(src, - func(path string, info os.FileInfo, err error) error { - if path != src { - localpath := path[len(src)+1:] - if info.IsDir() { - p.TempDir(projectRoot, localpath) - } else { - destpath := filepath.Join(p.ProjPath(), localpath) - copyFile(destpath, path) - } - } - return nil - }) -} - -func copyFile(dest, src string) { - in, err := os.Open(src) - if err != nil { - panic(err) - } - defer in.Close() - - out, err := os.Create(dest) - if err != nil { - panic(err) - } - defer out.Close() - - io.Copy(out, in) -} - // GetVendorPaths collects final vendor paths at a depth of three levels. func (p *TestProject) GetVendorPaths() []string { vendorPath := p.ProjPath("vendor")