remove!

dep_test.go

@@ -171,7 +171,8 @@ func (tg *testgoData) doRun(args []string) error {
 	} else {
 		prog = filepath.Join(tg.wd, "testdep"+exeSuffix)
 	}
-	cmd := exec.Command(prog, append(args, "-v")...)
+	args = append(args[:1], append([]string{"-v"}, args[1:]...)...)
+	cmd := exec.Command(prog, args...)
 	tg.stdout.Reset()
 	tg.stderr.Reset()
 	cmd.Stdout = &tg.stdout

ensure.go

@@ -176,11 +176,7 @@ func (cmd *ensureCommand) Run(args []string) error {
 		return errors.New(buf.String())
 	}
 
-	params := gps.SolveParameters{
-		RootDir:  p.absroot,
-		Manifest: p.m,
-		Lock:     p.l,
-	}
+	params := p.makeParams()
 	if *verbose {
 		params.Trace = true
 		params.TraceLogger = log.New(os.Stderr, "", 0)

main.go

@@ -181,6 +181,24 @@ type project struct {
 	l          *lock
 }
 
+// makeParams is a simple helper to create a gps.SolveParameters without setting
+// any nils incorrectly.
+func (p *project) makeParams() gps.SolveParameters {
+	params := gps.SolveParameters{
+		RootDir: p.absroot,
+	}
+
+	if p.m != nil {
+		params.Manifest = p.m
+	}
+
+	if p.l != nil {
+		params.Lock = p.l
+	}
+
+	return params
+}
+
 func logf(format string, args ...interface{}) {
 	// TODO: something else?
 	fmt.Fprintf(os.Stderr, "dep: "+format+"\n", args...)

remove.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/sdboyer/gps"
@@ -62,35 +63,110 @@ func (cmd *removeCommand) Run(args []string) error {
 		return errors.Wrap(err, "gps.ListPackages")
 	}
 
-	// get the list of packages
-	pd, err := getProjectData(pkgT, cpr, sm)
-	if err != nil {
-		return err
-	}
+	// TODO this will end up ignoring internal pkgs with errs (and any other
+	// internal pkgs that import them), which is not what we want for this mode.
+	// A new callback, or a new param on this one, will be introduced to gps
+	// soon, and we'll want to use that when it arrives.
+	//reachlist := pkgT.ExternalReach(true, true).ListExternalImports()
+	reachmap := pkgT.ExternalReach(true, true, nil)
 
-	for _, arg := range args {
-		/*
-		 * - Remove package from manifest
-		 *	- if the package IS NOT being used, solving should do what we want
-		 *	- if the package IS being used:
-		 *		- Desired behavior: stop and tell the user, unless --force
-		 *		- Actual solver behavior: ?
-		 */
-
-		if _, found := pd.dependencies[gps.ProjectRoot(arg)]; found {
-			//TODO: Tell the user where it is in use?
-			return fmt.Errorf("not removing '%s' because it is in use", arg)
+	if cmd.unused {
+		if len(args) > 0 {
+			return fmt.Errorf("remove takes no arguments when running with -unused")
+		}
+
+		reachlist := reachmap.ListExternalImports()
+
+		// warm the cache in parallel, in case any paths require go get metadata
+		// discovery
+		for _, im := range reachlist {
+			go sm.DeduceProjectRoot(im)
+		}
+
+		otherroots := make(map[gps.ProjectRoot]bool)
+		for _, im := range reachlist {
+			if isStdLib(im) {
+				continue
+			}
+			pr, err := sm.DeduceProjectRoot(im)
+			if err != nil {
+				// not being able to detect the root for an import path that's
+				// actually in the import list is a deeper problem. However,
+				// it's not our direct concern here, so we just warn.
+				logf("could not infer root for %q", pr)
+				continue
+			}
+			otherroots[pr] = true
+		}
+
+		var rm []gps.ProjectRoot
+		for pr, _ := range p.m.Dependencies {
+			if _, has := otherroots[pr]; !has {
+				delete(p.m.Dependencies, pr)
+				rm = append(rm, pr)
+			}
+		}
+
+		if len(rm) == 0 {
+			logf("nothing to do")
+			return nil
+		}
+	} else {
+		// warm the cache in parallel, in case any paths require go get metadata
+		// discovery
+		for _, arg := range args {
+			go sm.DeduceProjectRoot(arg)
 		}
-		delete(p.m.Dependencies, gps.ProjectRoot(arg))
-	}
 
-	params := gps.SolveParameters{
-		RootDir:         p.absroot,
-		RootPackageTree: pkgT,
-		Manifest:        p.m,
-		Lock:            p.l,
+		for _, arg := range args {
+			pr, err := sm.DeduceProjectRoot(arg)
+			if err != nil {
+				// couldn't detect the project root for this string -
+				// a non-valid project root was provided
+				return errors.Wrap(err, "gps.DeduceProjectRoot")
+			}
+			if string(pr) != arg {
+				// don't be magical with subpaths, otherwise we muddy the waters
+				// between project roots and import paths
+				return fmt.Errorf("%q is not a project root, but %q is - is that what you want to remove?", arg, pr)
+			}
+
+			/*
+			* - Remove package from manifest
+			*	- if the package IS NOT being used, solving should do what we want
+			*	- if the package IS being used:
+			*		- Desired behavior: stop and tell the user, unless --force
+			*		- Actual solver behavior: ?
+			 */
+			var pkgimport []string
+			for pkg, imports := range reachmap {
+				for _, im := range imports {
+					if hasImportPathPrefix(im, arg) {
+						pkgimport = append(pkgimport, pkg)
+						break
+					}
+				}
+			}
+
+			if _, indeps := p.m.Dependencies[gps.ProjectRoot(arg)]; !indeps {
+				return fmt.Errorf("%q is not present in the manifest, cannot remove it", arg)
+			}
+
+			if len(pkgimport) > 0 && !cmd.force {
+				if len(pkgimport) == 1 {
+					return fmt.Errorf("not removing %q because it is imported by %q (pass -force to override)", arg, pkgimport[0])
+				} else {
+					return fmt.Errorf("not removing %q because it is imported by:\n\t%s (pass -force to override)", arg, strings.Join(pkgimport, "\n\t"))
+				}
+			}
+
+			delete(p.m.Dependencies, gps.ProjectRoot(arg))
+		}
 	}
 
+	params := p.makeParams()
+	params.RootPackageTree = pkgT
+
 	if *verbose {
 		params.Trace = true
 		params.TraceLogger = log.New(os.Stderr, "", 0)

remove_test.go

@@ -0,0 +1,158 @@
+// Copyright 2016 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 "testing"
+
+func TestRemove(t *testing.T) {
+	needsExternalNetwork(t)
+	needsGit(t)
+
+	tg := testgo(t)
+	defer tg.cleanup()
+
+	tg.tempDir("src")
+	tg.setenv("GOPATH", tg.path("."))
+
+	importPaths := map[string]string{
+		"github.com/pkg/errors":      "v0.8.0",                                   // semver
+		"github.com/Sirupsen/logrus": "42b84f9ec624953ecbf81a94feccb3f5935c5edf", // random sha
+	}
+
+	// checkout the specified revisions
+	for ip, rev := range importPaths {
+		tg.runGo("get", ip)
+		repoDir := tg.path("src/" + ip)
+		tg.runGit(repoDir, "checkout", rev)
+	}
+
+	// Build a fake consumer of these packages.
+	const root = "github.com/golang/notexist"
+	m := `package main
+
+import (
+	"github.com/Sirupsen/logrus"
+	"github.com/pkg/errors"
+)
+
+func main() {
+	err := nil
+	if err != nil {
+		errors.Wrap(err, "thing")
+	}
+	logrus.Info("whatev")
+}`
+
+	tg.tempFile("src/"+root+"/thing.go", m)
+	origm := `{
+    "dependencies": {
+        "github.com/not/used": {
+            "version": "2.0.0"
+        },
+        "github.com/Sirupsen/logrus": {
+            "revision": "42b84f9ec624953ecbf81a94feccb3f5935c5edf"
+        },
+        "github.com/pkg/errors": {
+            "version": ">=0.8.0, <1.0.0"
+        }
+    }
+}
+`
+	expectedManifest := `{
+    "dependencies": {
+        "github.com/Sirupsen/logrus": {
+            "revision": "42b84f9ec624953ecbf81a94feccb3f5935c5edf"
+        },
+        "github.com/pkg/errors": {
+            "version": ">=0.8.0, <1.0.0"
+        }
+    }
+}
+`
+
+	tg.tempFile("src/"+root+"/manifest.json", origm)
+
+	tg.cd(tg.path("src/" + root))
+	tg.run("remove", "-unused")
+
+	manifest := tg.readManifest()
+	if manifest != expectedManifest {
+		t.Fatalf("expected %s, got %s", expectedManifest, manifest)
+	}
+
+	tg.tempFile("src/"+root+"/manifest.json", origm)
+	tg.run("remove", "github.com/not/used")
+
+	manifest = tg.readManifest()
+	if manifest != expectedManifest {
+		t.Fatalf("expected %s, got %s", expectedManifest, manifest)
+	}
+
+	if err := tg.doRun([]string{"remove", "-unused", "github.com/not/used"}); err == nil {
+		t.Fatal("rm with both -unused and arg should have failed")
+	}
+
+	if err := tg.doRun([]string{"remove", "github.com/not/present"}); err == nil {
+		t.Fatal("rm with arg not in manifest should have failed")
+	}
+
+	if err := tg.doRun([]string{"remove", "github.com/not/used", "github.com/not/present"}); err == nil {
+		t.Fatal("rm with one arg not in manifest should have failed")
+	}
+
+	if err := tg.doRun([]string{"remove", "github.com/pkg/errors"}); err == nil {
+		t.Fatal("rm of arg in manifest and imports should have failed without -force")
+	}
+
+	tg.tempFile("src/"+root+"/manifest.json", origm)
+	tg.run("remove", "-force", "github.com/pkg/errors", "github.com/not/used")
+
+	manifest = tg.readManifest()
+	if manifest != `{
+    "dependencies": {
+        "github.com/Sirupsen/logrus": {
+            "revision": "42b84f9ec624953ecbf81a94feccb3f5935c5edf"
+        }
+    }
+}
+` {
+		t.Fatalf("expected %s, got %s", expectedManifest, manifest)
+	}
+
+	sysCommit, err := getRepoLatestCommit("golang/sys")
+	tg.must(err)
+	expectedLock := `{
+    "projects": [
+        {
+            "name": "github.com/Sirupsen/logrus",
+            "revision": "42b84f9ec624953ecbf81a94feccb3f5935c5edf",
+            "packages": [
+                "."
+            ]
+        },
+        {
+            "name": "github.com/pkg/errors",
+            "version": "v0.8.0",
+            "revision": "645ef00459ed84a119197bfb8d8205042c6df63d",
+            "packages": [
+                "."
+            ]
+        },
+        {
+            "name": "golang.org/x/sys",
+            "branch": "master",
+            "revision": "` + sysCommit + `",
+            "packages": [
+                "unix"
+            ]
+        }
+    ]
+}
+`
+	lock := wipeMemo(tg.readLock())
+	if lock != expectedLock {
+		t.Fatalf("expected %s, got %s", expectedLock, lock)
+	}
+}