allow updating dependencies from multiple files

allow updating dependencies from multiple files

When there's a conflict, it prints a warning and
tries to choose the dependency from the earliest-specified
file or latest revision depending on command line flags.

Also changed the behaviour of the -n flag
so that it does not overlap so much with -x
and so that we don't see so much noise when experimenting
with godeps -u commands.

https://code.launchpad.net/~rogpeppe/godeps/002-multi-dependency-update/+merge/247684

(do not edit description out of merge proposal)

[rog, 2015-01-27 10:36:57]

Please take a look.

[mattyw, 2015-01-27 10:51:26]

Some small comments from me

https://codereview.appspot.com/197110043/diff/1/godeps.go
File godeps.go (right):

https://codereview.appspot.com/197110043/diff/1/godeps.go#newcode43
godeps.go:43: godeps -U [flags] file...
There should be more information about how it works in the usage message.

e.g.

-U takes multiple dependency files, any common dependcies will be updated to the
latest revision

https://codereview.appspot.com/197110043/diff/1/godeps.go#newcode776
godeps.go:776: fmt.Printf("warning: cannot parse revision number %s: %v",
i0.revno, err0)
So if we can't parse a revision id we always assume that i0 is the latest. What
situations is this likely to fail? I can't help but think we should return an
error at this point. We then have the choice of stopping the whole thing - or
skipping this project and printing a warning. It feels a bit consistent
otherwise.
If we left it like this the usage would be:

-U takes multiple dependency files, any common dependcies will be updated to the
latest revision. If a particular revid can't be parsed we assume the first revid
we find is the one we want. This means the list of dependency files can be
effectively ordered from most important -> least important

[rog, 2015-01-27 17:43:00]

Please take a look.

[rog, 2015-01-27 17:59:56]

Please take a look.

https://codereview.appspot.com/197110043/diff/1/godeps.go
File godeps.go (right):

https://codereview.appspot.com/197110043/diff/1/godeps.go#newcode43
godeps.go:43: godeps -U [flags] file...
On 2015/01/27 10:51:26, mattyw wrote:
> There should be more information about how it works in the usage message.
> 
> e.g.
> 
> -U takes multiple dependency files, any common dependcies will be updated to
the
> latest revision

Done.

https://codereview.appspot.com/197110043/diff/1/godeps.go#newcode776
godeps.go:776: fmt.Printf("warning: cannot parse revision number %s: %v",
i0.revno, err0)
On 2015/01/27 10:51:26, mattyw wrote:
> So if we can't parse a revision id we always assume that i0 is the latest.

That's not quite right. If we can't parse a revision id, we assume that
the one that parses OK is the latest. If we can't parse either of them,
we always return false.

> What
> situations is this likely to fail? I can't help but think we should return an
> error at this point. We then have the choice of stopping the whole thing - or
> skipping this project and printing a warning. It feels a bit consistent
> otherwise.
> If we left it like this the usage would be:
> 
> -U takes multiple dependency files, any common dependcies will be updated to
the
> latest revision. If a particular revid can't be parsed we assume the first
revid
> we find is the one we want. This means the list of dependency files can be
> effectively ordered from most important -> least important

I think this is a good idea. I have now changed the behaviour so that by default
it uses the earliest revision file stated, which makes it easy to update a bunch
of projects while keeping deps for a particular project intact.

Unfortunately if this is desired (for example we want to give juju-core deps
precedence), there's nothing to stop the highest-precedence project itself being
updated. We have
potential cyclic project dependencies here, and updating dependencies can
affect the dependency graph itself. Some thought is required.

I've also added a -latest flag to enable the previous behaviour (updating
to latest dependencies), which will also be useful, I think.

[mattyw, 2015-01-27 18:36:52]

two small nitpicks - otherwise LGTM

https://codereview.appspot.com/197110043/diff/40001/godeps.go
File godeps.go (right):

https://codereview.appspot.com/197110043/diff/40001/godeps.go#newcode65
godeps.go:65: a warning will be printed and  godeps will choose the first
dependency
two spaces between and  godeps

https://codereview.appspot.com/197110043/diff/40001/godeps.go#newcode781
godeps.go:781: func laterIntRevno(i0, i1 VCSInfo) bool {
I'd quite like a TODO here (or near the top) stating the reasons in your
explanation for leaving this as it is. Just as a reminder in case no one thinks
about this issue for 6 months.

[sinzui.is, 2015-01-27 18:37:17]

Thank you for adding multi-file support and confict resolution.

Juju QA is writing a script that does much of this. Our use case is to do
integration testing of to juju and another go-based project. We also choose to
pass multiple dependencies.tsv files and use the order as the order or
precedence. The script warns when there is conflict with a higher precedence
dep. We don't want an error since one or both code bases are changing in this
case.

I am inclined to wait a for this feature to merge and abandon the QA script. We
run with godeps tip so we will get the goodness when it is merged.

There is a second case that Juju QA needs to solve for release testing. We care
about specific versions when we test packaging. For example. when we know the
exact version of Juju to be released, and the other project does not specify the
juju version in its deps (possibly a bug in the other project), we include the
juju rev we require at the command line. In this case, we treat conflicts as
errors. Even if the other project specifies the exact version of Juju, we
override it from the command line arg to verify there are no conflicts (and warn
that the juju version was overridden by our command).

I know some projects assume there is a juju in the gopath. I would prefer they
include juju in the dependencies.tsv to be clear about the version of juju they
expect to work with. I am thinking of a case where project foo requires juju at
rev abc, and godeps discovers juju has a dependencies.tsv so uses it. In this
case the foo project is the base deps and warning are raised if juju conflicts.
I expect the developers of foo to change their deps, possibly the juju rev, or
contact juju developers to change their deps.