Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Allow transitive constraints #1735

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"commands": [
["ensure"]
],
"vendor-final": [
"github.com/carolynvs/deptest-transcons-b",
"github.com/carolynvs/deptest-transcons-c"
]
}
Original file line number Diff line number Diff line change
@@ -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.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

[[constraint]]
name = "github.com/carolynvs/deptest-transcons-a"
version = "1.0.0"

[prune]
go-tests = true
unused-packages = true
Original file line number Diff line number Diff line change
@@ -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()
}
Original file line number Diff line number Diff line change
@@ -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"
]
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
PROJECT CONSTRAINT VERSION REVISION LATEST PKGS USED
github.com/sdboyer/deptest ^0.8.0 v0.8.0 ff2948a v0.8.1 1
github.com/sdboyer/deptestdos v2.0.0 v2.0.0 5c60720 v2.0.0 1
github.com/sdboyer/deptest ^0.8.0 v0.8.0 ff2948a v0.8.1 1
github.com/sdboyer/deptestdos v2.0.0 v2.0.0 5c60720 v2.0.0 1
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
PROJECT CONSTRAINT REVISION LATEST
github.com/carolynvs/go-dep-test ^0.1.0 b9c5511 4069198
github.com/carolynvs/go-dep-test ^0.1.0 b9c5511 4069198
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
PROJECT CONSTRAINT VERSION REVISION LATEST PKGS USED
github.com/sdboyer/deptest v0.8.1 (override) v0.8.1 3f4c3be v0.8.1 1
github.com/sdboyer/deptestdos v2.0.0 v2.0.0 5c60720 v2.0.0 1
github.com/sdboyer/deptest v0.8.1 (override) v0.8.1 3f4c3be v0.8.1 1
github.com/sdboyer/deptestdos v2.0.0 v2.0.0 5c60720 v2.0.0 1
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
PROJECT CONSTRAINT VERSION REVISION LATEST PKGS USED
github.com/sdboyer/deptest v1.0.0 v1.0.0 ff2948a v1.0.0 1
github.com/sdboyer/deptestdos a0196ba a0196ba 1
github.com/sdboyer/deptest v1.0.0 v1.0.0 ff2948a v1.0.0 1
github.com/sdboyer/deptestdos a0196ba a0196ba 1
2 changes: 2 additions & 0 deletions gps/identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ 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
Expand Down
79 changes: 76 additions & 3 deletions gps/solve_bimodal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,78 @@ var bimodalFixtures = map[string]bimodalFixture{
"b 1.0.0",
),
},
"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",
),
},
"backtrack drops transitive constraint": {
ds: []depspec{
dsp(mkDepspec("root 1.0.0", "foo ^1.0.0"),
pkg("root", "foo"),
),
dsp(mkDepspec("foo 1.0.1", "bar =1.0.1", "baz =1.0.1"), // This transitive constraint should not be applied in the final solution
pkg("foo", "bar"),
),
dsp(mkDepspec("foo 1.0.0", "bar =1.0.0"),
pkg("foo", "bar"),
),
dsp(mkDepspec("bar 1.0.1", "baz =1.0.0-notexist"), // This should trigger a backtrack
pkg("bar", "baz"),
),
dsp(mkDepspec("bar 1.0.0", "baz =1.0.0"), // This will conflict with the transitive constraint if it's not dropped during backtracking
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",
),
},
"backtrack adds transitive constraint": {
ds: []depspec{
dsp(mkDepspec("root 1.0.0", "foo ^1.0.0"),
pkg("root", "foo"),
),
dsp(mkDepspec("foo 1.0.1", "bar =1.0.1"),
pkg("foo", "bar"),
),
dsp(mkDepspec("foo 1.0.0", "bar =1.0.0", "baz =1.0.0"), // This transitive constraint should be applied in the final solution
pkg("foo", "bar"),
),
dsp(mkDepspec("bar 1.0.1", "baz =1.0.0-notexist"), // This should trigger a backtrack
pkg("bar", "baz"),
),
dsp(mkDepspec("bar 1.0.0", "baz ^1.0.0"), // Normally this would pick baz 1.0.1 but the transitive constraint should force an older version
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",
),
},
// Constraints apply only if the project that declares them has a
// reachable import
"constraints activated by import": {
Expand All @@ -152,12 +224,13 @@ 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": {
/* TODO(carolynvs): This was broken by supporting transitive constraints. It will be fixed in a follow-up PR where gps tracks which package introduced a constraint.
"constraints activated by import, non-root": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "root/foo", "b"),
Expand All @@ -177,7 +250,7 @@ var bimodalFixtures = map[string]bimodalFixture{
"a 1.0.0",
"b 1.1.0",
),
},
},*/
// Import jump is in a dep, and points to a transitive dep - but only in not
// the first version we try
"transitive bm-add on older version": {
Expand Down
18 changes: 18 additions & 0 deletions gps/solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,13 @@ func (s *solver) selectRoot() error {
}

s.sel.pushDep(dependency{depender: awp.a, 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})
}
Expand Down Expand Up @@ -780,6 +787,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 {
Expand Down