Skip to content

Commit

Permalink
Remove Manifest.Name() method
Browse files Browse the repository at this point in the history
It seems to have been redundant in all cases, and just a source for
potential conflict errors. Fixes golang#59.
  • Loading branch information
sdboyer committed Jul 9, 2016
1 parent ecb6bf6 commit 6fcc2a9
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 49 deletions.
4 changes: 2 additions & 2 deletions analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
}
Expand Down
2 changes: 1 addition & 1 deletion manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
42 changes: 17 additions & 25 deletions manifest.go
Original file line number Diff line number Diff line change
@@ -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
}

Expand All @@ -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
Expand All @@ -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)),
}
Expand Down
2 changes: 1 addition & 1 deletion project_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
5 changes: 0 additions & 5 deletions solve_basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions solve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down
10 changes: 5 additions & 5 deletions solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 6fcc2a9

Please sign in to comment.