diff --git a/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/README.md b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/README.md new file mode 100644 index 0000000000..f54848a7a4 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/README.md @@ -0,0 +1,5 @@ +# Ensure - Transitive Constraint + +[github.com/carolynvs/deptest-transcons-c](github.com/carolynvs/deptest-transcons-c) +has a bug in the latest release. I am a library and need to define a constraint +so that consumers of my library don't pull in the bad version of C. diff --git a/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/final/Gopkg.lock new file mode 100644 index 0000000000..9b7f01d789 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/final/Gopkg.lock @@ -0,0 +1,21 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + name = "github.com/carolynvs/deptest-transcons-b" + packages = ["."] + revision = "bc9969bda2dd54b67470c2f199b14d2665c0f7a1" + version = "v1.0.0" + +[[projects]] + name = "github.com/carolynvs/deptest-transcons-c" + packages = ["."] + revision = "24ede1a3d5a6a036793bfcadf8802f60148adf90" + version = "v1.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "bd219aa442b67cb568bd608a1c39644db8835170c8c5b83bfd3e0f8f0caf07f3" + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/final/Gopkg.toml new file mode 100644 index 0000000000..a7a1930daa --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/final/Gopkg.toml @@ -0,0 +1,7 @@ +[[constraint]] + name = "github.com/carolynvs/deptest-transcons-b" + version = "1.0.0" + +[[constraint]] + name = "github.com/carolynvs/deptest-transcons-c" + version = "<1.0.1" diff --git a/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/initial/Gopkg.toml b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/initial/Gopkg.toml new file mode 100644 index 0000000000..a7a1930daa --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/initial/Gopkg.toml @@ -0,0 +1,7 @@ +[[constraint]] + name = "github.com/carolynvs/deptest-transcons-b" + version = "1.0.0" + +[[constraint]] + name = "github.com/carolynvs/deptest-transcons-c" + version = "<1.0.1" diff --git a/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/initial/a.go b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/initial/a.go new file mode 100644 index 0000000000..49c38a5558 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/initial/a.go @@ -0,0 +1,15 @@ +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package a + +import ( + "fmt" + "github.com/carolynvs/deptest-transcons-b" +) + +func A() { + fmt.Println("a did a thing") + b.B() +} diff --git a/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/testcase.json b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/testcase.json new file mode 100644 index 0000000000..f9b242b288 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/ensure/transitive-constraint/testcase.json @@ -0,0 +1,9 @@ +{ + "commands": [ + ["ensure"] + ], + "vendor-final": [ + "github.com/carolynvs/deptest-transcons-b", + "github.com/carolynvs/deptest-transcons-c" + ] +} diff --git a/cmd/dep/testdata/harness_tests/init/transitive-constraint/README.md b/cmd/dep/testdata/harness_tests/init/transitive-constraint/README.md new file mode 100644 index 0000000000..30fa4fd9da --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/transitive-constraint/README.md @@ -0,0 +1,7 @@ +# Init - Transitive Constraint + +[C](github.com/carolynvs/deptest-transcons-c) +has a bug in the latest release. I am an end-user of [A](github.com/carolynvs/deptest-transcons-a) +which transitively depends on C. A has a constraint on C which avoids a bad release of C. +End-users like me should be able to use A, and have `dep init` pick a version of C +that doesn't have the bug, without manually adding overrides. diff --git a/cmd/dep/testdata/harness_tests/init/transitive-constraint/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/init/transitive-constraint/final/Gopkg.lock new file mode 100644 index 0000000000..92959cf74a --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/transitive-constraint/final/Gopkg.lock @@ -0,0 +1,27 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + name = "github.com/carolynvs/deptest-transcons-a" + packages = ["."] + revision = "750587f3002d87affe4971e0a0f1652272633e76" + version = "v1.0.0" + +[[projects]] + name = "github.com/carolynvs/deptest-transcons-b" + packages = ["."] + revision = "bc9969bda2dd54b67470c2f199b14d2665c0f7a1" + version = "v1.0.0" + +[[projects]] + name = "github.com/carolynvs/deptest-transcons-c" + packages = ["."] + revision = "24ede1a3d5a6a036793bfcadf8802f60148adf90" + version = "v1.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "8d17577c18791655d6aa0f6fcb1034e51f9e47f57b55ce7091e077a1c4dc2da2" + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/init/transitive-constraint/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/transitive-constraint/final/Gopkg.toml new file mode 100644 index 0000000000..f904aeebfc --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/transitive-constraint/final/Gopkg.toml @@ -0,0 +1,4 @@ + +[[constraint]] + name = "github.com/carolynvs/deptest-transcons-a" + version = "1.0.0" diff --git a/cmd/dep/testdata/harness_tests/init/transitive-constraint/initial/main.go b/cmd/dep/testdata/harness_tests/init/transitive-constraint/initial/main.go new file mode 100644 index 0000000000..e4bd0d335f --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/transitive-constraint/initial/main.go @@ -0,0 +1,15 @@ +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "fmt" + "github.com/carolynvs/deptest-transcons-a" +) + +func main () { + fmt.Println("Stand back, I'm gonna do a thing!") + a.A() +} diff --git a/cmd/dep/testdata/harness_tests/init/transitive-constraint/testcase.json b/cmd/dep/testdata/harness_tests/init/transitive-constraint/testcase.json new file mode 100644 index 0000000000..d87ecde5ad --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/transitive-constraint/testcase.json @@ -0,0 +1,10 @@ +{ + "commands": [ + ["init", "--no-examples"] + ], + "vendor-final": [ + "github.com/carolynvs/deptest-transcons-a", + "github.com/carolynvs/deptest-transcons-b", + "github.com/carolynvs/deptest-transcons-c" + ] +} diff --git a/gps/identifier.go b/gps/identifier.go index cf3ca23513..093eb852d2 100644 --- a/gps/identifier.go +++ b/gps/identifier.go @@ -177,6 +177,8 @@ type bimodalIdentifier struct { prefv Version // Indicates that the bmi came from the root project originally fromRoot bool + // The path to the atom in the graph, e.g. root -> foo -> bar + path []atom } type atom struct { @@ -190,20 +192,8 @@ var nilpa = atom{ } type atomWithPackages struct { - a atom - pl []string -} - -// bmi converts an atomWithPackages into a bimodalIdentifier. -// -// This is mostly intended for (read-only) trace use, so the package list slice -// is not copied. It is the callers responsibility to not modify the pl slice, -// lest that backpropagate and cause inconsistencies. -func (awp atomWithPackages) bmi() bimodalIdentifier { - return bimodalIdentifier{ - id: awp.a.id, - pl: awp.pl, - } + a atom + bmi bimodalIdentifier } // completeDep (name hopefully to change) provides the whole picture of a @@ -215,12 +205,14 @@ type completeDep struct { workingConstraint // The specific packages required from the ProjectDep pl []string + // Flags the constraint as being defined on an indirect/transitive dependency + isTransitive bool } // dependency represents an incomplete edge in the depgraph. It has a // fully-realized atom as the depender (the tail/source of the edge), and a set // of requirements that any atom to be attached at the head/target must satisfy. type dependency struct { - depender atom + depender atomWithPackages dep completeDep } diff --git a/gps/rootdata.go b/gps/rootdata.go index ee78bdf265..20550d7ca6 100644 --- a/gps/rootdata.go +++ b/gps/rootdata.go @@ -198,7 +198,7 @@ func (rd rootdata) rootAtom() atomWithPackages { sort.Strings(list) return atomWithPackages{ - a: a, - pl: list, + a: a, + bmi: bimodalIdentifier{id: a.id, pl: list}, } } diff --git a/gps/satisfy.go b/gps/satisfy.go index abac0ea7ef..555e2b6887 100644 --- a/gps/satisfy.go +++ b/gps/satisfy.go @@ -11,8 +11,7 @@ package gps // The goal is to determine whether selecting the atom would result in a state // where all the solver requirements are still satisfied. func (s *solver) check(a atomWithPackages, pkgonly bool) error { - pa := a.a - if nilpa == pa { + if nilpa == a.a { // This shouldn't be able to happen, but if it does, it unequivocally // indicates a logical bug somewhere, so blowing up is preferable panic("canary - checking version of empty ProjectAtom") @@ -30,7 +29,7 @@ func (s *solver) check(a atomWithPackages, pkgonly bool) error { // If we're pkgonly, then base atom was already determined to be allowable, // so we can skip the checkAtomAllowable step. if !pkgonly { - if err = s.checkAtomAllowable(pa); err != nil { + if err = s.checkAtomAllowable(a); err != nil { return err } } @@ -78,24 +77,24 @@ func (s *solver) check(a atomWithPackages, pkgonly bool) error { // checkAtomAllowable ensures that an atom itself is acceptable with respect to // the constraints established by the current solution. -func (s *solver) checkAtomAllowable(pa atom) error { - constraint := s.sel.getConstraint(pa.id) - if s.vUnify.matches(pa.id, constraint, pa.v) { +func (s *solver) checkAtomAllowable(awp atomWithPackages) error { + constraint := s.sel.getConstraint(awp.a.id, awp.bmi) + if s.vUnify.matches(awp.a.id, constraint, awp.a.v) { return nil } // TODO(sdboyer) collect constraint failure reason (wait...aren't we, below?) - deps := s.sel.getDependenciesOn(pa.id) + deps := s.sel.getDependenciesOn(awp.a.id) var failparent []dependency for _, dep := range deps { - if !s.vUnify.matches(pa.id, dep.dep.Constraint, pa.v) { - s.fail(dep.depender.id) + if !s.vUnify.matches(awp.a.id, dep.dep.Constraint, awp.a.v) { + s.fail(dep.depender.a.id) failparent = append(failparent, dep) } } err := &versionNotAllowedFailure{ - goal: pa, + goal: awp.a, failparent: failparent, c: constraint, } @@ -120,14 +119,14 @@ func (s *solver) checkRequiredPackagesExist(a atomWithPackages) error { for _, dep := range deps { for _, pkg := range dep.dep.pl { if errdep, seen := fp[pkg]; seen { - errdep.deppers = append(errdep.deppers, dep.depender) + errdep.deppers = append(errdep.deppers, dep.depender.a) fp[pkg] = errdep } else { perr, has := ptree.Packages[pkg] if !has || perr.Err != nil { fp[pkg] = errDeppers{ err: perr.Err, - deppers: []atom{dep.depender}, + deppers: []atom{dep.depender.a}, } } } @@ -147,7 +146,7 @@ func (s *solver) checkRequiredPackagesExist(a atomWithPackages) error { // given dep are valid with respect to existing constraints. func (s *solver) checkDepsConstraintsAllowable(a atomWithPackages, cdep completeDep) error { dep := cdep.workingConstraint - constraint := s.sel.getConstraint(dep.Ident) + constraint := s.sel.getConstraint(dep.Ident, a.bmi) // Ensure the constraint expressed by the dep has at least some possible // intersection with the intersection of existing constraints. if s.vUnify.matchesAny(dep.Ident, constraint, dep.Constraint) { @@ -160,7 +159,7 @@ func (s *solver) checkDepsConstraintsAllowable(a atomWithPackages, cdep complete var nofailsib []dependency for _, sibling := range siblings { if !s.vUnify.matchesAny(dep.Ident, sibling.dep.Constraint, dep.Constraint) { - s.fail(sibling.depender.id) + s.fail(sibling.depender.a.id) failsib = append(failsib, sibling) } else { nofailsib = append(nofailsib, sibling) @@ -168,7 +167,7 @@ func (s *solver) checkDepsConstraintsAllowable(a atomWithPackages, cdep complete } return &disjointConstraintFailure{ - goal: dependency{depender: a.a, dep: cdep}, + goal: dependency{depender: a, dep: cdep}, failsib: failsib, nofailsib: nofailsib, c: constraint, @@ -185,7 +184,7 @@ func (s *solver) checkDepsDisallowsSelected(a atomWithPackages, cdep completeDep s.fail(dep.Ident) return &constraintNotAllowedFailure{ - goal: dependency{depender: a.a, dep: cdep}, + goal: dependency{depender: a, dep: cdep}, v: selected.a.v, } } @@ -206,7 +205,7 @@ func (s *solver) checkIdentMatches(a atomWithPackages, cdep completeDep) error { // Fail all the other deps, as there's no way atom can ever be // compatible with them for _, d := range deps { - s.fail(d.depender.id) + s.fail(d.depender.a.id) } return &sourceMismatchFailure{ @@ -236,7 +235,7 @@ func (s *solver) checkRootCaseConflicts(a atomWithPackages, cdep completeDep) er curid, _ := s.sel.getIdentFor(current) deps := s.sel.getDependenciesOn(curid) for _, d := range deps { - s.fail(d.depender.id) + s.fail(d.depender.a.id) } // If a project has multiple packages that import each other, we treat that @@ -260,13 +259,13 @@ func (s *solver) checkRootCaseConflicts(a atomWithPackages, cdep completeDep) er if current == a.a.id.ProjectRoot { return &wrongCaseFailure{ correct: pr, - goal: dependency{depender: a.a, dep: cdep}, + goal: dependency{depender: a, dep: cdep}, badcase: deps, } } return &caseMismatchFailure{ - goal: dependency{depender: a.a, dep: cdep}, + goal: dependency{depender: a, dep: cdep}, current: current, failsib: deps, } @@ -289,7 +288,7 @@ func (s *solver) checkPackageImportsFromDepExist(a atomWithPackages, cdep comple e := &depHasProblemPackagesFailure{ goal: dependency{ - depender: a.a, + depender: a, dep: cdep, }, v: sel.a.v, @@ -329,7 +328,7 @@ func (s *solver) checkRevisionExists(a atomWithPackages, cdep completeDep) error return &nonexistentRevisionFailure{ goal: dependency{ - depender: a.a, + depender: a, dep: cdep, }, r: r, diff --git a/gps/selection.go b/gps/selection.go index a74c60ae8d..a3d76fdc62 100644 --- a/gps/selection.go +++ b/gps/selection.go @@ -4,6 +4,10 @@ package gps +import ( + "fmt" +) + type selection struct { // projects is a stack of the atoms that have currently been selected by the // solver. It can also be thought of as the vertex set of the current @@ -134,7 +138,7 @@ func (s *selection) getSelectedPackagesIn(id ProjectIdentifier) map[string]int { uniq := make(map[string]int) for _, p := range s.projects { if p.a.a.id.eq(id) { - for _, pkg := range p.a.pl { + for _, pkg := range p.a.bmi.pl { uniq[pkg] = uniq[pkg] + 1 } } @@ -143,12 +147,18 @@ func (s *selection) getSelectedPackagesIn(id ProjectIdentifier) map[string]int { return uniq } -func (s *selection) getConstraint(id ProjectIdentifier) Constraint { +func (s *selection) getConstraint(id ProjectIdentifier, bmi bimodalIdentifier) Constraint { deps, exists := s.deps[id.ProjectRoot] if !exists || len(deps) == 0 { return any } + // Enable quick lookup of where in the depgraph a constraint was defined + ancestors := map[ProjectRoot]bool{} + for _, ancestor := range bmi.path { + ancestors[ancestor.id.ProjectRoot] = true + } + // TODO(sdboyer) recomputing this sucks and is quite wasteful. Precompute/cache it // on changes to the constraint set, instead. @@ -159,6 +169,19 @@ func (s *selection) getConstraint(id ProjectIdentifier) Constraint { // Start with the open set var ret Constraint = any for _, dep := range deps { + if dep.dep.isTransitive { + // TODO(carolynvs): Remove print statements. It's just to help debug the transitive constraint prototype. + path := "" + for ancestor := range ancestors { + path += "/" + string(ancestor) + } + if _, isAncestor := ancestors[dep.depender.a.id.ProjectRoot]; !isAncestor { + fmt.Printf("*** Ignoring unreachable constraint %s on %s (path: %s) from %s ***\n", dep.dep.Constraint.String(), dep.dep.workingConstraint.Ident.ProjectRoot, path, dep.depender.a.id.ProjectRoot) + continue + } + fmt.Printf("*** Applying transitive constraint %s on %s (path: %s) from %s ***\n", dep.dep.Constraint.String(), dep.dep.workingConstraint.Ident.ProjectRoot, path, dep.depender.a.id.ProjectRoot) + } + ret = s.vu.intersect(id, ret, dep.dep.Constraint) } diff --git a/gps/solve_basic_test.go b/gps/solve_basic_test.go index 910b9d622b..d457aa4c0c 100644 --- a/gps/solve_basic_test.go +++ b/gps/solve_basic_test.go @@ -226,14 +226,15 @@ func mkDepspec(pi string, deps ...string) depspec { func mkDep(atom, pdep string, pl ...string) dependency { return dependency{ - depender: mkAtom(atom), + depender: atomWithPackages{a: mkAtom(atom)}, dep: mkCDep(pdep, pl...), } } func mkADep(atom, pdep string, c Constraint, pl ...string) dependency { return dependency{ - depender: mkAtom(atom), + // TODO(carolynvs): I don't think we need to set the bmi (specifically the path to the atom) in the fixture data. + depender: atomWithPackages{a: mkAtom(atom)}, dep: completeDep{ workingConstraint: workingConstraint{ Ident: ProjectIdentifier{ diff --git a/gps/solve_bimodal_test.go b/gps/solve_bimodal_test.go index 7dfa756279..cc1996ffd1 100644 --- a/gps/solve_bimodal_test.go +++ b/gps/solve_bimodal_test.go @@ -132,6 +132,42 @@ var bimodalFixtures = map[string]bimodalFixture{ "b 1.0.0", ), }, + "used transitive constraint": { + ds: []depspec{ + dsp(mkDepspec("root 1.0.0", "foo 1.0.0"), + pkg("root", "foo"), + ), + dsp(mkDepspec("foo 1.0.0", "bar 1.0.0", "baz =1.0.0"), + pkg("foo", "bar"), + ), + dsp(mkDepspec("bar 1.0.0", "baz >=1.0.0"), + pkg("bar", "baz"), + ), + dsp(mkDepspec("baz 1.0.1"), pkg("baz")), + dsp(mkDepspec("baz 1.0.0"), pkg("baz")), + }, + r: mksolution( + "foo 1.0.0", + "bar 1.0.0", + "baz 1.0.0", + ), + }, + "unused transitive constraint": { + ds: []depspec{ + dsp(mkDepspec("root 1.0.0", "foo 1.0.0"), + pkg("root", "foo", "baz"), + ), + dsp(mkDepspec("foo 1.0.0", "baz =1.0.0"), + pkg("foo"), + ), + dsp(mkDepspec("baz 1.0.1"), pkg("baz")), + dsp(mkDepspec("baz 1.0.0"), pkg("baz")), + }, + r: mksolution( + "foo 1.0.0", + "baz 1.0.1", + ), + }, // Constraints apply only if the project that declares them has a // reachable import "constraints activated by import": { @@ -152,12 +188,12 @@ var bimodalFixtures = map[string]bimodalFixture{ }, r: mksolution( "a 1.0.0", - "b 1.1.0", + "b 1.0.0", // Now that constraints can be applied transitively, the constraint from root applies ), }, // Constraints apply only if the project that declares them has a // reachable import - non-root - "constraints activated by import, transitive": { + "constraints activated by import, non-root": { ds: []depspec{ dsp(mkDepspec("root 0.0.0"), pkg("root", "root/foo", "b"), diff --git a/gps/solve_failures.go b/gps/solve_failures.go index 05daedd707..1026bdc139 100644 --- a/gps/solve_failures.go +++ b/gps/solve_failures.go @@ -78,16 +78,16 @@ type caseMismatchFailure struct { func (e *caseMismatchFailure) Error() string { if len(e.failsib) == 1 { str := "Could not introduce %s due to a case-only variation: it depends on %q, but %q was already established as the case variant for that project root by depender %s" - return fmt.Sprintf(str, a2vs(e.goal.depender), e.goal.dep.Ident.ProjectRoot, e.current, a2vs(e.failsib[0].depender)) + return fmt.Sprintf(str, a2vs(e.goal.depender.a), e.goal.dep.Ident.ProjectRoot, e.current, a2vs(e.failsib[0].depender.a)) } var buf bytes.Buffer str := "Could not introduce %s due to a case-only variation: it depends on %q, but %q was already established as the case variant for that project root by the following other dependers:\n" - fmt.Fprintf(&buf, str, a2vs(e.goal.depender), e.goal.dep.Ident.ProjectRoot, e.current) + fmt.Fprintf(&buf, str, a2vs(e.goal.depender.a), e.goal.dep.Ident.ProjectRoot, e.current) for _, c := range e.failsib { - fmt.Fprintf(&buf, "\t%s\n", a2vs(c.depender)) + fmt.Fprintf(&buf, "\t%s\n", a2vs(c.depender.a)) } return buf.String() @@ -97,7 +97,7 @@ func (e *caseMismatchFailure) traceString() string { var buf bytes.Buffer fmt.Fprintf(&buf, "case-only variation in dependency on %q; %q already established by:\n", e.goal.dep.Ident.ProjectRoot, e.current) for _, f := range e.failsib { - fmt.Fprintf(&buf, "%s\n", a2vs(f.depender)) + fmt.Fprintf(&buf, "%s\n", a2vs(f.depender.a)) } return buf.String() @@ -122,16 +122,16 @@ type wrongCaseFailure struct { func (e *wrongCaseFailure) Error() string { if len(e.badcase) == 1 { str := "Could not introduce %s; imports amongst its packages establish %q as the canonical casing for root, but %s tried to import it as %q" - return fmt.Sprintf(str, a2vs(e.goal.depender), e.correct, a2vs(e.badcase[0].depender), e.badcase[0].dep.Ident.ProjectRoot) + return fmt.Sprintf(str, a2vs(e.goal.depender.a), e.correct, a2vs(e.badcase[0].depender.a), e.badcase[0].dep.Ident.ProjectRoot) } var buf bytes.Buffer str := "Could not introduce %s; imports amongst its packages establish %q as the canonical casing for root, but the following projects tried to import it as %q" - fmt.Fprintf(&buf, str, a2vs(e.goal.depender), e.correct, e.badcase[0].dep.Ident.ProjectRoot) + fmt.Fprintf(&buf, str, a2vs(e.goal.depender.a), e.correct, e.badcase[0].dep.Ident.ProjectRoot) for _, c := range e.badcase { - fmt.Fprintf(&buf, "\t%s\n", a2vs(c.depender)) + fmt.Fprintf(&buf, "\t%s\n", a2vs(c.depender.a)) } return buf.String() @@ -141,7 +141,7 @@ func (e *wrongCaseFailure) traceString() string { var buf bytes.Buffer fmt.Fprintf(&buf, "internal imports establish %q as correct casing; %q was used by:\n", e.correct, e.goal.dep.Ident.ProjectRoot) for _, f := range e.badcase { - fmt.Fprintf(&buf, "%s\n", a2vs(f.depender)) + fmt.Fprintf(&buf, "%s\n", a2vs(f.depender.a)) } return buf.String() @@ -171,7 +171,7 @@ type disjointConstraintFailure struct { func (e *disjointConstraintFailure) Error() string { if len(e.failsib) == 1 { str := "Could not introduce %s, as it has a dependency on %s with constraint %s, which has no overlap with existing constraint %s from %s" - return fmt.Sprintf(str, a2vs(e.goal.depender), e.goal.dep.Ident, e.goal.dep.Constraint.String(), e.failsib[0].dep.Constraint.String(), a2vs(e.failsib[0].depender)) + return fmt.Sprintf(str, a2vs(e.goal.depender.a), e.goal.dep.Ident, e.goal.dep.Constraint.String(), e.failsib[0].dep.Constraint.String(), a2vs(e.failsib[0].depender.a)) } var buf bytes.Buffer @@ -181,16 +181,16 @@ func (e *disjointConstraintFailure) Error() string { sibs = e.failsib str := "Could not introduce %s, as it has a dependency on %s with constraint %s, which has no overlap with the following existing constraints:\n" - fmt.Fprintf(&buf, str, a2vs(e.goal.depender), e.goal.dep.Ident, e.goal.dep.Constraint.String()) + fmt.Fprintf(&buf, str, a2vs(e.goal.depender.a), e.goal.dep.Ident, e.goal.dep.Constraint.String()) } else { sibs = e.nofailsib str := "Could not introduce %s, as it has a dependency on %s with constraint %s, which does not overlap with the intersection of existing constraints from other currently selected packages:\n" - fmt.Fprintf(&buf, str, a2vs(e.goal.depender), e.goal.dep.Ident, e.goal.dep.Constraint.String()) + fmt.Fprintf(&buf, str, a2vs(e.goal.depender.a), e.goal.dep.Ident, e.goal.dep.Constraint.String()) } for _, c := range sibs { - fmt.Fprintf(&buf, "\t%s from %s\n", c.dep.Constraint.String(), a2vs(c.depender)) + fmt.Fprintf(&buf, "\t%s from %s\n", c.dep.Constraint.String(), a2vs(c.depender.a)) } return buf.String() @@ -204,7 +204,7 @@ func (e *disjointConstraintFailure) traceString() string { &buf, "%s from %s (no overlap)\n", f.dep.Constraint.String(), - a2vs(f.depender), + a2vs(f.depender.a), ) } for _, f := range e.nofailsib { @@ -212,7 +212,7 @@ func (e *disjointConstraintFailure) traceString() string { &buf, "%s from %s (some overlap)\n", f.dep.Constraint.String(), - a2vs(f.depender), + a2vs(f.depender.a), ) } @@ -234,7 +234,7 @@ type constraintNotAllowedFailure struct { func (e *constraintNotAllowedFailure) Error() string { return fmt.Sprintf( "Could not introduce %s, as it has a dependency on %s with constraint %s, which does not allow the currently selected version of %s", - a2vs(e.goal.depender), + a2vs(e.goal.depender.a), e.goal.dep.Ident, e.goal.dep.Constraint, e.v, @@ -244,7 +244,7 @@ func (e *constraintNotAllowedFailure) Error() string { func (e *constraintNotAllowedFailure) traceString() string { return fmt.Sprintf( "%s depends on %s with %s, but that's already selected at %s", - a2vs(e.goal.depender), + a2vs(e.goal.depender.a), e.goal.dep.Ident.ProjectRoot, e.goal.dep.Constraint, e.v, @@ -274,7 +274,7 @@ func (e *versionNotAllowedFailure) Error() string { "Could not introduce %s, as it is not allowed by constraint %s from project %s.", a2vs(e.goal), e.failparent[0].dep.Constraint.String(), - e.failparent[0].depender.id, + e.failparent[0].depender.a.id, ) } @@ -283,7 +283,7 @@ func (e *versionNotAllowedFailure) Error() string { fmt.Fprintf(&buf, "Could not introduce %s, as it is not allowed by constraints from the following projects:\n", a2vs(e.goal)) for _, f := range e.failparent { - fmt.Fprintf(&buf, "\t%s from %s\n", f.dep.Constraint.String(), a2vs(f.depender)) + fmt.Fprintf(&buf, "\t%s from %s\n", f.dep.Constraint.String(), a2vs(f.depender.a)) } return buf.String() @@ -294,7 +294,7 @@ func (e *versionNotAllowedFailure) traceString() string { fmt.Fprintf(&buf, "%s not allowed by constraint %s:\n", a2vs(e.goal), e.c.String()) for _, f := range e.failparent { - fmt.Fprintf(&buf, " %s from %s\n", f.dep.Constraint.String(), a2vs(f.depender)) + fmt.Fprintf(&buf, " %s from %s\n", f.dep.Constraint.String(), a2vs(f.depender.a)) } return buf.String() @@ -333,7 +333,7 @@ type sourceMismatchFailure struct { func (e *sourceMismatchFailure) Error() string { var cur []string for _, c := range e.sel { - cur = append(cur, string(c.depender.id.ProjectRoot)) + cur = append(cur, string(c.depender.a.id.ProjectRoot)) } str := "Could not introduce %s, as it depends on %s from %s, but %s is already marked as coming from %s by %s" @@ -346,7 +346,7 @@ func (e *sourceMismatchFailure) traceString() string { fmt.Fprintf(&buf, " %s from %s\n", e.mismatch, e.prob.id) for _, dep := range e.sel { - fmt.Fprintf(&buf, " %s from %s\n", e.current, dep.depender.id) + fmt.Fprintf(&buf, " %s from %s\n", e.current, dep.depender.a.id) } return buf.String() @@ -485,7 +485,7 @@ func (e *depHasProblemPackagesFailure) Error() string { return fmt.Sprintf( "Could not introduce %s, as it requires package %s from %s, but in version %s that package %s", - a2vs(e.goal.depender), + a2vs(e.goal.depender.a), pkg, e.goal.dep.Ident, e.v, @@ -496,7 +496,7 @@ func (e *depHasProblemPackagesFailure) Error() string { var buf bytes.Buffer fmt.Fprintf( &buf, "Could not introduce %s, as it requires problematic packages from %s (current version %s):", - a2vs(e.goal.depender), + a2vs(e.goal.depender.a), e.goal.dep.Ident, e.v, ) @@ -526,7 +526,7 @@ func (e *depHasProblemPackagesFailure) traceString() string { fmt.Fprintf( &buf, "%s depping on %s at %s has problem subpkg(s):", - a2vs(e.goal.depender), + a2vs(e.goal.depender.a), e.goal.dep.Ident, e.v, ) @@ -556,7 +556,7 @@ type nonexistentRevisionFailure struct { func (e *nonexistentRevisionFailure) Error() string { return fmt.Sprintf( "Could not introduce %s, as it requires %s at revision %s, but that revision does not exist", - a2vs(e.goal.depender), + a2vs(e.goal.depender.a), e.goal.dep.Ident, e.r, ) @@ -565,7 +565,7 @@ func (e *nonexistentRevisionFailure) Error() string { func (e *nonexistentRevisionFailure) traceString() string { return fmt.Sprintf( "%s wants missing rev %s of %s", - a2vs(e.goal.depender), + a2vs(e.goal.depender.a), e.r, e.goal.dep.Ident, ) diff --git a/gps/solver.go b/gps/solver.go index bede9d53b4..44126d13fc 100644 --- a/gps/solver.go +++ b/gps/solver.go @@ -538,7 +538,7 @@ func (s *solver) solve(ctx context.Context) (map[atom]map[string]struct{}, error id: queue.id, v: queue.current(), }, - pl: bmi.pl, + bmi: bmi, } err = s.selectAtom(awp, false) s.mtr.pop() @@ -566,7 +566,7 @@ func (s *solver) solve(ctx context.Context) (map[atom]map[string]struct{}, error id: bmi.id, v: awp.a.v, }, - pl: bmi.pl, + bmi: bmi, } s.traceCheckPkgs(bmi) @@ -610,7 +610,7 @@ func (s *solver) solve(ctx context.Context) (map[atom]map[string]struct{}, error projs[sel.a.a] = pm } - for _, path := range sel.a.pl { + for _, path := range sel.a.bmi.pl { pm[path] = struct{}{} } } @@ -644,9 +644,19 @@ func (s *solver) selectRoot() error { go s.b.SyncSourceFor(dep.Ident) } - s.sel.pushDep(dependency{depender: awp.a, dep: dep}) + s.sel.pushDep(dependency{ + depender: awp, + dep: dep, + }) + + if dep.isTransitive { + // Do not add transitive dependencies to the queue immediately, + // instead wait until they are directly used + continue + } + // Add all to unselected queue - heap.Push(s.unsel, bimodalIdentifier{id: dep.Ident, pl: dep.pl, fromRoot: true}) + heap.Push(s.unsel, bimodalIdentifier{id: dep.Ident, pl: dep.pl, fromRoot: true, path: []atom{awp.a}}) } s.traceSelectRoot(s.rd.rpt, deps) @@ -677,7 +687,7 @@ func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]string, []com // Use maps to dedupe the unique internal and external packages. exmap, inmap := make(map[string]struct{}), make(map[string]struct{}) - for _, pkg := range a.pl { + for _, pkg := range a.bmi.pl { inmap[pkg] = struct{}{} for _, ipkg := range rm[pkg].Internal { inmap[ipkg] = struct{}{} @@ -687,8 +697,8 @@ func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]string, []com var pl []string // If lens are the same, then the map must have the same contents as the // slice; no need to build a new one. - if len(inmap) == len(a.pl) { - pl = a.pl + if len(inmap) == len(a.bmi.pl) { + pl = a.bmi.pl } else { pl = make([]string, 0, len(inmap)) for pkg := range inmap { @@ -699,7 +709,7 @@ func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]string, []com // Add to the list those packages that are reached by the packages // explicitly listed in the atom - for _, pkg := range a.pl { + for _, pkg := range a.bmi.pl { // Skip ignored packages if s.rd.ir.IsIgnored(pkg) { continue @@ -792,6 +802,17 @@ func (s *solver) intersectConstraintsWithImports(deps []workingConstraint, reach } } + // Include transitive constraints, flagging them as transitive for special handling later on + for _, wc := range deps { + root := wc.Ident.ProjectRoot + if _, ok := dmap[root]; !ok { + dmap[root] = completeDep{ + workingConstraint: wc, // TODO(carolynvs): deal with overrides and package prefix-foo (not sure if that's needed?) + isTransitive: true, + } + } + } + // Dump all the deps from the map into the expected return slice cdeps := make([]completeDep, 0, len(dmap)) for _, cdep := range dmap { @@ -827,7 +848,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error var lockv Version if len(s.rd.rlm) > 0 { - lockv, err = s.getLockVersionIfValid(id) + lockv, err = s.getLockVersionIfValid(id, bmi) if err != nil { // Can only get an error here if an upgrade was expressly requested on // code that exists only in vendor @@ -844,11 +865,11 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error // TODO(sdboyer) nested loop; prime candidate for a cache somewhere for _, dep := range s.sel.getDependenciesOn(bmi.id) { // Skip the root, of course - if s.rd.isRoot(dep.depender.id.ProjectRoot) { + if s.rd.isRoot(dep.depender.a.id.ProjectRoot) { continue } - _, l, err := s.b.GetManifestAndLock(dep.depender.id, dep.depender.v, s.rd.an) + _, l, err := s.b.GetManifestAndLock(dep.depender.a.id, dep.depender.a.v, s.rd.an) if err != nil || l == nil { // err being non-nil really shouldn't be possible, but the lock // being nil is quite likely @@ -902,7 +923,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error // TODO(sdboyer) while this does work, it bypasses the interface-implied guarantees // of the version queue, and is therefore not a great strategy for API // coherency. Folding this in to a formal interface would be better. - if tc, ok := s.sel.getConstraint(bmi.id).(Revision); ok && q.pi[0] != tc { + if tc, ok := s.sel.getConstraint(bmi.id, bmi).(Revision); ok && q.pi[0] != tc { // We know this is the only thing that could possibly match, so put it // in at the front - if it isn't there already. // TODO(sdboyer) existence of the revision is guaranteed by checkRevisionExists(); restore that call. @@ -911,7 +932,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error // Having assembled the queue, search it for a valid version. s.traceCheckQueue(q, bmi, false, 1) - return q, s.findValidVersion(q, bmi.pl) + return q, s.findValidVersion(q, bmi) } // findValidVersion walks through a versionQueue until it finds a version that @@ -920,7 +941,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error // The satisfiability checks triggered from here are constrained to operate only // on those dependencies induced by the list of packages given in the second // parameter. -func (s *solver) findValidVersion(q *versionQueue, pl []string) error { +func (s *solver) findValidVersion(q *versionQueue, bmi bimodalIdentifier) error { if nil == q.current() { // this case should not be reachable, but reflects improper solver state // if it is, so panic immediately @@ -937,7 +958,7 @@ func (s *solver) findValidVersion(q *versionQueue, pl []string) error { id: q.id, v: cur, }, - pl: pl, + bmi: bmi, }, false) if err == nil { // we have a good version, can return safely @@ -954,7 +975,7 @@ func (s *solver) findValidVersion(q *versionQueue, pl []string) error { } } - s.fail(s.sel.getDependenciesOn(q.id)[0].depender.id) + s.fail(s.sel.getDependenciesOn(q.id)[0].depender.a.id) // Return a compound error of all the new errors encountered during this // attempt to find a new, valid version @@ -973,7 +994,7 @@ func (s *solver) findValidVersion(q *versionQueue, pl []string) error { // // If any of these three conditions are true (or if the id cannot be found in // the root lock), then no atom will be returned. -func (s *solver) getLockVersionIfValid(id ProjectIdentifier) (Version, error) { +func (s *solver) getLockVersionIfValid(id ProjectIdentifier, bmi bimodalIdentifier) (Version, error) { // If the project is specifically marked for changes, then don't look for a // locked version. if _, explicit := s.rd.chng[id.ProjectRoot]; explicit || s.rd.chngall { @@ -1004,7 +1025,7 @@ func (s *solver) getLockVersionIfValid(id ProjectIdentifier) (Version, error) { return nil, nil } - constraint := s.sel.getConstraint(id) + constraint := s.sel.getConstraint(id, bmi) v := lp.Version() if !constraint.Matches(v) { var found bool @@ -1082,7 +1103,7 @@ func (s *solver) backtrack(ctx context.Context) (bool, error) { } return false, err } - s.traceBacktrack(awp.bmi(), !proj) + s.traceBacktrack(awp.bmi, !proj) } } @@ -1102,7 +1123,7 @@ func (s *solver) backtrack(ctx context.Context) (bool, error) { } return false, err } - s.traceBacktrack(awp.bmi(), !proj) + s.traceBacktrack(awp.bmi, !proj) } if !q.id.eq(awp.a.id) { @@ -1113,8 +1134,8 @@ func (s *solver) backtrack(ctx context.Context) (bool, error) { // TODO(sdboyer) is it feasible to make available the failure reason here? if q.advance(nil) == nil && !q.isExhausted() { // Search for another acceptable version of this failed dep in its queue - s.traceCheckQueue(q, awp.bmi(), true, 0) - if s.findValidVersion(q, awp.pl) == nil { + s.traceCheckQueue(q, awp.bmi, true, 0) + if s.findValidVersion(q, awp.bmi) == nil { // Found one! Put it back on the selected queue and stop // backtracking @@ -1131,7 +1152,7 @@ func (s *solver) backtrack(ctx context.Context) (bool, error) { } } - s.traceBacktrack(awp.bmi(), false) + s.traceBacktrack(awp.bmi, false) // No solution found; continue backtracking after popping the queue // we just inspected off the list @@ -1251,7 +1272,7 @@ func (s *solver) selectAtom(a atomWithPackages, pkgonly bool) error { s.mtr.push("select-atom") s.unsel.remove(bimodalIdentifier{ id: a.a.id, - pl: a.pl, + pl: a.bmi.pl, }) pl, deps, err := s.getImportsAndConstraintsOf(a) @@ -1265,7 +1286,7 @@ func (s *solver) selectAtom(a atomWithPackages, pkgonly bool) error { } // Assign the new internal package list into the atom, then push it onto the // selection stack - a.pl = pl + a.bmi.pl = pl s.sel.pushSelection(a, pkgonly) // If this atom has a lock, pull it out so that we can potentially inject @@ -1307,7 +1328,11 @@ func (s *solver) selectAtom(a atomWithPackages, pkgonly bool) error { go s.b.SyncSourceFor(dep.Ident) } - s.sel.pushDep(dependency{depender: a.a, dep: dep}) + s.sel.pushDep(dependency{ + depender: a, + dep: dep, + }) + // Go through all the packages introduced on this dep, selecting only // the ones where the only depper on them is what the preceding line just // pushed in. Then, put those into the unselected queue. @@ -1335,6 +1360,7 @@ func (s *solver) selectAtom(a atomWithPackages, pkgonly bool) error { // This puts in a preferred version if one's in the map, else // drops in the zero value (nil) prefv: lmap[dep.Ident], + path: append(a.bmi.path, a.a), } heap.Push(s.unsel, bmi) } @@ -1350,7 +1376,7 @@ func (s *solver) unselectLast() (atomWithPackages, bool, error) { s.mtr.push("unselect") defer s.mtr.pop() awp, first := s.sel.popSelection() - heap.Push(s.unsel, bimodalIdentifier{id: awp.a.id, pl: awp.pl}) + heap.Push(s.unsel, bimodalIdentifier{id: awp.a.id, pl: awp.bmi.pl, path: awp.bmi.path}) _, deps, err := s.getImportsAndConstraintsOf(awp) if err != nil { diff --git a/gps/trace.go b/gps/trace.go index 4c579d30aa..bfd0bf6b7a 100644 --- a/gps/trace.go +++ b/gps/trace.go @@ -140,9 +140,9 @@ func (s *solver) traceSelect(awp atomWithPackages, pkgonly bool) { var msg string if pkgonly { - msg = fmt.Sprintf("%s%s include %v more pkgs from %s", innerIndent, successChar, len(awp.pl), a2vs(awp.a)) + msg = fmt.Sprintf("%s%s include %v more pkgs from %s", innerIndent, successChar, len(awp.bmi.pl), a2vs(awp.a)) } else { - msg = fmt.Sprintf("%s select %s w/%v pkgs", successChar, a2vs(awp.a), len(awp.pl)) + msg = fmt.Sprintf("%s select %s w/%v pkgs", successChar, a2vs(awp.a), len(awp.bmi.pl)) } prefix := getprei(len(s.sel.projects) - 1)