diff --git a/CHANGELOG.md b/CHANGELOG.md index fe3efaf7fe..cdca97ffd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ IMPROVEMENTS: * Skip empty constraints during import ([#1414](https://github.com/golang/dep/pull/1349)) * Handle errors when writing status output ([#1420](https://github.com/golang/dep/pull/1420)) * Add constraint for locked projects in `status`. (#962) +* Make external config importers error tolerant. ([#1315](https://github.com/golang/dep/pull/1315)) # v0.3.2 diff --git a/cmd/dep/root_analyzer.go b/cmd/dep/root_analyzer.go index 017e5a202b..0dcf8683b3 100644 --- a/cmd/dep/root_analyzer.go +++ b/cmd/dep/root_analyzer.go @@ -40,10 +40,7 @@ func newRootAnalyzer(skipTools bool, ctx *dep.Ctx, directDeps map[string]bool, s func (a *rootAnalyzer) InitializeRootManifestAndLock(dir string, pr gps.ProjectRoot) (rootM *dep.Manifest, rootL *dep.Lock, err error) { if !a.skipTools { - rootM, rootL, err = a.importManifestAndLock(dir, pr, false) - if err != nil { - return - } + rootM, rootL = a.importManifestAndLock(dir, pr, false) } if rootM == nil { @@ -106,7 +103,7 @@ func (a *rootAnalyzer) cacheDeps(pr gps.ProjectRoot) error { return nil } -func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, suppressLogs bool) (*dep.Manifest, *dep.Lock, error) { +func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, suppressLogs bool) (*dep.Manifest, *dep.Lock) { logger := a.ctx.Err if suppressLogs { logger = log.New(ioutil.Discard, "", 0) @@ -117,16 +114,20 @@ func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, sup a.ctx.Err.Printf("Importing configuration from %s. These are only initial constraints, and are further refined during the solve process.", i.Name()) m, l, err := i.Import(dir, pr) if err != nil { - return nil, nil, err + a.ctx.Err.Printf( + "Warning: Encountered an unrecoverable error while trying to import %s config from %q: %s", + i.Name(), dir, err, + ) + break } a.removeTransitiveDependencies(m) - return m, l, err + return m, l } } var emptyManifest = dep.NewManifest() - return emptyManifest, nil, nil + return emptyManifest, nil } func (a *rootAnalyzer) removeTransitiveDependencies(m *dep.Manifest) { @@ -151,14 +152,14 @@ func (a *rootAnalyzer) DeriveManifestAndLock(dir string, pr gps.ProjectRoot) (gp // The assignment back to an interface prevents interface-based nil checks from failing later var manifest gps.Manifest = gps.SimpleManifest{} var lock gps.Lock - im, il, err := a.importManifestAndLock(dir, pr, true) + im, il := a.importManifestAndLock(dir, pr, true) if im != nil { manifest = im } if il != nil { lock = il } - return manifest, lock, err + return manifest, lock, nil } return gps.SimpleManifest{}, nil, nil diff --git a/cmd/dep/testdata/harness_tests/init/glide/case4/README.md b/cmd/dep/testdata/harness_tests/init/glide/case4/README.md new file mode 100644 index 0000000000..912e40844f --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/glide/case4/README.md @@ -0,0 +1 @@ +Ignore glide config if glide.yaml is malformed and cannot be parsed correctly. diff --git a/cmd/dep/testdata/harness_tests/init/glide/case4/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/init/glide/case4/final/Gopkg.lock new file mode 100644 index 0000000000..1aadf7f962 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/glide/case4/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 = "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" + version = "v1.0.0" + +[[projects]] + name = "github.com/sdboyer/deptestdos" + packages = ["."] + revision = "5c607206be5decd28e6263ffffdcee067266015e" + version = "v2.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "1ed417a0bec57ffe988fae1cba8f3d49994fb893394d61844e0b3c96d69573fe" + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/init/glide/case4/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/glide/case4/final/Gopkg.toml new file mode 100644 index 0000000000..aaf78303fa --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/glide/case4/final/Gopkg.toml @@ -0,0 +1,4 @@ + +[[constraint]] + name = "github.com/sdboyer/deptestdos" + version = "2.0.0" diff --git a/cmd/dep/testdata/harness_tests/init/glide/case4/initial/glide.yaml b/cmd/dep/testdata/harness_tests/init/glide/case4/initial/glide.yaml new file mode 100644 index 0000000000..a9c8e891fb --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/glide/case4/initial/glide.yaml @@ -0,0 +1,20 @@ +'package: github.com/golang/notexist +homepage: http://example.com +license: MIT +owners: +- name: Sam Boyer + email: sdboyer@example.com + homepage: http://sdboyer.io +ignore: +- github.com/sdboyer/dep-test +excludeDirs: +- samples +import: +- package: github.com/sdboyer/deptest # This is a transitive dep and will be ignored + repo: https://github.com/sdboyer/deptest.git + vcs: git + version: v1.0.0 +- package: github.com/sdboyer/deptestdos + version: v2.0.0 +testImport: +- package: github.com/golang/lint diff --git a/cmd/dep/testdata/harness_tests/init/glide/case4/initial/main.go b/cmd/dep/testdata/harness_tests/init/glide/case4/initial/main.go new file mode 100644 index 0000000000..2b2c7c396e --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/glide/case4/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/glide/case4/testcase.json b/cmd/dep/testdata/harness_tests/init/glide/case4/testcase.json new file mode 100644 index 0000000000..85a05ed658 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/glide/case4/testcase.json @@ -0,0 +1,14 @@ +{ + "commands": [ + ["init", "-no-examples"] + ], + "error-expected": "", + "gopath-initial": { + "github.com/sdboyer/deptest": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", + "github.com/sdboyer/deptestdos": "5c607206be5decd28e6263ffffdcee067266015e" + }, + "vendor-final": [ + "github.com/sdboyer/deptest", + "github.com/sdboyer/deptestdos" + ] +} diff --git a/internal/importers/base/importer.go b/internal/importers/base/importer.go index 7abc3d2bde..0091663039 100644 --- a/internal/importers/base/importer.go +++ b/internal/importers/base/importer.go @@ -123,7 +123,7 @@ type importedProject struct { } // loadPackages consolidates all package references into a set of project roots. -func (i *Importer) loadPackages(packages []ImportedPackage) ([]importedProject, error) { +func (i *Importer) loadPackages(packages []ImportedPackage) []importedProject { // preserve the original order of the packages so that messages that // are printed as they are processed are in a consistent order. orderedProjects := make([]importedProject, 0, len(packages)) @@ -132,7 +132,11 @@ func (i *Importer) loadPackages(packages []ImportedPackage) ([]importedProject, for _, pkg := range packages { pr, err := i.SourceManager.DeduceProjectRoot(pkg.Name) if err != nil { - return nil, errors.Wrapf(err, "Cannot determine the project root for %s", pkg.Name) + i.Logger.Printf( + " Warning: Skipping project. Cannot determine the project root for %s: %s\n", + pkg.Name, err, + ) + continue } pkg.Name = string(pr) @@ -159,7 +163,7 @@ func (i *Importer) loadPackages(packages []ImportedPackage) ([]importedProject, } } - return orderedProjects, nil + return orderedProjects } // ImportPackages loads imported packages into the manifest and lock. @@ -173,11 +177,8 @@ func (i *Importer) loadPackages(packages []ImportedPackage) ([]importedProject, // * Revision constraints are ignored. // * Versions that don't satisfy the constraint, drop the constraint. // * Untagged revisions ignore non-branch constraint hints. -func (i *Importer) ImportPackages(packages []ImportedPackage, defaultConstraintFromLock bool) (err error) { - projects, err := i.loadPackages(packages) - if err != nil { - return err - } +func (i *Importer) ImportPackages(packages []ImportedPackage, defaultConstraintFromLock bool) { + projects := i.loadPackages(packages) for _, prj := range projects { source := prj.Source @@ -201,6 +202,7 @@ func (i *Importer) ImportPackages(packages []ImportedPackage, defaultConstraintF }, } + var err error pc.Constraint, err = i.SourceManager.InferConstraint(prj.ConstraintHint, pc.Ident) if err != nil { pc.Constraint = gps.Any() @@ -212,9 +214,12 @@ func (i *Importer) ImportPackages(packages []ImportedPackage, defaultConstraintF // Determine if the lock hint is a revision or tag isTag, version, err = i.isTag(pc.Ident, prj.LockHint) if err != nil { - return err + i.Logger.Printf( + " Warning: Skipping project. Unable to apply constraint %q for %v: %s\n", + prj.LockHint, pc.Ident, err, + ) + continue } - // If the hint is a revision, check if it is tagged if !isTag { revision := gps.Revision(prj.LockHint) @@ -270,8 +275,6 @@ func (i *Importer) ImportPackages(packages []ImportedPackage, defaultConstraintF fb.NewLockedProjectFeedback(lp, fb.DepTypeImported).LogFeedback(i.Logger) } } - - return nil } // isConstraintPinned returns if a constraint is pinned to a specific revision. diff --git a/internal/importers/base/importer_test.go b/internal/importers/base/importer_test.go index 834928be64..e5d3e7c8b0 100644 --- a/internal/importers/base/importer_test.go +++ b/internal/importers/base/importer_test.go @@ -5,6 +5,7 @@ package base import ( + "fmt" "log" "testing" @@ -399,16 +400,42 @@ func TestBaseImporter_ImportProjects(t *testing.T) { }, }, }, + "invalid project root": { + importertest.TestCase{ + WantSourceRepo: "", + WantWarning: "Warning: Skipping project. Cannot determine the project root for invalid-project", + }, + []ImportedPackage{ + { + Name: "invalid-project", + }, + }, + }, + "nonexistent project": { + importertest.TestCase{ + WantSourceRepo: "", + WantWarning: fmt.Sprintf( + "Warning: Skipping project. Unable to apply constraint %q for %s", + importertest.V1Tag, importertest.NonexistentPrj, + ), + }, + []ImportedPackage{ + { + Name: importertest.NonexistentPrj, + LockHint: importertest.V1Tag, + }, + }, + }, } for name, tc := range testcases { name := name tc := tc t.Run(name, func(t *testing.T) { - err := tc.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) { + err := tc.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { i := NewImporter(logger, true, sm) - convertErr := i.ImportPackages(tc.projects, tc.DefaultConstraintFromLock) - return i.Manifest, i.Lock, convertErr + i.ImportPackages(tc.projects, tc.DefaultConstraintFromLock) + return i.Manifest, i.Lock }) if err != nil { t.Fatalf("%#v", err) diff --git a/internal/importers/glide/importer.go b/internal/importers/glide/importer.go index 6c0139ac73..ebaa3e2d8c 100644 --- a/internal/importers/glide/importer.go +++ b/internal/importers/glide/importer.go @@ -89,10 +89,13 @@ func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L return nil, nil, err } - return g.convert(pr) + m, l := g.convert(pr) + return m, l, nil } -// load the glide configuration files. +// load the glide configuration files. Failure to load `glide.yaml` is considered +// unrecoverable and an error is returned for it. But if there is any error while trying +// to load the lock file, only a warning is logged. func (g *Importer) load(projectDir string) error { g.Logger.Println("Detected glide configuration files...") y := filepath.Join(projectDir, glideYamlName) @@ -113,16 +116,18 @@ func (g *Importer) load(projectDir string) error { if g.Verbose { g.Logger.Printf(" Loading %s", l) } - g.lockFound = true lb, err := ioutil.ReadFile(l) if err != nil { - return errors.Wrapf(err, "unable to read %s", l) + g.Logger.Printf(" Warning: Ignoring lock file. Unable to read %s: %s\n", l, err) + return nil } lock := glideLock{} err = yaml.Unmarshal(lb, &lock) if err != nil { - return errors.Wrapf(err, "unable to parse %s", l) + g.Logger.Printf(" Warning: Ignoring lock file. Unable to parse %s: %s\n", l, err) + return nil } + g.lockFound = true g.glideLock = lock } @@ -130,7 +135,7 @@ func (g *Importer) load(projectDir string) error { } // convert the glide configuration files into dep configuration files. -func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { +func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock) { projectName := string(pr) task := bytes.NewBufferString("Converting from glide.yaml") @@ -147,7 +152,10 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) for _, pkg := range append(g.glideConfig.Imports, g.glideConfig.TestImports...) { // Validate if pkg.Name == "" { - return nil, nil, errors.New("invalid glide configuration: Name is required") + g.Logger.Println( + " Warning: Skipping project. Invalid glide configuration, Name is required", + ) + continue } // Warn @@ -172,7 +180,8 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) for _, pkg := range append(g.glideLock.Imports, g.glideLock.TestImports...) { // Validate if pkg.Name == "" { - return nil, nil, errors.New("invalid glide lock: Name is required") + g.Logger.Println(" Warning: Skipping project. Invalid glide lock, Name is required") + continue } ip := base.ImportedPackage{ @@ -183,10 +192,7 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) packages = append(packages, ip) } - err := g.ImportPackages(packages, false) - if err != nil { - return nil, nil, errors.Wrap(err, "invalid glide configuration") - } + g.ImportPackages(packages, false) // Ignores g.Manifest.Ignored = append(g.Manifest.Ignored, g.glideConfig.Ignores...) @@ -201,5 +207,5 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) } } - return g.Manifest, g.Lock, nil + return g.Manifest, g.Lock } diff --git a/internal/importers/glide/importer_test.go b/internal/importers/glide/importer_test.go index 1a7fd2b19d..a51f2a9b35 100644 --- a/internal/importers/glide/importer_test.go +++ b/internal/importers/glide/importer_test.go @@ -125,7 +125,7 @@ func TestGlideConfig_Convert(t *testing.T) { }, glideLock{}, importertest.TestCase{ - WantConvertErr: true, + WantWarning: "Warning: Skipping project. Invalid glide configuration, Name is required", }, }, "warn unused os field": { @@ -160,7 +160,7 @@ func TestGlideConfig_Convert(t *testing.T) { 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) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { g := NewImporter(logger, true, sm) g.glideConfig = testCase.yaml g.glideLock = testCase.lock diff --git a/internal/importers/glock/importer.go b/internal/importers/glock/importer.go index a1d1cb8672..6120d459aa 100644 --- a/internal/importers/glock/importer.go +++ b/internal/importers/glock/importer.go @@ -54,7 +54,8 @@ func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L return nil, nil, err } - return g.convert(pr) + m, l := g.convert(pr) + return m, l, nil } type glockPackage struct { @@ -79,7 +80,8 @@ func (g *Importer) load(projectDir string) error { for scanner.Scan() { pkg, err := parseGlockLine(scanner.Text()) if err != nil { - return err + g.Logger.Printf(" Warning: Skipping line. Unable to parse: %s\n", err) + continue } if pkg == nil { continue @@ -87,6 +89,10 @@ func (g *Importer) load(projectDir string) error { g.packages = append(g.packages, *pkg) } + if err := scanner.Err(); err != nil { + g.Logger.Printf(" Warning: Ignoring errors found while parsing %s: %s\n", path, err) + } + return nil } @@ -110,18 +116,27 @@ func parseGlockLine(line string) (*glockPackage, error) { }, nil } -func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { +func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock) { g.Logger.Println("Converting from GLOCKFILE ...") packages := make([]base.ImportedPackage, 0, len(g.packages)) for _, pkg := range g.packages { // Validate if pkg.importPath == "" { - return nil, nil, errors.New("invalid glock configuration, import path is required") + g.Logger.Println( + " Warning: Skipping project. Invalid glock configuration, import path is required", + ) + continue } if pkg.revision == "" { - return nil, nil, errors.New("invalid glock configuration, revision is required") + // Do not add 'empty constraints' to the manifest. Solve will add to lock if required. + g.Logger.Printf( + " Warning: Skipping import with empty constraints. "+ + "The solve step will add the dependency to the lock if needed: %q\n", + pkg.importPath, + ) + continue } packages = append(packages, base.ImportedPackage{ @@ -130,10 +145,6 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) }) } - err := g.ImportPackages(packages, true) - if err != nil { - return nil, nil, err - } - - return g.Manifest, g.Lock, nil + g.ImportPackages(packages, true) + return g.Manifest, g.Lock } diff --git a/internal/importers/glock/importer_test.go b/internal/importers/glock/importer_test.go index cc38c7b8c2..3cb8138d46 100644 --- a/internal/importers/glock/importer_test.go +++ b/internal/importers/glock/importer_test.go @@ -38,13 +38,17 @@ func TestGlockConfig_Convert(t *testing.T) { }, "missing package name": { importertest.TestCase{ - WantConvertErr: true, + WantWarning: "Warning: Skipping project. Invalid glock configuration, import path is required", }, []glockPackage{{importPath: ""}}, }, "missing revision": { importertest.TestCase{ - WantConvertErr: true, + WantWarning: fmt.Sprintf( + " Warning: Skipping import with empty constraints. "+ + "The solve step will add the dependency to the lock if needed: %q", + importertest.Project, + ), }, []glockPackage{{importPath: importertest.Project}}, }, @@ -54,7 +58,7 @@ func TestGlockConfig_Convert(t *testing.T) { 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) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { g := NewImporter(logger, true, sm) g.packages = testCase.packages return g.convert(importertest.RootProject) diff --git a/internal/importers/godep/importer.go b/internal/importers/godep/importer.go index dbcfc04c40..c3389d8cb5 100644 --- a/internal/importers/godep/importer.go +++ b/internal/importers/godep/importer.go @@ -62,7 +62,8 @@ func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L return nil, nil, err } - return g.convert(pr) + m, l := g.convert(pr) + return m, l, nil } func (g *Importer) load(projectDir string) error { @@ -83,20 +84,24 @@ func (g *Importer) load(projectDir string) error { return nil } -func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { +func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock) { g.Logger.Println("Converting from Godeps.json ...") packages := make([]base.ImportedPackage, 0, len(g.json.Imports)) for _, pkg := range g.json.Imports { // Validate if pkg.ImportPath == "" { - err := errors.New("invalid godep configuration, ImportPath is required") - return nil, nil, err + g.Logger.Println( + " Warning: Skipping project. Invalid godep configuration, ImportPath is required", + ) + continue } if pkg.Rev == "" { - err := errors.New("invalid godep configuration, Rev is required") - return nil, nil, err + g.Logger.Printf( + " Warning: Invalid godep configuration, Rev not found for ImportPath %q\n", + pkg.ImportPath, + ) } ip := base.ImportedPackage{ @@ -107,10 +112,6 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) packages = append(packages, ip) } - err := g.ImportPackages(packages, true) - if err != nil { - return nil, nil, err - } - - return g.Manifest, g.Lock, nil + g.ImportPackages(packages, true) + return g.Manifest, g.Lock } diff --git a/internal/importers/godep/importer_test.go b/internal/importers/godep/importer_test.go index 12e067d8f2..ec9969f8ba 100644 --- a/internal/importers/godep/importer_test.go +++ b/internal/importers/godep/importer_test.go @@ -6,6 +6,7 @@ package godep import ( "bytes" + "fmt" "log" "path/filepath" "testing" @@ -55,7 +56,7 @@ func TestGodepConfig_Convert(t *testing.T) { }, "missing package name": { importertest.TestCase{ - WantConvertErr: true, + WantWarning: "Warning: Skipping project. Invalid godep configuration, ImportPath is required", }, godepJSON{ Imports: []godepPackage{{ImportPath: ""}}, @@ -63,7 +64,10 @@ func TestGodepConfig_Convert(t *testing.T) { }, "missing revision": { importertest.TestCase{ - WantConvertErr: true, + WantWarning: fmt.Sprintf( + "Warning: Invalid godep configuration, Rev not found for ImportPath %q", + importertest.Project, + ), }, godepJSON{ Imports: []godepPackage{ @@ -79,7 +83,7 @@ func TestGodepConfig_Convert(t *testing.T) { 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) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { g := NewImporter(logger, true, sm) g.json = testCase.json return g.convert(importertest.RootProject) diff --git a/internal/importers/govend/importer.go b/internal/importers/govend/importer.go index b3f1972699..e4af4368c3 100644 --- a/internal/importers/govend/importer.go +++ b/internal/importers/govend/importer.go @@ -63,7 +63,8 @@ func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L return nil, nil, err } - return g.convert(pr) + m, l := g.convert(pr) + return m, l, nil } // load the govend configuration files. @@ -85,14 +86,27 @@ func (g *Importer) load(projectDir string) error { } // convert the govend configuration files into dep configuration files. -func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { +func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock) { g.Logger.Println("Converting from vendor.yaml...") packages := make([]base.ImportedPackage, 0, len(g.yaml.Imports)) for _, pkg := range g.yaml.Imports { // Path must not be empty - if pkg.Path == "" || pkg.Revision == "" { - return nil, nil, errors.New("invalid govend configuration, path and rev are required") + if pkg.Path == "" { + g.Logger.Println( + " Warning: Skipping project. Invalid govend configuration, path is required", + ) + continue + } + + if pkg.Revision == "" { + // Do not add 'empty constraints' to the manifest. Solve will add to lock if required. + g.Logger.Printf( + " Warning: Skipping import with empty constraints. "+ + "The solve step will add the dependency to the lock if needed: %q\n", + pkg.Path, + ) + continue } ip := base.ImportedPackage{ @@ -102,10 +116,6 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) packages = append(packages, ip) } - err := g.ImportPackages(packages, true) - if err != nil { - return nil, nil, err - } - - return g.Manifest, g.Lock, nil + g.ImportPackages(packages, true) + return g.Manifest, g.Lock } diff --git a/internal/importers/govend/importer_test.go b/internal/importers/govend/importer_test.go index 6aa3ae19e5..e949efaced 100644 --- a/internal/importers/govend/importer_test.go +++ b/internal/importers/govend/importer_test.go @@ -6,6 +6,7 @@ package govend import ( "bytes" + "fmt" "log" "path/filepath" "testing" @@ -46,7 +47,7 @@ func TestGovendConfig_Convert(t *testing.T) { }, }, importertest.TestCase{ - WantConvertErr: true, + WantWarning: "Warning: Skipping project. Invalid govend configuration, path is required", }, }, @@ -59,7 +60,11 @@ func TestGovendConfig_Convert(t *testing.T) { }, }, importertest.TestCase{ - WantConvertErr: true, + WantWarning: fmt.Sprintf( + " Warning: Skipping import with empty constraints. "+ + "The solve step will add the dependency to the lock if needed: %q\n", + importertest.Project, + ), }, }, } @@ -68,7 +73,7 @@ func TestGovendConfig_Convert(t *testing.T) { 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) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { g := NewImporter(logger, true, sm) g.yaml = testCase.yaml return g.convert(importertest.RootProject) diff --git a/internal/importers/govendor/importer.go b/internal/importers/govendor/importer.go index f7a5d44d2f..09611f49ed 100644 --- a/internal/importers/govendor/importer.go +++ b/internal/importers/govendor/importer.go @@ -70,7 +70,9 @@ func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L if err != nil { return nil, nil, err } - return g.convert(pr) + + m, l := g.convert(pr) + return m, l, nil } func (g *Importer) load(projectDir string) error { @@ -90,15 +92,17 @@ func (g *Importer) load(projectDir string) error { return nil } -func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { +func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock) { g.Logger.Println("Converting from vendor.json...") packages := make([]base.ImportedPackage, 0, len(g.file.Package)) for _, pkg := range g.file.Package { // Path must not be empty if pkg.Path == "" { - err := errors.New("invalid govendor configuration, Path is required") - return nil, nil, err + g.Logger.Println( + " Warning: Skipping project. Invalid govendor configuration, Path is required", + ) + continue } // There are valid govendor configs in the wild that don't have a revision set @@ -112,10 +116,7 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) packages = append(packages, ip) } - err := g.ImportPackages(packages, true) - if err != nil { - return nil, nil, err - } + g.ImportPackages(packages, true) if len(g.file.Ignore) > 0 { // Govendor has three use cases here @@ -146,5 +147,5 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) } } - return g.Manifest, g.Lock, nil + return g.Manifest, g.Lock } diff --git a/internal/importers/govendor/importer_test.go b/internal/importers/govendor/importer_test.go index 02707cf215..66ed7ff58f 100644 --- a/internal/importers/govendor/importer_test.go +++ b/internal/importers/govendor/importer_test.go @@ -121,7 +121,7 @@ func TestGovendorConfig_Convert(t *testing.T) { }, }, importertest.TestCase{ - WantConvertErr: true, + WantWarning: "Warning: Skipping project. Invalid govendor configuration, Path is required", }, }, "missing package revision doesn't cause an error": { @@ -142,7 +142,7 @@ func TestGovendorConfig_Convert(t *testing.T) { 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) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { g := NewImporter(logger, true, sm) g.file = testCase.file return g.convert(importertest.RootProject) diff --git a/internal/importers/gvt/importer.go b/internal/importers/gvt/importer.go index 611e5bf07e..97f61a16be 100644 --- a/internal/importers/gvt/importer.go +++ b/internal/importers/gvt/importer.go @@ -63,7 +63,8 @@ func (g *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L return nil, nil, err } - return g.convert(pr) + m, l := g.convert(pr) + return m, l, nil } func (g *Importer) load(projectDir string) error { @@ -84,20 +85,24 @@ func (g *Importer) load(projectDir string) error { return nil } -func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { +func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock) { 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 + g.Logger.Println( + " Warning: Skipping project. Invalid gvt configuration, ImportPath is required", + ) + continue } if pkg.Revision == "" { - err := errors.New("invalid gvt configuration, Revision is required") - return nil, nil, err + g.Logger.Printf( + " Warning: Invalid gvt configuration, Revision not found for ImportPath %q\n", + pkg.ImportPath, + ) } var contstraintHint = "" @@ -120,10 +125,6 @@ func (g *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) packages = append(packages, ip) } - err := g.ImportPackages(packages, true) - if err != nil { - return nil, nil, err - } - - return g.Manifest, g.Lock, nil + g.ImportPackages(packages, true) + return g.Manifest, g.Lock } diff --git a/internal/importers/gvt/importer_test.go b/internal/importers/gvt/importer_test.go index 7dc3ff7652..cf72f1b229 100644 --- a/internal/importers/gvt/importer_test.go +++ b/internal/importers/gvt/importer_test.go @@ -6,6 +6,7 @@ package gvt import ( "bytes" + "fmt" "log" "path/filepath" "testing" @@ -89,7 +90,7 @@ func TestGvtConfig_Convert(t *testing.T) { }, "missing package name": { importertest.TestCase{ - WantConvertErr: true, + WantWarning: "Warning: Skipping project. Invalid gvt configuration, ImportPath is required", }, gvtManifest{ Deps: []gvtPkg{{ImportPath: ""}}, @@ -97,7 +98,10 @@ func TestGvtConfig_Convert(t *testing.T) { }, "missing revision": { importertest.TestCase{ - WantConvertErr: true, + WantWarning: fmt.Sprintf( + "Warning: Invalid gvt configuration, Revision not found for ImportPath %q", + importertest.Project, + ), }, gvtManifest{ Deps: []gvtPkg{ @@ -113,7 +117,7 @@ func TestGvtConfig_Convert(t *testing.T) { 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) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { g := NewImporter(logger, true, sm) g.gvtConfig = testCase.gvtConfig return g.convert(importertest.RootProject) diff --git a/internal/importers/importertest/testcase.go b/internal/importers/importertest/testcase.go index 0d629531ec..59b589e14e 100644 --- a/internal/importers/importertest/testcase.go +++ b/internal/importers/importertest/testcase.go @@ -22,7 +22,6 @@ import ( // of an importer converting from an external config format to dep's. type TestCase struct { DefaultConstraintFromLock bool - WantConvertErr bool WantSourceRepo string WantConstraint string WantRevision gps.Revision @@ -45,7 +44,7 @@ func NewTestContext(h *test.Helper) *dep.Ctx { } // Execute and validate the test case. -func (tc TestCase) Execute(t *testing.T, convert func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error)) error { +func (tc TestCase) Execute(t *testing.T, convert func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock)) error { h := test.NewHelper(t) defer h.Cleanup() // Disable parallel tests until we can resolve this error on the Windows builds: @@ -61,23 +60,12 @@ func (tc TestCase) Execute(t *testing.T, convert func(logger *log.Logger, sm gps output := &bytes.Buffer{} ctx.Err = log.New(output, "", 0) - manifest, lock, convertErr := convert(ctx.Err, sm) - return tc.validate(manifest, lock, convertErr, output) + manifest, lock := convert(ctx.Err, sm) + return tc.validate(manifest, lock, output) } // validate returns an error if any of the testcase validations failed. -func (tc TestCase) validate(manifest *dep.Manifest, lock *dep.Lock, convertErr error, output *bytes.Buffer) error { - if tc.WantConvertErr { - if convertErr == nil { - return errors.New("Expected the conversion to fail, but it did not return an error") - } - return nil - } - - if convertErr != nil { - return errors.Wrap(convertErr, "Expected the conversion to pass, but it returned an error") - } - +func (tc TestCase) validate(manifest *dep.Manifest, lock *dep.Lock, output *bytes.Buffer) error { if !equalSlice(manifest.Ignored, tc.WantIgnored) { return errors.Errorf("unexpected set of ignored projects: \n\t(GOT) %#v \n\t(WNT) %#v", manifest.Ignored, tc.WantIgnored) diff --git a/internal/importers/importertest/testdata.go b/internal/importers/importertest/testdata.go index cd7f2c0879..fc037ff0dd 100644 --- a/internal/importers/importertest/testdata.go +++ b/internal/importers/importertest/testdata.go @@ -61,4 +61,7 @@ const ( // MultiTaggedPlainTag is a non-semver tag on MultiTaggedRev. MultiTaggedPlainTag = "stable" + + // NonexistentPrj is a dummy project which does not exist on Github. + NonexistentPrj = "github.com/nonexistent/project" ) diff --git a/internal/importers/vndr/importer.go b/internal/importers/vndr/importer.go index e8a162b3f5..60e28e2320 100644 --- a/internal/importers/vndr/importer.go +++ b/internal/importers/vndr/importer.go @@ -50,15 +50,17 @@ func (v *Importer) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.L return nil, nil, errors.Wrapf(err, "unable to load vndr file") } - return v.convert(pr) + m, l := v.convert(pr) + return m, l, nil } func (v *Importer) loadVndrFile(dir string) error { v.Logger.Printf("Converting from vendor.conf...") - f, err := os.Open(vndrFile(dir)) + path := vndrFile(dir) + f, err := os.Open(path) if err != nil { - return errors.Wrapf(err, "unable to open %s", vndrFile(dir)) + return errors.Wrapf(err, "unable to open %s", path) } defer f.Close() @@ -66,7 +68,8 @@ func (v *Importer) loadVndrFile(dir string) error { for scanner.Scan() { pkg, err := parseVndrLine(scanner.Text()) if err != nil { - return errors.Wrapf(err, "unable to parse line") + v.Logger.Printf(" Warning: Skipping line. Unable to parse: %s\n", err) + continue } if pkg == nil { // Could be an empty line or one which is just a comment @@ -75,25 +78,29 @@ func (v *Importer) loadVndrFile(dir string) error { v.packages = append(v.packages, *pkg) } - if scanner.Err() != nil { - return errors.Wrapf(err, "unable to read %s", vndrFile(dir)) + if err := scanner.Err(); err != nil { + v.Logger.Printf(" Warning: Ignoring errors found while parsing %s: %s\n", path, err) } return nil } -func (v *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { +func (v *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock) { packages := make([]base.ImportedPackage, 0, len(v.packages)) for _, pkg := range v.packages { // Validate if pkg.importPath == "" { - err := errors.New("invalid vndr configuration: import path is required") - return nil, nil, err + v.Logger.Println( + " Warning: Skipping project. Invalid vndr configuration, import path is required", + ) + continue } if pkg.reference == "" { - err := errors.New("invalid vndr configuration: revision is required") - return nil, nil, err + v.Logger.Printf( + " Warning: Invalid vndr configuration, reference not found for import path %q\n", + pkg.importPath, + ) } ip := base.ImportedPackage{ @@ -103,12 +110,8 @@ func (v *Importer) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) } packages = append(packages, ip) } - err := v.ImportPackages(packages, true) - if err != nil { - return nil, nil, err - } - - return v.Manifest, v.Lock, nil + v.ImportPackages(packages, true) + return v.Manifest, v.Lock } type vndrPackage struct { diff --git a/internal/importers/vndr/importer_test.go b/internal/importers/vndr/importer_test.go index a445c28198..db4b1794c8 100644 --- a/internal/importers/vndr/importer_test.go +++ b/internal/importers/vndr/importer_test.go @@ -6,6 +6,7 @@ package vndr import ( "bytes" + "fmt" "log" "path/filepath" "reflect" @@ -41,7 +42,7 @@ func TestVndrConfig_Convert(t *testing.T) { reference: importertest.V1Tag, }}, importertest.TestCase{ - WantConvertErr: true, + WantWarning: "Warning: Skipping project. Invalid vndr configuration, import path is required", }, }, "missing reference": { @@ -49,7 +50,10 @@ func TestVndrConfig_Convert(t *testing.T) { importPath: importertest.Project, }}, importertest.TestCase{ - WantConvertErr: true, + WantWarning: fmt.Sprintf( + "Warning: Invalid vndr configuration, reference not found for import path %q", + importertest.Project, + ), }, }, } @@ -58,7 +62,7 @@ func TestVndrConfig_Convert(t *testing.T) { 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) { + err := testCase.Execute(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock) { g := NewImporter(logger, true, sm) g.packages = testCase.packages return g.convert(importertest.RootProject)