Tidy up new import interfaces

cmd/dep/compositeAnalyzer.go

@@ -65,8 +65,8 @@ func (a compositeAnalyzer) DeriveRootManifestAndLock(path string, n gps.ProjectR
 	return rootM, rootL, nil
 }
 
-func (a compositeAnalyzer) PostSolveShenanigans(m *dep.Manifest, l *dep.Lock) {
+func (a compositeAnalyzer) FinalizeManifestAndLock(m *dep.Manifest, l *dep.Lock) {
 	for _, a := range a.Analyzers {
-		a.PostSolveShenanigans(m, l)
+		a.FinalizeManifestAndLock(m, l)
 	}
 }

cmd/dep/compositeAnalyzer_test.go

@@ -20,7 +20,7 @@ func (a testRootProjectAnalyzer) DeriveRootManifestAndLock(path string, n gps.Pr
 	return a.Manifest, a.Lock, nil
 }
 
-func (a testRootProjectAnalyzer) PostSolveShenanigans(*dep.Manifest, *dep.Lock) {
+func (a testRootProjectAnalyzer) FinalizeManifestAndLock(*dep.Manifest, *dep.Lock) {
 	// do nothing
 }
 

cmd/dep/ensure_test.go

@@ -52,7 +52,7 @@ func TestDeduceConstraint_InvalidInput(t *testing.T) {
 	sm, err := gps.NewSourceManager(h.Path(cacheDir))
 	h.Must(err)
 
-	constraints := []string {
+	constraints := []string{
 		// invalid bzr revs
 		"go4@golang.org-lskjdfnkjsdnf-ksjdfnskjdfn",
 		//"go4@golang.org-sadfasdf-",

cmd/dep/glideImporter.go

@@ -17,17 +17,13 @@ const glideLockName = "glide.lock"
 
 type glideImporter struct {
 	loggers *dep.Loggers
-	sm gps.SourceManager
+	sm      gps.SourceManager
 }
 
 func newGlideImporter(loggers *dep.Loggers, sm gps.SourceManager) glideImporter {
 	return glideImporter{loggers: loggers, sm: sm}
 }
 
-func (i glideImporter) Info() (name string, version int) {
-	return "glide", 1
-}
-
 func (i glideImporter) HasConfig(dir string) bool {
 	// Only require glide.yaml, the lock is optional
 	y := filepath.Join(dir, glideYamlName)
@@ -38,7 +34,7 @@ func (i glideImporter) HasConfig(dir string) bool {
 	return true
 }
 
-func (i glideImporter) DeriveRootManifestAndLock(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
+func (i glideImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
 	files := newGlideFiles(i.loggers)
 	err := files.load(dir)
 	if err != nil {
@@ -47,12 +43,3 @@ func (i glideImporter) DeriveRootManifestAndLock(dir string, pr gps.ProjectRoot)
 
 	return files.convert(string(pr), i.sm)
 }
-
-func (i glideImporter) DeriveManifestAndLock(dir string, pr gps.ProjectRoot) (gps.Manifest, gps.Lock, error) {
-	return i.DeriveRootManifestAndLock(dir, pr)
-}
-
-func (a glideImporter) PostSolveShenanigans(*dep.Manifest, *dep.Lock) {
-	// do nothing
-	// TODO: importers don't need to be full root analyzers
-}

cmd/dep/glideImporter_test.go

@@ -42,7 +42,7 @@ func TestGlideImport(t *testing.T) {
 		t.Fatal("Expected the importer to detect the glide configuration files")
 	}
 
-	m, l, err := i.DeriveRootManifestAndLock(projectRoot, gps.ProjectRoot(testGlideProjectRoot))
+	m, l, err := i.Import(projectRoot, gps.ProjectRoot(testGlideProjectRoot))
 	h.Must(err)
 
 	if m == nil {
@@ -78,7 +78,7 @@ func TestGlideImport_MissingLockFile(t *testing.T) {
 		t.Fatal("The glide importer should gracefully handle when only glide.yaml is present")
 	}
 
-	m, l, err := i.DeriveRootManifestAndLock(projectRoot, gps.ProjectRoot(testGlideProjectRoot))
+	m, l, err := i.Import(projectRoot, gps.ProjectRoot(testGlideProjectRoot))
 	h.Must(err)
 
 	if m == nil {

cmd/dep/gopathAnalyzer.go

@@ -6,9 +6,9 @@ package main
 
 import (
 	"github.com/golang/dep"
+	fb "github.com/golang/dep/internal/feedback"
 	"github.com/golang/dep/internal/gps"
 	"github.com/golang/dep/internal/gps/pkgtree"
-	fb "github.com/golang/dep/internal/feedback"
 )
 
 // gopathAnalyzer deduces configuration from the projects in the GOPATH
@@ -31,6 +31,9 @@ func newGopathAnalyzer(ctx *dep.Ctx, pkgT pkgtree.PackageTree, cpr string, sm *g
 	}
 }
 
+// Perform analysis of the filesystem tree rooted at path, with the
+// root import path importRoot, to determine the project's constraints, as
+// indicated by a Manifest and Lock.
 func (a *gopathAnalyzer) DeriveRootManifestAndLock(path string, n gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
 	var err error
 
@@ -72,7 +75,7 @@ func (a *gopathAnalyzer) DeriveRootManifestAndLock(path string, n gps.ProjectRoo
 	return m, l, nil
 }
 
-func (a *gopathAnalyzer) PostSolveShenanigans(m *dep.Manifest, l *dep.Lock) {
+func (a *gopathAnalyzer) FinalizeManifestAndLock(m *dep.Manifest, l *dep.Lock) {
 	// Iterate through the new projects in solved lock and add them to manifest
 	// if direct deps and log feedback for all the new projects.
 	for _, x := range l.Projects() {

cmd/dep/importAnalyzer.go

@@ -11,8 +11,7 @@ import (
 
 // importer
 type importer interface {
-	gps.ProjectAnalyzer
-	rootProjectAnalyzer
+	Import(path string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error)
 	HasConfig(dir string) bool
 }
 
@@ -27,49 +26,54 @@ func newImportAnalyzer(loggers *dep.Loggers, sm gps.SourceManager) importAnalyze
 	return importAnalyzer{loggers: loggers, sm: sm}
 }
 
-func (a importAnalyzer) Info() (string, int) {
-	// TODO: do not merge until this is set to something unique.
-	// I'm not changing it now because that will cause the memo to change in tests
-	// which I'll deal with and update later
-	return "dep", 1
-}
+func (a importAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
+	importers := []importer{
+		newGlideImporter(a.loggers, a.sm),
+	}
 
-func (a importAnalyzer) DeriveRootManifestAndLock(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
-	var importers []importer = []importer{newGlideImporter(a.loggers, a.sm)}
 	for _, i := range importers {
 		if i.HasConfig(dir) {
-			tool, _ := i.Info()
 			if a.loggers.Verbose {
-				a.loggers.Err.Printf("Importing %s configuration for %s. Run with -skip-tools to skip.", tool, pr)
+				a.loggers.Err.Printf("Importing %T configuration for %s. Run with -skip-tools to skip.", i, pr)
 			}
-			return i.DeriveRootManifestAndLock(dir, pr)
+			return i.Import(dir, pr)
 		}
 	}
 
 	return nil, nil, nil
 }
 
+func (a importAnalyzer) DeriveRootManifestAndLock(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
+	return a.importManifestAndLock(dir, pr)
+}
+
 func (a importAnalyzer) DeriveManifestAndLock(dir string, pr gps.ProjectRoot) (gps.Manifest, gps.Lock, error) {
 	// Ignore other tools if we find dep configuration
 	var depAnalyzer dep.Analyzer
 	if depAnalyzer.HasConfig(dir) {
 		return depAnalyzer.DeriveManifestAndLock(dir, pr)
 	}
 
-	var importers []importer = []importer{newGlideImporter(a.loggers, a.sm)}
-	for _, i := range importers {
-		if i.HasConfig(dir) {
-			tool, _ := i.Info()
-			if a.loggers.Verbose {
-				a.loggers.Err.Printf("Importing %s configuration for %s. Run with -skip-tools to skip.", tool, pr)
-			}
-			return i.DeriveManifestAndLock(dir, pr)
-		}
+	// The assignment back to an interface prevents interface-based nil checks from failing later
+	var manifest gps.Manifest
+	var lock gps.Lock
+	im, il, err := a.importManifestAndLock(dir, pr)
+	if im != nil {
+		manifest = im
 	}
-
-	return nil, nil, nil
+	if il != nil {
+		lock = il
+	}
+	return manifest, lock, err
 }
 
-func (a importAnalyzer) PostSolveShenanigans(m *dep.Manifest, l *dep.Lock) {
+func (a importAnalyzer) FinalizeManifestAndLock(m *dep.Manifest, l *dep.Lock) {
 	// do nothing
 }
+
+func (a importAnalyzer) Info() (string, int) {
+	// TODO(carolynvs): do not merge until this is set to something unique.
+	// I'm not changing it now because that will cause the memo to change in tests
+	// which I'll deal with and update later
+	return "dep", 1
+}

cmd/dep/init.go

@@ -133,7 +133,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 				newGopathAnalyzer(ctx, pkgT, cpr, sm),
 				newImportAnalyzer(ctx.Loggers, sm),
 			}}
-		analyzer = importAnalyzer{ctx.Loggers, sm}
+		analyzer = newImportAnalyzer(ctx.Loggers, sm)
 	}
 
 	// Generate a manifest and lock for the root project
@@ -172,7 +172,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	}
 	l = dep.LockFromInterface(soln)
 
-	rootAnalyzer.PostSolveShenanigans(m, l)
+	rootAnalyzer.FinalizeManifestAndLock(m, l)
 
 	// Run gps.Prepare with appropriate constraint solutions from solve run
 	// to generate the final lock memo.

cmd/dep/rootProjectAnalyzer.go

@@ -9,13 +9,12 @@ import (
 	"github.com/golang/dep/internal/gps"
 )
 
-// rootProjectAnalyzer is responsible for generating a root manifest and lock for
+// rootProjectAnalyzer is responsible for generating a manifest and lock for
 // a root project.
 type rootProjectAnalyzer interface {
-	// Perform analysis of the filesystem tree rooted at path, with the
-	// root import path importRoot, to determine the project's constraints, as
-	// indicated by a Manifest and Lock.
-	DeriveRootManifestAndLock(path string, n gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error)
+	// Generate an initial manifest and lock for the root project.
+	DeriveRootManifestAndLock(path string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error)
 
-	PostSolveShenanigans(*dep.Manifest, *dep.Lock)
+	// Apply any final changes to the manifest and lock after the solver has been run.
+	FinalizeManifestAndLock(*dep.Manifest, *dep.Lock)
 }