Skip to content
This repository has been archived by the owner on Feb 3, 2018. It is now read-only.

Introduce 'requires' #114

Merged
merged 6 commits into from
Dec 13, 2016
Merged
Show file tree
Hide file tree
Changes from 2 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
24 changes: 23 additions & 1 deletion manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,22 @@ type RootManifest interface {
// paths can be within the root project, or part of other projects. Ignoring
// a package means that both it and its (unique) imports will be disregarded
// by all relevant solver operations.
//
// It is an error to include a package in both the ignored and required
// sets.
IgnoredPackages() map[string]bool

// RequiredPackages returns a set of import paths to require. These packages
// are required to be present in any solution. The list can include main
// packages.
//
// It is meaningless to specify packages that are within the
// PackageTree of the ProjectRoot (though not an error, because the
// RootManifest itself does not report a ProjectRoot).
//
// It is an error to include a package in both the ignored and required
// sets.
RequiredPackages() map[string]bool
}

// SimpleManifest is a helper for tools to enumerate manifest data. It's
Expand Down Expand Up @@ -72,7 +87,7 @@ func (m SimpleManifest) TestDependencyConstraints() ProjectConstraints {
// Also, for tests.
type simpleRootManifest struct {
c, tc, ovr ProjectConstraints
ig map[string]bool
ig, req map[string]bool
}

func (m simpleRootManifest) DependencyConstraints() ProjectConstraints {
Expand All @@ -87,12 +102,16 @@ func (m simpleRootManifest) Overrides() ProjectConstraints {
func (m simpleRootManifest) IgnoredPackages() map[string]bool {
return m.ig
}
func (m simpleRootManifest) RequiredPackages() map[string]bool {
return m.req
}
func (m simpleRootManifest) dup() simpleRootManifest {
m2 := simpleRootManifest{
c: make(ProjectConstraints, len(m.c)),
tc: make(ProjectConstraints, len(m.tc)),
ovr: make(ProjectConstraints, len(m.ovr)),
ig: make(map[string]bool, len(m.ig)),
req: make(map[string]bool, len(m.req)),
}

for k, v := range m.c {
Expand All @@ -107,6 +126,9 @@ func (m simpleRootManifest) dup() simpleRootManifest {
for k, v := range m.ig {
m2.ig[k] = v
}
for k, v := range m.req {
m2.req[k] = v
}

return m2
}
Expand Down
93 changes: 93 additions & 0 deletions solve_bimodal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,97 @@ var bimodalFixtures = map[string]bimodalFixture{
"bar from baz 1.0.0",
),
},
"require package": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0", "bar 1.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0"),
pkg("foo", "bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
dsp(mkDepspec("baz 1.0.0"),
pkg("baz")),
},
require: []string{"baz"},
r: mksolution(
"foo 1.0.0",
"bar 1.0.0",
"baz 1.0.0",
),
},
"require subpackage": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0", "bar 1.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0"),
pkg("foo", "bar")),
dsp(mkDepspec("bar 1.0.0"),
pkg("bar")),
dsp(mkDepspec("baz 1.0.0"),
pkg("baz", "baz/qux"),
pkg("baz/qux")),
},
require: []string{"baz/qux"},
r: mksolution(
"foo 1.0.0",
"bar 1.0.0",
mklp("baz 1.0.0", "baz/qux"),
),
},
"require impossible subpackage": {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This brings us back to another variant of a familiar question - if an unimported dep is constrained in the manifest, does that constraint still come into effect?

I think it's pretty clear in this case, though: having the requires list is basically equivalent to having an import path, and these are both emerging from the same manifest (so we can assume the same user has control over it), thus the constraint should be applied. And because deps can't express requires, we don't have the same transitivity problem as with normal imports.

ds: []depspec{
dsp(mkDepspec("root 0.0.0", "baz 1.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0"),
pkg("foo")),
dsp(mkDepspec("baz 1.0.0"),
pkg("baz")),
dsp(mkDepspec("baz 2.0.0"),
pkg("baz", "baz/qux"),
pkg("baz/qux")),
},
require: []string{"baz/qux"},
fail: &noVersionError{
pn: mkPI("baz"),
//fails: , // TODO new fail type for failed require
},
},
"require subpkg conflicts with other dep constraint": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0", "baz 1.0.0"),
pkg("foo", "baz")),
dsp(mkDepspec("baz 1.0.0"),
pkg("baz")),
dsp(mkDepspec("baz 2.0.0"),
pkg("baz", "baz/qux"),
pkg("baz/qux")),
},
require: []string{"baz/qux"},
fail: &noVersionError{
pn: mkPI("baz"),
//fails: , // TODO new fail type for failed require
},
},
"require independent subpkg conflicts with other dep constraint": {
ds: []depspec{
dsp(mkDepspec("root 0.0.0"),
pkg("root", "foo")),
dsp(mkDepspec("foo 1.0.0", "baz 1.0.0"),
pkg("foo", "baz")),
dsp(mkDepspec("baz 1.0.0"),
pkg("baz")),
dsp(mkDepspec("baz 2.0.0"),
pkg("baz"),
pkg("baz/qux")),
},
require: []string{"baz/qux"},
fail: &noVersionError{
pn: mkPI("baz"),
//fails: , // TODO new fail type for failed require
},
},
}

