Merge branch 'master' into add-gps

.travis.yml

@@ -17,7 +17,7 @@ install:
   - echo "This is an override of the default install deps step in travis."
 before_script:
   - PKGS=$(go list ./... | grep -v /vendor/)
-  - go get -v honnef.co/go/tools/cmd/staticcheck
+  - go get -v honnef.co/go/tools/cmd/{gosimple,staticcheck}
 script:
   - go build -v ./cmd/dep
   - go vet $PKGS

CONTRIBUTING.md

@@ -8,6 +8,8 @@ Keep an eye on the [Roadmap](https://github.com/golang/dep/wiki/Roadmap) for a s
 
 ## Filing issues
 
+Please check the existing issues and [FAQ](FAQ.md) to see if your feedback has already been reported.
+
 When [filing an issue](https://github.com/golang/dep/issues/new), make sure to answer these five questions:
 
 1. What version of Go are you using (`go version`)?

FAQ.md

@@ -0,0 +1,123 @@
+# FAQ
+
+_The first rule of FAQ is don't bikeshed the FAQ, leave that for
+[Create structure for managing docs](https://github.com/golang/dep/issues/331)._
+
+Please contribute to the FAQ! Found an explanation in an issue or pull request helpful?
+Summarize the question and quote the reply, linking back to the original comment.
+
+* [What is a direct or transitive dependency?](#what-is-a-direct-or-transitive-dependency)
+* [Should I commit my vendor directory?](#should-i-commit-my-vendor-directory)
+* [Why is it `dep ensure` instead of `dep install`?](#why-is-it-dep-ensure-instead-of-dep-install)
+* [Does `dep` replace `go get`?](#does-dep-replace-go-get)
+* [Why is `dep` ignoring a version constraint in the manifest?](#why-is-dep-ignoring-a-version-constraint-in-the-manifest)
+* [How do I constrain a transitive dependency's version?](#how-do-i-constrain-a-transitive-dependencys-version)
+* [`dep` deleted my files in the vendor directory!](#dep-deleted-my-files-in-the-vendor-directory)
+* [Can I put the manifest and lock in the vendor directory?](#can-i-put-the-manifest-and-lock-in-the-vendor-directory)
+* [Why did dep use a different revision for package X instead of the revision in the lock file?](#why-did-dep-use-a-different-revision-for-package-x-instead-of-the-revision-in-the-lock-file)
+
+## What is a direct or transitive dependency?
+* Direct dependencies appear in at least one import statement from your project - are dependencies that are imported by your project.
+* Transitive dependencies are the dependencies of your dependencies. Necessary to compile but are not directly used by your code.
+
+## Should I commit my vendor directory?
+
+Committing the vendor directory is totally up to you. There is no general advice that applies in all cases.
+
+**Pros**: it's the only way to get truly reproducible builds, as it guards against upstream renames and deletes; and you don't need an extra `dep ensure` step on fresh clones to build your repo.
+
+**Cons**: your repo will be bigger, potentially a lot bigger; and PR diffs are more annoying.
+
+## Why is it `dep ensure` instead of `dep install`?
+
+> Yeah, we went round and round on names. [A lot](https://gist.github.com/jessfraz/315db91b272441f510e81e449f675a8b).
+>
+> The idea of "ensure" is roughly, "ensure that all my local states - code tree, manifest, lock, and vendor - are in sync with each other." When arguments are passed, it becomes "ensure this argument is satisfied, along with synchronization between all my local states."
+>
+> We opted for this approach because we came to the conclusion that allowing the tool to perform partial work/exit in intermediate states ended up creating a tool that had more commands, had far more possible valid exit and input states, and was generally full of footguns. In this approach, the user has most of the same ultimate control, but exercises it differently (by modifying the code/manifest and re-running dep ensure).
+-[@sdboyer in #371](https://github.com/golang/dep/issues/371#issuecomment-293246832)
+
+## Does `dep` replace `go get`?
+
+No, `dep` is an experiment and is still in its infancy. Depending on how this
+experiment goes, it may be considered for inclusion in the go project in some form
+or another in the future but that is not guaranteed.
+
+Here are some suggestions for when you could use `dep` or `go get`:
+> I would say that dep doesn't replace go get, but they both can do similar things. Here's how I use them:
+>
+> `go get`: I want to download the source code for a go project so that I can work on it myself, or to install a tool. This clones the repo under GOPATH for all to use.
+>
+> `dep ensure`: I have imported a new dependency in my code and want to download the dependency so I can start using it. My workflow is "add the import to the code, and then run dep ensure so that the manifest/lock/vendor are updated". This clones the repo under my project's vendor directory, and remembers the revision used so that everyone who works on my project is guaranteed to be using the same version of dependencies.
+-[@carolynvs in #376](https://github.com/golang/dep/issues/376#issuecomment-293964655)
+
+> The long term vision is a sane, overall-consistent go tool. My general take is that `go get`
+> is for people consuming Go code, and dep-family commands are for people developing it.
+-[@sdboyer in #376](https://github.com/golang/dep/issues/376#issuecomment-294045873)
+
+## Why is `dep` ignoring a version constraint in the manifest?
+Only your project's directly imported dependencies are affected by a `dependencies` entry
+in the manifest. Transitive dependencies are unaffected.
+
+Use an `overrides` entry for transitive dependencies.
+
+## How do I constrain a transitive dependency's version?
+First, if you're wondering about this because you're trying to keep the version
+of the transitive dependency from changing, then you're working against `dep`'s
+design. The lock file, `Gopkg.lock`, will keep the selected version of the
+transitive dependency stable, unless you explicitly request an upgrade or it's
+impossible to find a solution without changing that version.
+
+If that isn't your use case and you still need to constrain a transitive
+dependency, you have a couple of options:
+
+1. Make the transitive dependency a direct one, either with a dummy import or an entry in the `required` list in `Gopkg.toml`.
+2. Use an override.
+
+Overrides are a sledgehammer, and should only be used as a last resort. While
+dependencies and overrides are declared in the same way in `Gopkg.toml`, they
+behave differently:
+
+* Dependencies:
+   1. Can be declared by any project's manifest, yours or a dependency
+   2. Apply only to direct dependencies of the project declaring the constraint
+   3. Must not conflict with the `dependencies` declared in any other project's manifest
+* Overrides:
+   1. Are only utilized from the current/your project's manifest
+   2. Apply globally, to direct and transitive dependencies
+   3. Supersede constraints declared in all manifests, yours or a dependency's
+
+Overrides are also discussed with some visuals in [the gps docs](https://github.com/sdboyer/gps/wiki/gps-for-Implementors#overrides).
+
+## `dep` deleted my files in the vendor directory!
+First, sorry! 😞 We hope you were able to recover your files...
+
+> dep assumes complete control of vendor/, and may indeed blow things away if it feels like it.
+-[@peterbourgon in #206](https://github.com/golang/dep/issues/206#issuecomment-277139419)
+
+## Can I put the manifest and lock in the vendor directory?
+No.
+
+> Placing these files inside `vendor/` would concretely bind us to `vendor/` in the long term.
+> We prefer to treat the `vendor/` as an implementation detail.
+-[@sdboyer on go package management list](https://groups.google.com/d/msg/go-package-management/et1qFUjrkP4/LQFCHP4WBQAJ)
+
+## Why did dep use a different revision for package X instead of the revision in the lock file?
+Sometimes the revision specified in the lock file is no longer valid. There are a few
+ways this can occur:
+
+* When you generated the lock file, you had an unpushed commit in your local copy of package X's repository in your GOPATH. (This case will be going away soon)
+* After generating the lock file, new commits were force pushed to package X's repository, causing the commit revision in your lock file to no longer exist.
+
+To troubleshoot, you can revert dep's changes to your lock, and then run `dep ensure -v -n`.
+This retries the command in dry-run mode with verbose logs enabled. Check the output
+for a warning like the one below, indicating that a commit in the lock is no longer valid.
+
+```
+Unable to update checked out version: fatal: reference is not a tree: 4dfc6a8a7e15229398c0a018b6d7a078cccae9c8
+```
+
+> The lock file represents a set of precise, typically immutable versions for the entire transitive closure of dependencies for a project. But "the project" can be, and is, decomposed into just a bunch of arguments to an algorithm. When those inputs change, the lock may need to change as well.
+>
+> Under most circumstances, if those arguments don't change, then the lock remains fine and correct. You've hit one one of the few cases where that guarantee doesn't apply. The fact that you ran dep ensure and it DID a solve is a product of some arguments changing; that solving failed because this particular commit had become stale is a separate problem.
+-[@sdboyer in #405](https://github.com/golang/dep/issues/405#issuecomment-295998489)

README.md

@@ -21,6 +21,7 @@ Please see below for feedback and contribution guidelines.
   - [User Stories](https://docs.google.com/document/d/1wT8e8wBHMrSRHY4UF_60GCgyWGqvYye4THvaDARPySs/edit)
   - [Features](https://docs.google.com/document/d/1JNP6DgSK-c6KqveIhQk-n_HAw3hsZkL-okoleM43NgA/edit)
   - [Design Space](https://docs.google.com/document/d/1TpQlQYovCoX9FkpgsoxzdvZplghudHAiQOame30A-v8/edit)
+- [Frequently Asked Questions](FAQ.md)
 
 ## Usage
 
@@ -55,7 +56,7 @@ Feedback is greatly appreciated.
 At this stage, the maintainers are most interested in feedback centered on the user experience (UX) of the tool.
 Do you have workflows that the tool supports well, or doesn't support at all?
 Do any of the commands have surprising effects, output, or results?
-Please check the existing issues to see if your feedback has already been reported.
+Please check the existing issues and [FAQ](FAQ.md) to see if your feedback has already been reported.
 If not, please file an issue, describing what you did or wanted to do, what you expected to happen, and what actually happened.
 
 ## Contributing

cmd/dep/ensure.go

@@ -16,6 +16,7 @@ import (
 	"strings"
 
 	"github.com/golang/dep"
+	"github.com/golang/dep/internal"
 	"github.com/pkg/errors"
 	"github.com/golang/dep/gps"
 	"github.com/golang/dep/gps/pkgtree"
@@ -205,7 +206,7 @@ func applyEnsureArgs(args []string, overrides stringSlice, p *dep.Project, sm *g
 			// TODO(sdboyer): for this case - or just in general - do we want to
 			// add project args to the requires list temporarily for this run?
 			if _, has := p.Manifest.Dependencies[pc.Ident.ProjectRoot]; !has {
-				logf("No constraint or alternate source specified for %q, omitting from manifest", pc.Ident.ProjectRoot)
+				internal.Logf("No constraint or alternate source specified for %q, omitting from manifest", pc.Ident.ProjectRoot)
 			}
 			// If it's already in the manifest, no need to log
 			continue

cmd/dep/init.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 
 	"github.com/golang/dep"
+	"github.com/golang/dep/internal"
 	"github.com/pkg/errors"
 	"github.com/golang/dep/gps"
 	"github.com/golang/dep/gps/pkgtree"
@@ -89,12 +90,12 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	if err != nil {
 		return errors.Wrap(err, "determineProjectRoot")
 	}
-	vlogf("Finding dependencies for %q...", cpr)
+	internal.Vlogf("Finding dependencies for %q...", cpr)
 	pkgT, err := pkgtree.ListPackages(root, cpr)
 	if err != nil {
 		return errors.Wrap(err, "gps.ListPackages")
 	}
-	vlogf("Found %d dependencies.", len(pkgT.Packages))
+	internal.Vlogf("Found %d dependencies.", len(pkgT.Packages))
 	sm, err := ctx.SourceManager()
 	if err != nil {
 		return errors.Wrap(err, "getSourceManager")
@@ -135,7 +136,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	}
 
 	if len(pd.notondisk) > 0 {
-		vlogf("Solving...")
+		internal.Vlogf("Solving...")
 		params := gps.SolveParameters{
 			RootDir:         root,
 			RootPackageTree: pkgT,
@@ -161,7 +162,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 		l = dep.LockFromInterface(soln)
 	}
 
-	vlogf("Writing manifest and lock files.")
+	internal.Vlogf("Writing manifest and lock files.")
 
 	var sw dep.SafeWriter
 	sw.Prepare(m, nil, l, dep.VendorAlways)
@@ -236,7 +237,7 @@ func getProjectData(ctx *dep.Ctx, pkgT pkgtree.PackageTree, cpr string, sm *gps.
 		return projectData{}, nil
 	}
 
-	vlogf("Building dependency graph...")
+	internal.Vlogf("Building dependency graph...")
 	// Exclude stdlib imports from the list returned from Flatten().
 	const omitStdlib = false
 	for _, ip := range rm.Flatten(omitStdlib) {
@@ -252,13 +253,13 @@ func getProjectData(ctx *dep.Ctx, pkgT pkgtree.PackageTree, cpr string, sm *gps.
 		}
 		go syncDep(pr, sm)
 
-		vlogf("Found import of %q, analyzing...", ip)
+		internal.Vlogf("Found import of %q, analyzing...", ip)
 
 		dependencies[pr] = []string{ip}
 		v, err := ctx.VersionInWorkspace(pr)
 		if err != nil {
 			notondisk[pr] = true
-			vlogf("Could not determine version for %q, omitting from generated manifest", pr)
+			internal.Vlogf("Could not determine version for %q, omitting from generated manifest", pr)
 			continue
 		}
 
@@ -275,7 +276,7 @@ func getProjectData(ctx *dep.Ctx, pkgT pkgtree.PackageTree, cpr string, sm *gps.
 		constraints[pr] = pp
 	}
 
-	vlogf("Analyzing transitive imports...")
+	internal.Vlogf("Analyzing transitive imports...")
 	// Explore the packages we've found for transitive deps, either
 	// completing the lock or identifying (more) missing projects that we'll
 	// need to ask gps to solve for us.
@@ -294,7 +295,7 @@ func getProjectData(ctx *dep.Ctx, pkgT pkgtree.PackageTree, cpr string, sm *gps.
 	dft = func(pkg string) error {
 		switch colors[pkg] {
 		case white:
-			vlogf("Analyzing %q...", pkg)
+			internal.Vlogf("Analyzing %q...", pkg)
 			colors[pkg] = grey
 
 			pr, err := sm.DeduceProjectRoot(pkg)

cmd/dep/main.go

@@ -14,6 +14,7 @@ import (
 	"text/tabwriter"
 
 	"github.com/golang/dep"
+	"github.com/golang/dep/internal"
 )
 
 var (
@@ -114,6 +115,8 @@ func main() {
 				os.Exit(1)
 			}
 
+			internal.Verbose = *verbose
+
 			// Set up the dep context.
 			ctx, err := dep.NewContext()
 			if err != nil {
@@ -167,7 +170,7 @@ func resetUsage(fs *flag.FlagSet, name, args, longHelp string) {
 	}
 }
 
-// parseArgs determines the name of the dep command and wether the user asked for
+// parseArgs determines the name of the dep command and whether the user asked for
 // help to be printed.
 func parseArgs(args []string) (cmdName string, printCmdUsage bool, exit bool) {
 	isHelpArg := func() bool {
@@ -192,15 +195,3 @@ func parseArgs(args []string) (cmdName string, printCmdUsage bool, exit bool) {
 	}
 	return cmdName, printCmdUsage, exit
 }
-
-func logf(format string, args ...interface{}) {
-	// TODO: something else?
-	fmt.Fprintf(os.Stderr, "dep: "+format+"\n", args...)
-}
-
-func vlogf(format string, args ...interface{}) {
-	if !*verbose {
-		return
-	}
-	logf(format, args...)
-}

cmd/dep/prune.go

@@ -59,12 +59,9 @@ func (cmd *pruneCommand) Run(ctx *dep.Ctx, args []string) error {
 	}
 
 	// Set up a solver in order to check the InputHash.
-	params := gps.SolveParameters{
-		RootDir:         p.AbsRoot,
-		RootPackageTree: ptree,
-		Manifest:        p.Manifest,
-		// Locks aren't a part of the input hash check, so we can omit it.
-	}
+	params := p.MakeParams()
+	params.RootPackageTree = ptree
+
 	if *verbose {
 		params.Trace = true
 		params.TraceLogger = log.New(os.Stderr, "", 0)

cmd/dep/remove.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/golang/dep"
+	"github.com/golang/dep/internal"
 	"github.com/pkg/errors"
 	"github.com/golang/dep/gps"
 	"github.com/golang/dep/gps/pkgtree"
@@ -91,7 +92,7 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, args []string) error {
 				// 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)
+				internal.Logf("could not infer root for %q", pr)
 				continue
 			}
 			otherroots[pr] = true
@@ -106,7 +107,7 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, args []string) error {
 		}
 
 		if len(rm) == 0 {
-			logf("nothing to do")
+			internal.Logf("nothing to do")
 			return nil
 		}
 	} else {

cmd/dep/status.go

@@ -364,23 +364,43 @@ func runStatusAll(out outputter, p *dep.Project, sm *gps.SourceMgr) error {
 	//
 	// It's possible for digests to not match, but still have a correct
 	// lock.
-	out.MissingHeader()
-
 	rm, _ := ptree.ToReachMap(true, true, false, nil)
 
 	external := rm.Flatten(false)
-	roots := make(map[gps.ProjectRoot][]string)
-	var errs []string
+	roots := make(map[gps.ProjectRoot][]string, len(external))
+
+	type fail struct {
+		ex  string
+		err error
+	}
+	var errs []fail
 	for _, e := range external {
 		root, err := sm.DeduceProjectRoot(e)
 		if err != nil {
-			errs = append(errs, string(root))
+			errs = append(errs, fail{
+				ex:  e,
+				err: err,
+			})
 			continue
 		}
 
 		roots[root] = append(roots[root], e)
 	}
 
+	if len(errs) != 0 {
+		// TODO this is just a fix quick so staticcheck doesn't complain.
+		// Visually reconciling failure to deduce project roots with the rest of
+		// the mismatch output is a larger problem.
+		fmt.Fprintf(os.Stderr, "Failed to deduce project roots for import paths:\n")
+		for _, fail := range errs {
+			fmt.Fprintf(os.Stderr, "\t%s: %s\n", fail.ex, fail.err.Error())
+		}
+
+		return errors.New("address issues with undeducable import paths to get more status information.")
+	}
+
+	out.MissingHeader()
+
 outer:
 	for root, pkgs := range roots {
 		// TODO also handle the case where the project is present, but there