From 43ea0fc52288d967a74d909a2d3b9c8b742c3451 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Tue, 24 Jan 2017 08:38:28 -0500 Subject: [PATCH] Chase gps again This fixes an issue where project cycles involving the root project were erroneously rejected. It should fix #116. --- lock.json | 2 +- vendor/github.com/sdboyer/gps/bridge.go | 2 +- vendor/github.com/sdboyer/gps/rootdata.go | 11 +++++-- vendor/github.com/sdboyer/gps/selection.go | 14 ++------ .../sdboyer/gps/solve_bimodal_test.go | 32 +++++++++++++++++++ vendor/github.com/sdboyer/gps/solver.go | 7 +++- 6 files changed, 51 insertions(+), 17 deletions(-) diff --git a/lock.json b/lock.json index 7671de7363..85b4aa85e7 100644 --- a/lock.json +++ b/lock.json @@ -36,7 +36,7 @@ { "name": "github.com/sdboyer/gps", "branch": "master", - "revision": "7609a0652e71ad8738aff44557bed62d5ed0d0fa", + "revision": "379c7154f809d84a6be55a0dd7fa8947bb43c1ed", "packages": [ "." ] diff --git a/vendor/github.com/sdboyer/gps/bridge.go b/vendor/github.com/sdboyer/gps/bridge.go index 34945dcdb7..222b372039 100644 --- a/vendor/github.com/sdboyer/gps/bridge.go +++ b/vendor/github.com/sdboyer/gps/bridge.go @@ -284,7 +284,7 @@ func (b *bridge) vtu(id ProjectIdentifier, v Version) versionTypeUnion { // responsible for that code. func (b *bridge) ListPackages(id ProjectIdentifier, v Version) (PackageTree, error) { if b.s.rd.isRoot(id.ProjectRoot) { - panic("should never call ListPackages on root project") + return b.s.rd.rpt, nil } b.s.mtr.push("b-list-pkgs") diff --git a/vendor/github.com/sdboyer/gps/rootdata.go b/vendor/github.com/sdboyer/gps/rootdata.go index c8ae5ac1d5..af075b268d 100644 --- a/vendor/github.com/sdboyer/gps/rootdata.go +++ b/vendor/github.com/sdboyer/gps/rootdata.go @@ -141,12 +141,17 @@ func (rd rootdata) combineConstraints() []workingConstraint { // needVersionListFor indicates whether we need a version list for a given // project root, based solely on general solver inputs (no constraint checking -// required). This will be true if any of the following conditions hold: +// required). Assuming the argument is not the root project itself, this will be +// true if any of the following conditions hold: // // - ChangeAll is on // - The project is not in the lock // - The project is in the lock, but is also in the list of projects to change func (rd rootdata) needVersionsFor(pr ProjectRoot) bool { + if rd.isRoot(pr) { + return false + } + if rd.chngall { return true } @@ -154,7 +159,9 @@ func (rd rootdata) needVersionsFor(pr ProjectRoot) bool { if _, has := rd.rlm[pr]; !has { // not in the lock return true - } else if _, has := rd.chng[pr]; has { + } + + if _, has := rd.chng[pr]; has { // in the lock, but marked for change return true } diff --git a/vendor/github.com/sdboyer/gps/selection.go b/vendor/github.com/sdboyer/gps/selection.go index 7f03c5171c..cab3e7798f 100644 --- a/vendor/github.com/sdboyer/gps/selection.go +++ b/vendor/github.com/sdboyer/gps/selection.go @@ -82,12 +82,7 @@ func (s *selection) getRequiredPackagesIn(id ProjectIdentifier) map[string]int { uniq := make(map[string]int) for _, dep := range s.deps[id.ProjectRoot] { for _, pkg := range dep.dep.pl { - if count, has := uniq[pkg]; has { - count++ - uniq[pkg] = count - } else { - uniq[pkg] = 1 - } + uniq[pkg] = uniq[pkg] + 1 } } @@ -105,12 +100,7 @@ func (s *selection) getSelectedPackagesIn(id ProjectIdentifier) map[string]int { for _, p := range s.projects { if p.a.a.id.eq(id) { for _, pkg := range p.a.pl { - if count, has := uniq[pkg]; has { - count++ - uniq[pkg] = count - } else { - uniq[pkg] = 1 - } + uniq[pkg] = uniq[pkg] + 1 } } } diff --git a/vendor/github.com/sdboyer/gps/solve_bimodal_test.go b/vendor/github.com/sdboyer/gps/solve_bimodal_test.go index 885afe05ae..cf6674007b 100644 --- a/vendor/github.com/sdboyer/gps/solve_bimodal_test.go +++ b/vendor/github.com/sdboyer/gps/solve_bimodal_test.go @@ -271,6 +271,38 @@ var bimodalFixtures = map[string]bimodalFixture{ "b 1.0.0", ), }, + "project cycle involving root": { + ds: []depspec{ + dsp(mkDepspec("root 0.0.0", "a ~1.0.0"), + pkg("root", "a"), + pkg("root/foo"), + ), + dsp(mkDepspec("a 1.0.0"), + pkg("a", "root/foo"), + ), + }, + r: mksolution( + "a 1.0.0", + ), + }, + "project cycle not involving root": { + ds: []depspec{ + dsp(mkDepspec("root 0.0.0", "a ~1.0.0"), + pkg("root", "a"), + ), + dsp(mkDepspec("a 1.0.0"), + pkg("a", "b"), + pkg("a/foo"), + ), + dsp(mkDepspec("b 1.0.0"), + pkg("b", "a/foo"), + ), + }, + r: mksolution( + mklp("a 1.0.0", ".", "foo"), + "b 1.0.0", + ), + }, // Ensure that if a constraint is expressed, but no actual import exists, // then the constraint is disregarded - the project named in the constraint // is not part of the solution. diff --git a/vendor/github.com/sdboyer/gps/solver.go b/vendor/github.com/sdboyer/gps/solver.go index d75166ea2f..a039e4c6d1 100644 --- a/vendor/github.com/sdboyer/gps/solver.go +++ b/vendor/github.com/sdboyer/gps/solver.go @@ -455,7 +455,6 @@ func (s *solver) solve() (map[atom]map[string]struct{}, error) { func (s *solver) selectRoot() error { s.mtr.push("select-root") // Push the root project onto the queue. - // TODO(sdboyer) maybe it'd just be better to skip this? awp := s.rd.rootAtom() s.sel.pushSelection(awp, true) @@ -1063,6 +1062,12 @@ func (s *solver) selectAtom(a atomWithPackages, pkgonly bool) { } for _, dep := range deps { + // Root can come back up here if there's a project-level cycle. + // Satisfiability checks have already ensured invariants are maintained, + // so we know we can just skip it here. + if s.rd.isRoot(dep.Ident.ProjectRoot) { + continue + } // If this is dep isn't in the lock, do some prefetching. (If it is, we // might be able to get away with zero network activity for it, so don't // prefetch). This provides an opportunity for some parallelism wins, on