From 34b8ee6ff36eaf935b301f3ee701ebc09295c01b Mon Sep 17 00:00:00 2001 From: Michael Gokhman Date: Thu, 14 Sep 2017 23:46:40 +0300 Subject: [PATCH 1/5] import gvt manifest on init --- internal/importers/gvt/importer.go | 124 ++++++++++++ internal/importers/gvt/importer_test.go | 211 +++++++++++++++++++++ internal/importers/gvt/testdata/golden.txt | 8 + internal/importers/gvt/testdata/manifest | 17 ++ internal/importers/importers.go | 2 + 5 files changed, 362 insertions(+) create mode 100644 internal/importers/gvt/importer.go create mode 100644 internal/importers/gvt/importer_test.go create mode 100644 internal/importers/gvt/testdata/golden.txt create mode 100644 internal/importers/gvt/testdata/manifest diff --git a/internal/importers/gvt/importer.go b/internal/importers/gvt/importer.go new file mode 100644 index 0000000000..a8c966875a --- /dev/null +++ b/internal/importers/gvt/importer.go @@ -0,0 +1,124 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package gvt + +import ( + "encoding/json" + "io/ioutil" + "log" + "os" + "path/filepath" + + "github.com/golang/dep" + "github.com/golang/dep/internal/gps" + "github.com/golang/dep/internal/importers/base" + "github.com/pkg/errors" +) + +const gvtPath = "vendor" + string(os.PathSeparator) + "manifest" + +// Importer imports gvt configuration into the dep configuration format. +type Importer struct { + *base.Importer + gvtConfig gvtManifest +} + +// NewImporter for gvt. +func NewImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *Importer { + return &Importer{Importer: base.NewImporter(logger, verbose, sm)} +} + +type gvtManifest struct { + Deps []gvtPkg `json:"dependencies"` +} + +type gvtPkg struct { + ImportPath string + Repository string + Revision string + Branch string +} + +// Name of the importer. +func (g *Importer) Name() string { + return "gvt" +} + +// HasDepMetadata checks if a directory contains config that the importer can handle. +func (g *Importer) HasDepMetadata(dir string) bool { + y := filepath.Join(dir, gvtPath) + if _, err := os.Stat(y); err != nil { + return false + } + + return true +} + +// Import the config found in the directory. +func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { + err := g.load(dir) + if err != nil { + return nil, nil, err + } + + return g.convert(pr) +} + +func (g *Importer) load(projectDir string) error { + g.Logger.Println("Detected gvt configuration files...") + j := filepath.Join(projectDir, gvtPath) + if g.Verbose { + g.Logger.Printf(" Loading %s", j) + } + jb, err := ioutil.ReadFile(j) + if err != nil { + return errors.Wrapf(err, "unable to read %s", j) + } + err = json.Unmarshal(jb, &g.gvtConfig) + if err != nil { + return errors.Wrapf(err, "unable to parse %s", j) + } + + return nil +} + +func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { + g.Logger.Println("Converting from vendor/manifest ...") + + packages := make([]base.ImportedPackage, 0, len(g.gvtConfig.Deps)) + for _, pkg := range g.gvtConfig.Deps { + // Validate + if pkg.ImportPath == "" { + err := errors.New("invalid gvt configuration, ImportPath is required") + return nil, nil, err + } + + if pkg.Revision == "" { + err := errors.New("invalid gvt configuration, Revision is required") + return nil, nil, err + } + + var contstraintHint = "" + if pkg.Branch != "master" { + contstraintHint = pkg.Branch + } + + ip := base.ImportedPackage{ + Name: pkg.ImportPath, + //TODO: temporarly ignore .Repository. see https://github.com/golang/dep/pull/1166 + // Source: pkg.Repository, + LockHint: pkg.Revision, + ConstraintHint: contstraintHint, + } + packages = append(packages, ip) + } + + err := g.ImportPackages(packages, true) + if err != nil { + return nil, nil, err + } + + return g.Manifest, g.Lock, nil +} diff --git a/internal/importers/gvt/importer_test.go b/internal/importers/gvt/importer_test.go new file mode 100644 index 0000000000..d672193b77 --- /dev/null +++ b/internal/importers/gvt/importer_test.go @@ -0,0 +1,211 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package gvt + +import ( + "bytes" + "log" + "path/filepath" + "testing" + + "github.com/golang/dep" + "github.com/golang/dep/internal/gps" + "github.com/golang/dep/internal/importers/importertest" + "github.com/golang/dep/internal/test" + "github.com/pkg/errors" +) + +func TestGvtConfig_Convert(t *testing.T) { + testCases := map[string]struct { + importertest.TestCase + gvtConfig gvtManifest + }{ + "package with master branch": { + importertest.TestCase{ + WantConstraint: importertest.V1Constraint, + WantRevision: importertest.V1Rev, + WantVersion: importertest.V1Tag, + }, + gvtManifest{ + Deps: []gvtPkg{ + { + ImportPath: importertest.Project, + Revision: importertest.V1Rev, + Branch: "master", + }, + }, + }, + }, + "package with non-master branch": { + importertest.TestCase{ + WantConstraint: importertest.V2Branch, + WantRevision: importertest.V2PatchRev, + WantVersion: importertest.V2PatchTag, + }, + gvtManifest{ + Deps: []gvtPkg{ + { + ImportPath: importertest.Project, + Revision: importertest.V2PatchRev, + Branch: importertest.V2Branch, + }, + }, + }, + }, + "missing package name": { + importertest.TestCase{ + WantConvertErr: true, + }, + gvtManifest{ + Deps: []gvtPkg{{ImportPath: ""}}, + }, + }, + "missing revision": { + importertest.TestCase{ + WantConvertErr: true, + }, + gvtManifest{ + Deps: []gvtPkg{ + { + ImportPath: importertest.Project, + }, + }, + }, + }, + } + + for name, testCase := range testCases { + name := name + testCase := testCase + t.Run(name, func(t *testing.T) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) { + g := NewImporter(logger, true, sm) + g.gvtConfig = testCase.gvtConfig + return g.convert(importertest.RootProject) + }) + if err != nil { + t.Fatalf("%#v", err) + } + }) + } +} + +func TestGvtConfig_Import(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + cacheDir := "gps-repocache" + h.TempDir(cacheDir) + h.TempDir("src") + h.TempDir(filepath.Join("src", importertest.RootProject)) + h.TempCopy(filepath.Join(importertest.RootProject, gvtPath), "manifest") + + projectRoot := h.Path(importertest.RootProject) + sm, err := gps.NewSourceManager(gps.SourceManagerConfig{ + Cachedir: h.Path(cacheDir), + Logger: log.New(test.Writer{TB: t}, "", 0), + }) + h.Must(err) + defer sm.Release() + + // Capture stderr so we can verify output + verboseOutput := &bytes.Buffer{} + logger := log.New(verboseOutput, "", 0) + + g := NewImporter(logger, false, sm) // Disable verbose so that we don't print values that change each test run + if !g.HasDepMetadata(projectRoot) { + t.Fatal("Expected the importer to detect gvt configuration file") + } + + m, l, err := g.Import(projectRoot, importertest.RootProject) + h.Must(err) + + if m == nil { + t.Fatal("Expected the manifest to be generated") + } + + if l == nil { + t.Fatal("Expected the lock to be generated") + } + + goldenFile := "golden.txt" + got := verboseOutput.String() + want := h.GetTestFileString(goldenFile) + if want != got { + if *test.UpdateGolden { + if err := h.WriteTestFile(goldenFile, got); err != nil { + t.Fatalf("%+v", errors.Wrapf(err, "Unable to write updated golden file %s", goldenFile)) + } + } else { + t.Fatalf("want %s, got %s", want, got) + } + } +} + +func TestGvtConfig_JsonLoad(t *testing.T) { + // This is same as testdata/manifest + wantConfig := gvtManifest{ + Deps: []gvtPkg{ + { + ImportPath: "github.com/sdboyer/deptest", + Revision: "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", + Branch: "master", + }, + { + ImportPath: "github.com/sdboyer/deptestdos", + Revision: "5c607206be5decd28e6263ffffdcee067266015e", + Branch: "master", + }, + { + ImportPath: "github.com/carolynvs/deptest-importers", + Revision: "b79bc9482da8bb7402cdc3e3fd984db250718dd7", + Branch: "v2", + }, + }, + } + + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := importertest.NewTestContext(h) + + h.TempCopy(filepath.Join(importertest.RootProject, gvtPath), "manifest") + + projectRoot := h.Path(importertest.RootProject) + + g := NewImporter(ctx.Err, true, nil) + err := g.load(projectRoot) + if err != nil { + t.Fatalf("Error while loading... %v", err) + } + + if !equalImports(g.gvtConfig.Deps, wantConfig.Deps) { + t.Fatalf("Expected imports to be equal. \n\t(GOT): %v\n\t(WNT): %v", g.gvtConfig.Deps, wantConfig.Deps) + } +} + +// equalImports compares two slices of gvtPkg and checks if they are +// equal. +func equalImports(a, b []gvtPkg) bool { + if a == nil && b == nil { + return true + } + + if a == nil || b == nil { + return false + } + + if len(a) != len(b) { + return false + } + + for i := range a { + if a[i] != b[i] { + return false + } + } + + return true +} diff --git a/internal/importers/gvt/testdata/golden.txt b/internal/importers/gvt/testdata/golden.txt new file mode 100644 index 0000000000..b7541aa9b7 --- /dev/null +++ b/internal/importers/gvt/testdata/golden.txt @@ -0,0 +1,8 @@ +Detected gvt configuration files... +Converting from vendor/manifest ... + Using ^0.8.1 as initial constraint for imported dep github.com/sdboyer/deptest + Trying v0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest + Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos + Trying v2.0.0 (5c60720) as initial lock for imported dep github.com/sdboyer/deptestdos + Using v2 as initial constraint for imported dep github.com/carolynvs/deptest-importers + Trying v2 (b79bc94) as initial lock for imported dep github.com/carolynvs/deptest-importers diff --git a/internal/importers/gvt/testdata/manifest b/internal/importers/gvt/testdata/manifest new file mode 100644 index 0000000000..61ff49a8f1 --- /dev/null +++ b/internal/importers/gvt/testdata/manifest @@ -0,0 +1,17 @@ +{ + "dependencies": [ + { + "importpath": "github.com/sdboyer/deptest", + "revision": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + { + "importpath": "github.com/sdboyer/deptestdos", + "revision": "5c607206be5decd28e6263ffffdcee067266015e" + }, + { + "importpath": "github.com/carolynvs/deptest-importers", + "revision": "b79bc9482da8bb7402cdc3e3fd984db250718dd7", + "branch": "v2" + } + ] +} diff --git a/internal/importers/importers.go b/internal/importers/importers.go index 7cc7cfac78..ad669b55bd 100644 --- a/internal/importers/importers.go +++ b/internal/importers/importers.go @@ -12,6 +12,7 @@ import ( "github.com/golang/dep/internal/importers/glide" "github.com/golang/dep/internal/importers/godep" "github.com/golang/dep/internal/importers/govend" + "github.com/golang/dep/internal/importers/gvt" "github.com/golang/dep/internal/importers/vndr" ) @@ -35,5 +36,6 @@ func BuildAll(logger *log.Logger, verbose bool, sm gps.SourceManager) []Importer godep.NewImporter(logger, verbose, sm), vndr.NewImporter(logger, verbose, sm), govend.NewImporter(logger, verbose, sm), + gvt.NewImporter(logger, verbose, sm), } } From 28f55f7c49bec14aa6b6682bbb9e71e8cad84f84 Mon Sep 17 00:00:00 2001 From: Michael Gokhman Date: Fri, 15 Sep 2017 13:29:45 +0300 Subject: [PATCH 2/5] add gvt importer integration test and update docs --- cmd/dep/init.go | 2 +- .../init/gvt/case1/final/Gopkg.lock | 21 +++++++++++++++++++ .../init/gvt/case1/final/Gopkg.toml | 3 +++ .../init/gvt/case1/initial/main.go | 16 ++++++++++++++ .../init/gvt/case1/initial/vendor/manifest | 17 +++++++++++++++ .../init/gvt/case1/testcase.json | 13 ++++++++++++ docs/FAQ.md | 2 +- 7 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock create mode 100644 cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml create mode 100644 cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go create mode 100644 cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest create mode 100644 cmd/dep/testdata/harness_tests/init/gvt/case1/testcase.json diff --git a/cmd/dep/init.go b/cmd/dep/init.go index 7fef07f4ec..cb4f21b10f 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -29,7 +29,7 @@ specified, use the current directory. When configuration for another dependency management tool is detected, it is imported into the initial manifest and lock. Use the -skip-tools flag to disable this behavior. The following external tools are supported: -glide, godep, vndr, govend. +glide, godep, vndr, govend, gvt. Any dependencies that are not constrained by external configuration use the GOPATH analysis below. diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock new file mode 100644 index 0000000000..74c44abc9d --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock @@ -0,0 +1,21 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + name = "github.com/sdboyer/deptest" + packages = ["."] + revision = "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + version = "v0.8.1" + +[[projects]] + name = "github.com/sdboyer/deptestdos" + packages = ["."] + revision = "5c607206be5decd28e6263ffffdcee067266015e" + version = "v2.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "1caabc68a242bf012bf5e10e30d21e342beb7e338707a6fd08f0398c18548e32" + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml new file mode 100644 index 0000000000..898e622122 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml @@ -0,0 +1,3 @@ + +[[constraint]] + name = "github.com/sdboyer/deptestdos" diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go new file mode 100644 index 0000000000..2b2c7c396e --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go @@ -0,0 +1,16 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "fmt" + + "github.com/sdboyer/deptestdos" +) + +func main() { + var x deptestdos.Bar + fmt.Println(x) +} diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest new file mode 100644 index 0000000000..2d30d3732d --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest @@ -0,0 +1,17 @@ +{ + "version": "0", + "dependencies": [ + { + "importpath": "github.com/sdboyer/deptest", + "repository": "https://github.com/sdboyer/deptest", + "revision": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", + "branch": "master" + }, + { + "importpath": "github.com/sdboyer/deptestdos", + "repository": "https://github.com/sdboyer/deptestdos", + "revision": "5c607206be5decd28e6263ffffdcee067266015eXXX", + "branch": "master" + } + ] +} diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/testcase.json b/cmd/dep/testdata/harness_tests/init/gvt/case1/testcase.json new file mode 100644 index 0000000000..017dc4cd55 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/testcase.json @@ -0,0 +1,13 @@ +{ + "commands": [ + ["init", "-no-examples"] + ], + "error-expected": "", + "gopath-initial": { + "github.com/sdboyer/deptest": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + "vendor-final": [ + "github.com/sdboyer/deptest", + "github.com/sdboyer/deptestdos" + ] +} diff --git a/docs/FAQ.md b/docs/FAQ.md index 02d426e1b6..ab272c85a0 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -210,7 +210,7 @@ about what's going on. During `dep init` configuration from other dependency managers is detected and imported, unless `-skip-tools` is specified. -The following tools are supported: `glide`, `godep`, `vndr` and `govend`. +The following tools are supported: `glide`, `godep`, `vndr`, `govend` and `gvt`. See [#186](https://github.com/golang/dep/issues/186#issuecomment-306363441) for how to add support for another tool. From 956f34d9d80c0ccd3bfe3586d4ef348101dc1297 Mon Sep 17 00:00:00 2001 From: Michael Gokhman Date: Sat, 16 Sep 2017 22:46:05 +0300 Subject: [PATCH 3/5] gvt importer imports gb-vendor too --- cmd/dep/init.go | 2 +- .../init/gvt/case1/initial/vendor/manifest | 2 +- docs/FAQ.md | 2 +- internal/importers/gvt/importer.go | 10 ++++++++-- internal/importers/gvt/importer_test.go | 18 +++++++++++++++++- internal/importers/gvt/testdata/golden.txt | 4 ++-- internal/importers/gvt/testdata/manifest | 6 ++++-- 7 files changed, 34 insertions(+), 10 deletions(-) diff --git a/cmd/dep/init.go b/cmd/dep/init.go index cb4f21b10f..4cebf4ae34 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -29,7 +29,7 @@ specified, use the current directory. When configuration for another dependency management tool is detected, it is imported into the initial manifest and lock. Use the -skip-tools flag to disable this behavior. The following external tools are supported: -glide, godep, vndr, govend, gvt. +glide, godep, vndr, govend, gb, gvt. Any dependencies that are not constrained by external configuration use the GOPATH analysis below. diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest index 2d30d3732d..2ab367604c 100644 --- a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest @@ -5,7 +5,7 @@ "importpath": "github.com/sdboyer/deptest", "repository": "https://github.com/sdboyer/deptest", "revision": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", - "branch": "master" + "branch": "HEAD" }, { "importpath": "github.com/sdboyer/deptestdos", diff --git a/docs/FAQ.md b/docs/FAQ.md index ab272c85a0..671effe06d 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -210,7 +210,7 @@ about what's going on. During `dep init` configuration from other dependency managers is detected and imported, unless `-skip-tools` is specified. -The following tools are supported: `glide`, `godep`, `vndr`, `govend` and `gvt`. +The following tools are supported: `glide`, `godep`, `vndr`, `govend`, `gb` and `gvt`. See [#186](https://github.com/golang/dep/issues/186#issuecomment-306363441) for how to add support for another tool. diff --git a/internal/importers/gvt/importer.go b/internal/importers/gvt/importer.go index a8c966875a..92cd61b6c4 100644 --- a/internal/importers/gvt/importer.go +++ b/internal/importers/gvt/importer.go @@ -67,7 +67,7 @@ func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L } func (g *Importer) load(projectDir string) error { - g.Logger.Println("Detected gvt configuration files...") + g.Logger.Println("Detected gb/gvt configuration files...") j := filepath.Join(projectDir, gvtPath) if g.Verbose { g.Logger.Printf(" Loading %s", j) @@ -101,7 +101,13 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) } var contstraintHint = "" - if pkg.Branch != "master" { + if pkg.Branch == "HEAD" { + // gb-vendor sets "branch" to "HEAD", if the package was feteched via -tag or -revision, + // we we pass the revision as the constraint hint + contstraintHint = pkg.Revision + } else if pkg.Branch != "master" { + // both gvt & gb-vendor set "branch" to "master" unless a different branch was requested. + // so it's not realy a constraint unless it's a different branch contstraintHint = pkg.Branch } diff --git a/internal/importers/gvt/importer_test.go b/internal/importers/gvt/importer_test.go index d672193b77..443e1eb723 100644 --- a/internal/importers/gvt/importer_test.go +++ b/internal/importers/gvt/importer_test.go @@ -54,6 +54,22 @@ func TestGvtConfig_Convert(t *testing.T) { }, }, }, + "package with HEAD branch": { + importertest.TestCase{ + WantConstraint: "*", + WantRevision: importertest.V1Rev, + WantVersion: importertest.V1Tag, + }, + gvtManifest{ + Deps: []gvtPkg{ + { + ImportPath: importertest.Project, + Revision: importertest.V1Rev, + Branch: "HEAD", + }, + }, + }, + }, "missing package name": { importertest.TestCase{ WantConvertErr: true, @@ -151,7 +167,7 @@ func TestGvtConfig_JsonLoad(t *testing.T) { { ImportPath: "github.com/sdboyer/deptest", Revision: "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", - Branch: "master", + Branch: "HEAD", }, { ImportPath: "github.com/sdboyer/deptestdos", diff --git a/internal/importers/gvt/testdata/golden.txt b/internal/importers/gvt/testdata/golden.txt index b7541aa9b7..d65e86d60b 100644 --- a/internal/importers/gvt/testdata/golden.txt +++ b/internal/importers/gvt/testdata/golden.txt @@ -1,6 +1,6 @@ -Detected gvt configuration files... +Detected gb/gvt configuration files... Converting from vendor/manifest ... - Using ^0.8.1 as initial constraint for imported dep github.com/sdboyer/deptest + Using * as initial constraint for imported dep github.com/sdboyer/deptest Trying v0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos Trying v2.0.0 (5c60720) as initial lock for imported dep github.com/sdboyer/deptestdos diff --git a/internal/importers/gvt/testdata/manifest b/internal/importers/gvt/testdata/manifest index 61ff49a8f1..36e49d56b1 100644 --- a/internal/importers/gvt/testdata/manifest +++ b/internal/importers/gvt/testdata/manifest @@ -2,11 +2,13 @@ "dependencies": [ { "importpath": "github.com/sdboyer/deptest", - "revision": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + "revision": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", + "branch": "HEAD" }, { "importpath": "github.com/sdboyer/deptestdos", - "revision": "5c607206be5decd28e6263ffffdcee067266015e" + "revision": "5c607206be5decd28e6263ffffdcee067266015e", + "branch": "master" }, { "importpath": "github.com/carolynvs/deptest-importers", From 1cfc27fb29c6369b42d99a51e06e680f1539e819 Mon Sep 17 00:00:00 2001 From: Michael Gokhman Date: Wed, 20 Sep 2017 16:15:40 +0300 Subject: [PATCH 4/5] import alternate repo url from gb/gvt manifest --- .../init/gvt/case1/final/Gopkg.lock | 9 ++++++- .../init/gvt/case1/final/Gopkg.toml | 8 ++++++ .../init/gvt/case1/initial/main.go | 8 ++++-- .../init/gvt/case1/initial/vendor/manifest | 8 +++++- internal/importers/base/importer.go | 27 ++++++++++++++++++- internal/importers/gvt/importer.go | 7 +++-- internal/importers/gvt/importer_test.go | 18 +++++++++++++ 7 files changed, 76 insertions(+), 9 deletions(-) diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock index 74c44abc9d..6b38e621f9 100644 --- a/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.lock @@ -5,6 +5,7 @@ name = "github.com/sdboyer/deptest" packages = ["."] revision = "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + source = "https://github.com/carolynvs/deptest" version = "v0.8.1" [[projects]] @@ -13,9 +14,15 @@ revision = "5c607206be5decd28e6263ffffdcee067266015e" version = "v2.0.0" +[[projects]] + branch = "v2" + name = "gopkg.in/yaml.v2" + packages = ["."] + revision = "f7716cbe52baa25d2e9b0d0da546fcf909fc16b4" + [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "1caabc68a242bf012bf5e10e30d21e342beb7e338707a6fd08f0398c18548e32" + inputs-digest = "9ee70d58a0bc986cfb1d57a351786e9cf1bfc2ca763ce17f601a6529aebe65d1" solver-name = "gps-cdcl" solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml index 898e622122..4b67f82f25 100644 --- a/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/final/Gopkg.toml @@ -1,3 +1,11 @@ +[[constraint]] + name = "github.com/sdboyer/deptest" + source = "https://github.com/carolynvs/deptest" + [[constraint]] name = "github.com/sdboyer/deptestdos" + +[[constraint]] + branch = "v2" + name = "gopkg.in/yaml.v2" diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go index 2b2c7c396e..6d893af2ff 100644 --- a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/main.go @@ -7,10 +7,14 @@ package main import ( "fmt" + "github.com/sdboyer/deptest" "github.com/sdboyer/deptestdos" + "gopkg.in/yaml.v2" ) func main() { - var x deptestdos.Bar - fmt.Println(x) + var a deptestdos.Bar + var b yaml.MapItem + var c deptest.Foo + fmt.Println(a, b, c) } diff --git a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest index 2ab367604c..628df3cf33 100644 --- a/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest +++ b/cmd/dep/testdata/harness_tests/init/gvt/case1/initial/vendor/manifest @@ -3,7 +3,7 @@ "dependencies": [ { "importpath": "github.com/sdboyer/deptest", - "repository": "https://github.com/sdboyer/deptest", + "repository": "https://github.com/carolynvs/deptest", "revision": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", "branch": "HEAD" }, @@ -12,6 +12,12 @@ "repository": "https://github.com/sdboyer/deptestdos", "revision": "5c607206be5decd28e6263ffffdcee067266015eXXX", "branch": "master" + }, + { + "importpath": "gopkg.in/yaml.v2", + "repository": "https://gopkg.in/yaml.v2", + "revision": "f7716cbe52baa25d2e9b0d0da546fcf909fc16b4", + "branch": "v2" } ] } diff --git a/internal/importers/base/importer.go b/internal/importers/base/importer.go index 0a572fea6d..62883fc9b6 100644 --- a/internal/importers/base/importer.go +++ b/internal/importers/base/importer.go @@ -180,10 +180,18 @@ func (i *Importer) ImportPackages(packages []ImportedPackage, defaultConstraintF } for _, prj := range projects { + source := prj.Source + if len(source) > 0 { + isDefault, err := i.isDefaultSource(prj.Root, source) + if err == nil && isDefault { + source = "" + } + } + pc := gps.ProjectConstraint{ Ident: gps.ProjectIdentifier{ ProjectRoot: prj.Root, - Source: prj.Source, + Source: source, }, } @@ -291,3 +299,20 @@ func (i *Importer) convertToConstraint(v gps.Version) gps.Constraint { } return v } + +func (i *Importer) isDefaultSource(projectRoot gps.ProjectRoot, sourceURL string) (bool, error) { + if sourceURL == "https://"+string(projectRoot) { + return true, nil + } + + sourceURLs, err := i.sm.SourceURLsForPath(string(projectRoot)) + if err != nil { + return false, err + } + // The first url in the slice will be the default one (usually https://...) + if len(sourceURLs) > 0 && sourceURL == sourceURLs[0].String() { + return true, nil + } + + return false, nil +} diff --git a/internal/importers/gvt/importer.go b/internal/importers/gvt/importer.go index 92cd61b6c4..b0a968d9b8 100644 --- a/internal/importers/gvt/importer.go +++ b/internal/importers/gvt/importer.go @@ -103,7 +103,7 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) var contstraintHint = "" if pkg.Branch == "HEAD" { // gb-vendor sets "branch" to "HEAD", if the package was feteched via -tag or -revision, - // we we pass the revision as the constraint hint + // we pass the revision as the constraint hint contstraintHint = pkg.Revision } else if pkg.Branch != "master" { // both gvt & gb-vendor set "branch" to "master" unless a different branch was requested. @@ -112,9 +112,8 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) } ip := base.ImportedPackage{ - Name: pkg.ImportPath, - //TODO: temporarly ignore .Repository. see https://github.com/golang/dep/pull/1166 - // Source: pkg.Repository, + Name: pkg.ImportPath, + Source: pkg.Repository, LockHint: pkg.Revision, ConstraintHint: contstraintHint, } diff --git a/internal/importers/gvt/importer_test.go b/internal/importers/gvt/importer_test.go index 443e1eb723..e927af8dbf 100644 --- a/internal/importers/gvt/importer_test.go +++ b/internal/importers/gvt/importer_test.go @@ -70,6 +70,24 @@ func TestGvtConfig_Convert(t *testing.T) { }, }, }, + "package with alternate repository": { + importertest.TestCase{ + WantConstraint: importertest.V1Constraint, + WantRevision: importertest.V1Rev, + WantVersion: importertest.V1Tag, + WantSourceRepo: importertest.ProjectSrc, + }, + gvtManifest{ + Deps: []gvtPkg{ + { + ImportPath: importertest.Project, + Repository: importertest.ProjectSrc, + Revision: importertest.V1Rev, + Branch: "master", + }, + }, + }, + }, "missing package name": { importertest.TestCase{ WantConvertErr: true, From cbf282ba3c5af0180b8cb0bbe0b02260b4b5a262 Mon Sep 17 00:00:00 2001 From: Michael Gokhman Date: Wed, 27 Sep 2017 22:31:26 +0300 Subject: [PATCH 5/5] gvt/gb importer PR fixes --- internal/importers/base/importer.go | 8 +++++++- internal/importers/base/importer_test.go | 24 ++++++++++++++++++++++++ internal/importers/gvt/importer.go | 2 +- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/internal/importers/base/importer.go b/internal/importers/base/importer.go index 62883fc9b6..262411c744 100644 --- a/internal/importers/base/importer.go +++ b/internal/importers/base/importer.go @@ -183,7 +183,10 @@ func (i *Importer) ImportPackages(packages []ImportedPackage, defaultConstraintF source := prj.Source if len(source) > 0 { isDefault, err := i.isDefaultSource(prj.Root, source) - if err == nil && isDefault { + if err != nil { + i.Logger.Printf(" Ignoring imported source %s for %s: %s", source, prj.Root, err.Error()) + source = "" + } else if isDefault { source = "" } } @@ -301,6 +304,9 @@ func (i *Importer) convertToConstraint(v gps.Version) gps.Constraint { } func (i *Importer) isDefaultSource(projectRoot gps.ProjectRoot, sourceURL string) (bool, error) { + // this condition is mainly for gopkg.in imports, + // as some importers specify the repository url as https://gopkg.in/..., + // but sm.SourceURLsForPath() returns https://github.com/... urls for gopkg.in if sourceURL == "https://"+string(projectRoot) { return true, nil } diff --git a/internal/importers/base/importer_test.go b/internal/importers/base/importer_test.go index a70dd8fb03..f298efefd7 100644 --- a/internal/importers/base/importer_test.go +++ b/internal/importers/base/importer_test.go @@ -370,6 +370,30 @@ func TestBaseImporter_ImportProjects(t *testing.T) { }, }, }, + "alternate source": { + importertest.TestCase{ + WantConstraint: "*", + WantSourceRepo: importertest.ProjectSrc, + }, + []ImportedPackage{ + { + Name: importertest.Project, + Source: importertest.ProjectSrc, + }, + }, + }, + "ignoring default source": { + importertest.TestCase{ + WantConstraint: "*", + WantSourceRepo: "", + }, + []ImportedPackage{ + { + Name: importertest.Project, + Source: "https://" + importertest.Project, + }, + }, + }, } for name, tc := range testcases { diff --git a/internal/importers/gvt/importer.go b/internal/importers/gvt/importer.go index b0a968d9b8..8177f21d80 100644 --- a/internal/importers/gvt/importer.go +++ b/internal/importers/gvt/importer.go @@ -25,7 +25,7 @@ type Importer struct { gvtConfig gvtManifest } -// NewImporter for gvt. +// NewImporter for gvt. It handles gb (gb-vendor) too as they share a common manifest file & format func NewImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *Importer { return &Importer{Importer: base.NewImporter(logger, verbose, sm)} }