From 6fcc2a9a27e06ccddfcf1c309f547c40f1a9cb35 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Fri, 8 Jul 2016 20:51:20 -0400 Subject: [PATCH] Remove Manifest.Name() method It seems to have been redundant in all cases, and just a source for potential conflict errors. Fixes #59. --- analysis.go | 4 ++-- hash_test.go | 4 ++-- manager_test.go | 2 +- manifest.go | 42 +++++++++++++++++------------------------- project_manager.go | 2 +- solve_basic_test.go | 5 ----- solve_test.go | 16 ++++++++-------- solver.go | 10 +++++----- 8 files changed, 36 insertions(+), 49 deletions(-) diff --git a/analysis.go b/analysis.go index eca422ca7f..331b720aff 100644 --- a/analysis.go +++ b/analysis.go @@ -81,8 +81,8 @@ func listPackages(fileRoot, importRoot string) (PackageTree, error) { Packages: make(map[string]PackageOrErr), } - // mkfilter returns two funcs that can be injected into a - // build.Context, letting us filter the results into an "in" and "out" set. + // mkfilter returns two funcs that can be injected into a build.Context, + // letting us filter the results into an "in" and "out" set. mkfilter := func(files map[string]struct{}) (in, out func(dir string) (fi []os.FileInfo, err error)) { in = func(dir string) (fi []os.FileInfo, err error) { all, err := ioutil.ReadDir(dir) diff --git a/hash_test.go b/hash_test.go index eac0e7d3c2..09907202e8 100644 --- a/hash_test.go +++ b/hash_test.go @@ -10,8 +10,8 @@ func TestHashInputs(t *testing.T) { fix := basicFixtures[2] args := SolveArgs{ - RootDir: string(fix.ds[0].Name()), - ImportRoot: fix.ds[0].Name(), + RootDir: string(fix.ds[0].n), + ImportRoot: fix.ds[0].n, Manifest: fix.ds[0], Ignore: []string{"foo", "bar"}, } diff --git a/manager_test.go b/manager_test.go index fd5fcf6cb9..17b6336e2c 100644 --- a/manager_test.go +++ b/manager_test.go @@ -18,7 +18,7 @@ var bd string type dummyAnalyzer struct{} func (dummyAnalyzer) GetInfo(ctx build.Context, p ProjectName) (Manifest, Lock, error) { - return SimpleManifest{N: p}, nil, nil + return SimpleManifest{}, nil, nil } func sv(s string) *semver.Version { diff --git a/manifest.go b/manifest.go index 51dac26782..d685525403 100644 --- a/manifest.go +++ b/manifest.go @@ -1,21 +1,24 @@ package vsolver -// Manifest represents the data from a manifest file (or however the -// implementing tool chooses to store it) at a particular version that is -// relevant to the satisfiability solving process. That means constraints on -// dependencies, both for normal dependencies and for tests. +// Manifest represents manifest-type data for a project at a particular version. +// That means dependency constraints, both for normal dependencies and for +// tests. The constraints expressed in a manifest determine the set of versions that +// are acceptable to try for a given project. // -// Finding a solution that satisfies the constraints expressed by all of these -// dependencies (and those from all other projects, transitively), is what the -// solver does. +// Expressing a constraint in a manifest does not guarantee that a particular +// dependency will be present. It only guarantees that if packages in the +// project specified by the dependency are discovered through static analysis of +// the (transitive) import graph, then they will conform to the constraint. // -// Note that vsolver does perform static analysis on all projects' codebases; -// if dependencies it finds through that analysis are missing from what the -// Manifest lists, it is considered an error that will eliminate that version -// from consideration in the solving algorithm. +// This does entail that manifests can express constraints on projects they do +// not themselves import. This is by design, but its implications are complex. +// See the gps docs for more information: https://github.com/sdboyer/gps/wiki type Manifest interface { - Name() ProjectName + // Returns a list of project constraints that will be universally to + // the depgraph. DependencyConstraints() []ProjectDep + // Returns a list of constraints applicable to test imports. Note that this + // will only be consulted for root manifests. TestDependencyConstraints() []ProjectDep } @@ -24,18 +27,12 @@ type Manifest interface { // the fly for projects with no manifest metadata, or metadata through a foreign // tool's idioms. type SimpleManifest struct { - N ProjectName Deps []ProjectDep TestDeps []ProjectDep } var _ Manifest = SimpleManifest{} -// Name returns the name of the project described by the manifest. -func (m SimpleManifest) Name() ProjectName { - return m.N -} - // GetDependencies returns the project's dependencies. func (m SimpleManifest) DependencyConstraints() []ProjectDep { return m.Deps @@ -55,20 +52,15 @@ func (m SimpleManifest) TestDependencyConstraints() []ProjectDep { // the solver is in-flight. // // This is achieved by copying the manifest's data into a new SimpleManifest. -func prepManifest(m Manifest, n ProjectName) Manifest { +func prepManifest(m Manifest) Manifest { if m == nil { - // Only use the provided ProjectName if making an empty manifest; - // otherwise, we trust the input manifest. - return SimpleManifest{ - N: n, - } + return SimpleManifest{} } deps := m.DependencyConstraints() ddeps := m.TestDependencyConstraints() rm := SimpleManifest{ - N: m.Name(), Deps: make([]ProjectDep, len(deps)), TestDeps: make([]ProjectDep, len(ddeps)), } diff --git a/project_manager.go b/project_manager.go index 2f5426a50f..778671b170 100644 --- a/project_manager.go +++ b/project_manager.go @@ -128,7 +128,7 @@ func (pm *projectManager) GetInfoAt(v Version) (Manifest, Lock, error) { // If m is nil, prepManifest will provide an empty one. pi := projectInfo{ - Manifest: prepManifest(m, pm.n), + Manifest: prepManifest(m), Lock: l, } diff --git a/solve_basic_test.go b/solve_basic_test.go index 9906edd1f9..1b6433a13c 100644 --- a/solve_basic_test.go +++ b/solve_basic_test.go @@ -1163,11 +1163,6 @@ func (ds depspec) TestDependencyConstraints() []ProjectDep { return ds.devdeps } -// impl Spec interface -func (ds depspec) Name() ProjectName { - return ds.n -} - type fixLock []LockedProject func (fixLock) SolverVersion() string { diff --git a/solve_test.go b/solve_test.go index c8d74dd722..9b33d224dc 100644 --- a/solve_test.go +++ b/solve_test.go @@ -63,8 +63,8 @@ func solveBasicsAndCheck(fix basicFixture, t *testing.T) (res Solution, err erro sm := newdepspecSM(fix.ds, nil) args := SolveArgs{ - RootDir: string(fix.ds[0].Name()), - ImportRoot: ProjectName(fix.ds[0].Name()), + RootDir: string(fix.ds[0].n), + ImportRoot: ProjectName(fix.ds[0].n), Manifest: fix.ds[0], Lock: dummyLock{}, } @@ -116,8 +116,8 @@ func solveBimodalAndCheck(fix bimodalFixture, t *testing.T) (res Solution, err e sm := newbmSM(fix) args := SolveArgs{ - RootDir: string(fix.ds[0].Name()), - ImportRoot: ProjectName(fix.ds[0].Name()), + RootDir: string(fix.ds[0].n), + ImportRoot: ProjectName(fix.ds[0].n), Manifest: fix.ds[0], Lock: dummyLock{}, Ignore: fix.ignore, @@ -274,8 +274,8 @@ func TestRootLockNoVersionPairMatching(t *testing.T) { l2[0].v = nil args := SolveArgs{ - RootDir: string(fix.ds[0].Name()), - ImportRoot: ProjectName(fix.ds[0].Name()), + RootDir: string(fix.ds[0].n), + ImportRoot: ProjectName(fix.ds[0].n), Manifest: fix.ds[0], Lock: l2, } @@ -370,8 +370,8 @@ func TestIgnoreDedupe(t *testing.T) { ig := []string{"foo", "foo", "bar"} args := SolveArgs{ - RootDir: string(fix.ds[0].Name()), - ImportRoot: ProjectName(fix.ds[0].Name()), + RootDir: string(fix.ds[0].n), + ImportRoot: ProjectName(fix.ds[0].n), Manifest: fix.ds[0], Ignore: ig, } diff --git a/solver.go b/solver.go index 3c614ddbbb..b1d2285ca7 100644 --- a/solver.go +++ b/solver.go @@ -246,7 +246,7 @@ func (s *solver) Solve() (Solution, error) { } // Prep safe, normalized versions of root manifest and lock data - s.rm = prepManifest(s.args.Manifest, s.args.ImportRoot) + s.rm = prepManifest(s.args.Manifest) if s.args.Lock != nil { for _, lp := range s.args.Lock.Projects() { s.rlm[lp.Ident().normalize()] = lp @@ -461,7 +461,7 @@ func (s *solver) selectRoot() error { func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]completeDep, error) { var err error - if s.rm.Name() == a.a.id.LocalName { + if s.args.ImportRoot == a.a.id.LocalName { panic("Should never need to recheck imports/constraints from root during solve") } @@ -609,7 +609,7 @@ func (s *solver) intersectConstraintsWithImports(deps []ProjectDep, reach []stri func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error) { id := bmi.id // If on the root package, there's no queue to make - if id.LocalName == s.rm.Name() { + if s.args.ImportRoot == id.LocalName { return newVersionQueue(id, nil, nil, s.b) } @@ -649,7 +649,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error // TODO nested loop; prime candidate for a cache somewhere for _, dep := range s.sel.getDependenciesOn(bmi.id) { // Skip the root, of course - if dep.depender.id.LocalName == s.rm.Name() { + if s.args.ImportRoot == dep.depender.id.LocalName { continue } @@ -1003,7 +1003,7 @@ func (s *solver) fail(id ProjectIdentifier) { // selection? // skip if the root project - if s.rm.Name() != id.LocalName { + if s.args.ImportRoot != id.LocalName { // just look for the first (oldest) one; the backtracker will necessarily // traverse through and pop off any earlier ones for _, vq := range s.versions {