// tpkg is a representation of a single package. It has its own import path, as
Expand Down Expand Up @@ -783,6 +874,8 @@ type bimodalFixture struct {
changeall bool
// pkgs to ignore
ignore []string
// pkgs to require
require []string
}

func (f bimodalFixture) name() string {
Expand Down
30 changes: 23 additions & 7 deletions solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,12 @@ type solver struct {
// removal.
unsel *unselected

// Map of packages to ignore. Derived by converting SolveParameters.Ignore
// into a map during solver prep - which also, nicely, deduplicates it.
// Map of packages to ignore.
ig map[string]bool

// Map of packages to require.
req map[string]bool

// A stack of all the currently active versionQueues in the solver. The set
// of projects represented here corresponds closely to what's in s.sel,
// although s.sel will always contain the root project, and s.vqs never
Expand Down Expand Up @@ -216,11 +218,29 @@ func Prepare(params SolveParameters, sm SourceManager) (Solver, error) {
s := &solver{
params: params,
ig: params.Manifest.IgnoredPackages(),
req: params.Manifest.RequiredPackages(),
ovr: params.Manifest.Overrides(),
tl: params.TraceLogger,
rpt: params.RootPackageTree.dup(),
}

if len(s.ig) != 0 {
var both []string
for pkg := range params.Manifest.RequiredPackages() {
if s.ig[pkg] {
both = append(both, pkg)
}
}
switch len(both) {
case 0:
break
case 1:
return nil, badOptsFailure(fmt.Sprintf("%q was given as both a required and ignored package", both[0]))
default:
return nil, badOptsFailure(fmt.Sprintf("multiple packages given as both required and ignored: %q", strings.Join(both, "\", \"")))
}
}

// Ensure the ignore and overrides maps are at least initialized
if s.ig == nil {
s.ig = make(map[string]bool)
Expand Down Expand Up @@ -481,11 +501,7 @@ func (s *solver) selectRoot() error {
// If we're looking for root's deps, get it from opts and local root
// analysis, rather than having the sm do it
mdeps := s.ovr.overrideAll(s.rm.DependencyConstraints().merge(s.rm.TestDependencyConstraints()))

// Err is not possible at this point, as it could only come from
// listPackages(), which if we're here already succeeded for root
reach := s.rpt.ExternalReach(true, true, s.ig).ListExternalImports()

deps, err := s.intersectConstraintsWithImports(mdeps, reach)
if err != nil {
// TODO(sdboyer) this could well happen; handle it with a more graceful error
Expand Down Expand Up @@ -661,7 +677,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error
// Project exists only in vendor (and in some manifest somewhere)
// TODO(sdboyer) mark this for special handling, somehow?
} else {
return nil, fmt.Errorf("Project '%s' could not be located.", id)
return nil, fmt.Errorf("project '%s' could not be located", id)
}
}

Expand Down