Dep: `dep ensure -add` should add [[constraint]] for all unconstrained direct dependencies

[shabbyrobe]

What version of dep are you using (dep version)?

v0.3.2-72-g3670e70

What dep command did you run?

dep ensure -add

What did you expect to see?

As per the docs, add should "add new dependencies, or populate Gopkg.toml with constraints for existing dependencies (default: false)" but I see no way to invoke -add to accomplish the latter behaviour.

What did you see instead?

must specify at least one project or package to -add

[carolynvs]

Thank you for bringing this up! I talked it over with the other maintainers and here's what we need:

dep ensure -add should inspect the lock file and attempt to generate sane constraints for any projects which don't already have constraints/overrides/requires in the manifest.

Example:

Gopkg.toml

[[constraint]]
  name = "github.com/example/A"
  version = "1.0.0"

Gopkg.lock

[[project]]
  name = "github.com/example/A"
  revision = "abc123"
  version = "1.0.0"

[[project]]
  name = "github.com/example/B"
  revision = "def456"
  version = "2.1.0"

[[project]]
  name = "github.com/example/C"
  revision = "ghi987" #this doesn't point to any tagged revision

After running dep ensure --add with the above configuration, the manifest would be modified and look like this:

[[constraint]]
  name = "github.com/example/A"
  version = "1.0.0"

[[constraint]]
  name = "github.com/example/B"
  version = "2.1.0"

The logic to infer a constraint from a value is already implemented, we use it during import:

https://github.com/golang/dep/blob/44d8995a35b45ae0e55ed4c1a8dc918562923efb/internal/importers/base/importer.go#L197-L204

So when someone specifies dep ensure --add without any additional values, we should iterate over the projects in the existing lock, identify projects that do not already have a constraint/override/requires in the manifest, and use InferConstraint to try to come up with one (it could fail, in which case skip and move to the next project). Then add it to the new constraints to the manifest and write out it and the lock again.

[sdboyer]

@shabbyrobe we're hijacking your issue a bit (editing the title and such) to try to get this fixed up 😄

@carolynvs' explanation looks great, just a couple additional considerations:

So when someone specifies dep ensure --add without any additional values, we should iterate over the projects in the existing lock

we also have to accommodate the possibility that there may be new imports (or requireds) in the project that are not in the Gopkg.lock yet, and make sure that we add constraints for those, too, after the solver has selected a version for them.

identify projects that do not already have a constraint/override

it's only the presence of a [[constraint]] that we care about here - if there's an [[override]] but no [[constraint]], we should still add a [[constraint]].

[carolynvs]

it's only the presence of a [[constraint]] that we care about here - if there's an [[override]] but no [[constraint]], we should still add a [[constraint]]

@sdboyer Can you confirm that this results in a manifest that makes sense? I wasn't quite sure what the solver would do with the example below, which is why I originally wrote out what I did.

Before

[[overrides]]
  name = "github.com/example/A"
  branch = "2.x"

requires = ["github.com/example/B"]

After

[[overrides]]
  name = "github.com/example/A"
  version = "~1.0.0"

requires = ["github.com/example/B"]

[[constraints]]
  name = "github.com/example/A"
  version = "1.0.5"

[[constraints]]
  name = "github.com/example/B"
  version = "2.3.0"

[shabbyrobe]

Hijack away! I was afraid that the response would be "oh, let's remove that from the documentation then". This is exactly what I'd hoped would happen. I'm quite happy to help if you've got something specific I can do, but I haven't worked on dep's code before so my usefulness may be limited.

[rogpeppe]

dep ensure -add should inspect the lock file and attempt to generate sane constraints for any projects which don't already have constraints/overrides/requires in the manifest.

I'm not sure that's quite right. The dependencies might not be in the lock file yet. I think it should look both in the lock file and at the packages that are actually imported, and generate additional Gopkg.lock and Gopkg.toml entries as required.

[sdboyer]

@carolynvs yes, that looks right - assuming github.com/example/A is in the imports.

@rogpeppe yep, agreed