Merge lp:~rogpeppe/godeps/002-multi-dependency-update into lp:godeps

[Roger Peppe]

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://codereview.appspot.com/197110043/

[Roger Peppe (rogpeppe)]

Reviewers: mp+247684_code.launchpad.net,

Message:
Please take a look.

Description:
allow updating dependencies from multiple files

When there's a conflict, it prints a warning and
tries to choose the latest dependency.

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

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/197110043/

Affected files (+121, -9 lines):
   A [revision details]
   M godeps.go

[Matthew Williams (mattyw)]

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

https://codereview.appspot.com/197110043/

[lp:~rogpeppe/godeps/002-multi-dependency-update]

29. By Roger Peppe on 2015-01-27

add latest flag, docs; change -dryrun behaviour slightly

[Roger Peppe (rogpeppe)]

Please take a look.

https://codereview.appspot.com/197110043/

[Roger Peppe (rogpeppe)]

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.

https://codereview.appspot.com/197110043/

[Matthew Williams (mattyw)]

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.

https://codereview.appspot.com/197110043/

Unmerged revisions

29. By Roger Peppe on 2015-01-27

add latest flag, docs; change -dryrun behaviour slightly

28. By Roger Peppe on 2015-01-27

godeps: allow multiple dependency files to be updated

=== modified file 'godeps.go'
--- godeps.go	2014-12-04 16:32:37 +0000
+++ godeps.go	2015-01-27 17:42:53 +0000
@@ -23,9 +23,11 @@
 
 var (
 	revFile       = flag.String("u", "", "update dependencies")
+	updateMany    = flag.Bool("U", false, "update many dependencies")
 	testDeps      = flag.Bool("t", false, "include testing dependencies")
 	printCommands = flag.Bool("x", false, "show executed commands")
-	dryRun        = flag.Bool("n", false, "print but do not execute update commands")
+	latest        = flag.Bool("latest", false, "use latest dependency version on update conflicts")
+	dryRun        = flag.Bool("n", false, "do not execute update commands but pretend they have run successfully")
 	_             = flag.Bool("f", true, "(deprecated, superceded by -F) when updating, try to fetch deps if the update fails")
 	noFetch       = flag.Bool("F", false, "when updating, do not try to fetch deps if the update fails")
 	parallel      = flag.Int("P", 1, "max number of concurrent updates")
@@ -39,6 +41,7 @@
 Usage:
 	godeps [flags] [pkg ...]
 	godeps -u file [flags]
+	godeps -U [flags] file...
 
 In the first form of usage (without the -u flag), godeps prints to
 standard output a list of all the source dependencies of the named
@@ -54,6 +57,14 @@
 than one line for the same package root. If a specified revision is not
 currently available, godeps will attempt to fetch it, unless the -F flag
 is provided.
+
+In the third form, the sources will be updated to versions specified
+by any of the given files, which should all hold version information
+in the same form printed by godeps. If there is a dependency conflict
+(that is, more than one dependency is found for a given package root),
+a warning will be printed and  godeps will choose the first dependency
+found. If the -latest flag is specified, it will instead update to
+the latest revision number or revision date found in the files.
 `[1:]
 
 func main() {
@@ -63,11 +74,20 @@
 		os.Exit(2)
 	}
 	flag.Parse()
-	if *revFile != "" {
-		if flag.NArg() != 0 {
-			flag.Usage()
+	if *revFile != "" || *updateMany {
+		var files []string
+		if *revFile != "" {
+			if flag.NArg() != 0 {
+				flag.Usage()
+			}
+			files = []string{*revFile}
+		} else {
+			if flag.NArg() == 0 {
+				flag.Usage()
+			}
+			files = flag.Args()
 		}
-		update(*revFile)
+		update(files)
 	} else {
 		pkgs := flag.Args()
 		if len(pkgs) == 0 {
@@ -81,12 +101,17 @@
 	os.Exit(exitCode)
 }
 
-func update(file string) {
-	projects, err := parseDepFile(file)
-	if err != nil {
-		errorf("cannot parse %q: %v", file, err)
-		return
+func update(files []string) {
+	fp := make(map[string]map[string]*depInfo)
+	for _, file := range files {
+		projects, err := parseDepFile(file)
+		if err != nil {
+			errorf("cannot parse %q: %v", file, err)
+			return
+		}
+		fp[file] = projects
 	}
+	projects := reconcileDeps(fp)
 	// First get info on all the projects, make sure their working
 	// directories are all clean and prune out the ones which
 	// don't need updating.
@@ -117,6 +142,57 @@
 	updateProjects(projects)
 }
 
+type fileDepInfo struct {
+	file string
+	*depInfo
+}
+
+// reconcileDeps takes dependencies from a bunch
+// of different dependency files and returns a single
+// unified project-to-info map containing all the dependencies,
+// each one at the latest version found.
+func reconcileDeps(fp map[string]map[string]*depInfo) map[string]*depInfo {
+	m := make(map[string]byLatestDep)
+	for file, projects := range fp {
+		for _, info := range projects {
+			m[info.project] = append(m[info.project], fileDepInfo{
+				file:    file,
+				depInfo: info,
+			})
+		}
+	}
+	r := make(map[string]*depInfo)
+	for project, infos := range m {
+		if *latest {
+			sort.Stable(infos)
+		}
+		r[project] = infos[0].depInfo
+		for _, info := range infos {
+			if info.revid != infos[0].revid {
+				fmt.Printf("%s: warning: ignoring %s at %s; using %s from %s\n", info.file, project, info.revno, infos[0].revno, infos[0].file)
+			}
+		}
+	}
+	return r
+}
+
+// byLatestDep can be used to sort dependency infos
+// into latest-first order.
+type byLatestDep []fileDepInfo
+
+func (s byLatestDep) Len() int      { return len(s) }
+func (s byLatestDep) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
+func (s byLatestDep) Less(i, j int) bool {
+	i0, i1 := s[i], s[j]
+	if i0.revid != i1.revid {
+		if i0.vcs.Kind() != i1.vcs.Kind() {
+			return i0.vcs.Kind() < i1.vcs.Kind()
+		}
+		return i0.vcs.Later(i0.depInfo.VCSInfo, i1.depInfo.VCSInfo)
+	}
+	return i0.file < i1.file
+}
+
 func updateProjects(projects map[string]*depInfo) {
 	limit := make(chan struct{}, *parallel)
 	type result struct {
@@ -516,6 +592,7 @@
 	Info(dir string) (VCSInfo, error)
 	Update(dir, revid string) error
 	Fetch(dir string) error
+	Later(i0, i1 VCSInfo) bool
 }
 
 type VCSInfo struct {
@@ -587,6 +664,24 @@
 	return err
 }
 
+func (gitVCS) Later(i0, i1 VCSInfo) bool {
+	if i0.revno == "" || i1.revno == "" {
+		return i0.revno != ""
+	}
+	t0, err0 := time.Parse(time.RFC3339, i0.revno)
+	if err0 != nil {
+		fmt.Printf("warning: cannot parse time stamp %s: %v", i0.revno, err0)
+	}
+	t1, err1 := time.Parse(time.RFC3339, i1.revno)
+	if err1 != nil {
+		fmt.Printf("warning: cannot parse time stamp %s: %v", i1.revno, err1)
+	}
+	if err0 != nil || err1 != nil {
+		return err0 == nil
+	}
+	return t0.After(t1)
+}
+
 type bzrVCS struct{}
 
 func (bzrVCS) Kind() string {
@@ -636,6 +731,10 @@
 	return err
 }
 
+func (bzrVCS) Later(i0, i1 VCSInfo) bool {
+	return laterIntRevno(i0, i1)
+}
+
 var validHgInfo = regexp.MustCompile(`^([a-f0-9]+) ([0-9]+)$`)
 
 type hgVCS struct{}
@@ -675,9 +774,33 @@
 	return err
 }
 
+func (hgVCS) Later(i0, i1 VCSInfo) bool {
+	return laterIntRevno(i0, i1)
+}
+
+func laterIntRevno(i0, i1 VCSInfo) bool {
+	if i0.revno == "" || i1.revno == "" {
+		return i0.revno != ""
+	}
+	r0, err0 := strconv.Atoi(i0.revno)
+	if err0 != nil {
+		fmt.Printf("warning: cannot parse revision number %s: %v", i0.revno, err0)
+	}
+	r1, err1 := strconv.Atoi(i1.revno)
+	if err1 != nil {
+		fmt.Printf("warning: cannot parse revision number %s: %v", i1.revno, err1)
+	}
+	if err0 != nil || err1 != nil {
+		return err0 == nil
+	}
+	return r0 > r1
+}
+
 func runUpdateCmd(dir string, name string, args ...string) (string, error) {
 	if *dryRun {
-		printShellCommand(dir, name, args)
+		if *printCommands {
+			printShellCommand(dir, name, args)
+		}
 		return "", nil
 	}
 	return runCmd(dir, name, args...)
@@ -685,9 +808,6 @@
 
 func runCmd(dir string, name string, args ...string) (string, error) {
 	var outData, errData bytes.Buffer
-	if *printCommands {
-		printShellCommand(dir, name, args)
-	}
 	c := exec.Command(name, args...)
 	c.Stdout = &outData
 	c.Stderr = &errData