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

Commit

Permalink
Make ProjectAnalyzer a solver param, not the sm
Browse files Browse the repository at this point in the history
This was really always the intended model - there's no reason a
SourceManager needs to be permanently coupled with just one analyzer.
It's perfectly sufficient to provide one as an argument to the relevant
methods.

Fixes #195.
  • Loading branch information
sdboyer committed Apr 1, 2017
1 parent 5a6e64c commit 38bedf4
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 64 deletions.
8 changes: 2 additions & 6 deletions bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,17 @@ var mkBridge = func(s *solver, sm SourceManager, down bool) sourceBridge {
}
}

func (b *bridge) GetManifestAndLock(id ProjectIdentifier, v Version) (Manifest, Lock, error) {
func (b *bridge) GetManifestAndLock(id ProjectIdentifier, v Version, an ProjectAnalyzer) (Manifest, Lock, error) {
if b.s.rd.isRoot(id.ProjectRoot) {
return b.s.rd.rm, b.s.rd.rl, nil
}

b.s.mtr.push("b-gmal")
m, l, e := b.sm.GetManifestAndLock(id, v)
m, l, e := b.sm.GetManifestAndLock(id, v, an)
b.s.mtr.pop()
return m, l, e
}

func (b *bridge) AnalyzerInfo() (string, int) {
return b.sm.AnalyzerInfo()
}

func (b *bridge) ListVersions(id ProjectIdentifier) ([]Version, error) {
if vl, exists := b.vlists[id]; exists {
return vl, nil
Expand Down
2 changes: 1 addition & 1 deletion hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (s *solver) writeHashingInputs(w io.Writer) {
}

writeString(hhAnalyzer)
an, av := s.b.AnalyzerInfo()
an, av := s.rd.an.Info()
writeString(an)
writeString(strconv.Itoa(av))
}
Expand Down
31 changes: 17 additions & 14 deletions hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ func TestHashInputs(t *testing.T) {
RootDir: string(fix.ds[0].n),
RootPackageTree: fix.rootTree(),
Manifest: fix.rootmanifest(),
ProjectAnalyzer: naiveAnalyzer{},
}

s, err := Prepare(params, newdepspecSM(fix.ds, nil))
Expand All @@ -39,7 +40,7 @@ func TestHashInputs(t *testing.T) {
hhIgnores,
hhOverrides,
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
}
for _, v := range elems {
Expand Down Expand Up @@ -67,6 +68,7 @@ func TestHashInputsReqsIgs(t *testing.T) {
RootDir: string(fix.ds[0].n),
RootPackageTree: fix.rootTree(),
Manifest: rm,
ProjectAnalyzer: naiveAnalyzer{},
}

s, err := Prepare(params, newdepspecSM(fix.ds, nil))
Expand All @@ -92,7 +94,7 @@ func TestHashInputsReqsIgs(t *testing.T) {
"foo",
hhOverrides,
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
}
for _, v := range elems {
Expand Down Expand Up @@ -137,7 +139,7 @@ func TestHashInputsReqsIgs(t *testing.T) {
"foo",
hhOverrides,
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
}
for _, v := range elems {
Expand Down Expand Up @@ -176,7 +178,7 @@ func TestHashInputsReqsIgs(t *testing.T) {
hhIgnores,
hhOverrides,
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
}
for _, v := range elems {
Expand All @@ -198,6 +200,7 @@ func TestHashInputsOverrides(t *testing.T) {
RootDir: string(basefix.ds[0].n),
RootPackageTree: basefix.rootTree(),
Manifest: rm,
ProjectAnalyzer: naiveAnalyzer{},
}

table := []struct {
Expand Down Expand Up @@ -231,7 +234,7 @@ func TestHashInputsOverrides(t *testing.T) {
"c",
"car",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -262,7 +265,7 @@ func TestHashInputsOverrides(t *testing.T) {
"c",
"car",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -292,7 +295,7 @@ func TestHashInputsOverrides(t *testing.T) {
"c",
"car",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -320,7 +323,7 @@ func TestHashInputsOverrides(t *testing.T) {
"c",
"car",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -352,7 +355,7 @@ func TestHashInputsOverrides(t *testing.T) {
"d",
"b-foobranch",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -380,7 +383,7 @@ func TestHashInputsOverrides(t *testing.T) {
"d",
"b-foobranch",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -410,7 +413,7 @@ func TestHashInputsOverrides(t *testing.T) {
"d",
"b-foobranch",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -441,7 +444,7 @@ func TestHashInputsOverrides(t *testing.T) {
"d",
"b-foobranch",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -473,7 +476,7 @@ func TestHashInputsOverrides(t *testing.T) {
"d",
"b-foobranch",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down Expand Up @@ -507,7 +510,7 @@ func TestHashInputsOverrides(t *testing.T) {
"d",
"b-foobranch",
hhAnalyzer,
"depspec-sm-builtin",
"naive-analyzer",
"1",
},
},
Expand Down
18 changes: 9 additions & 9 deletions manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func mkNaiveSM(t *testing.T) (*SourceMgr, func()) {
t.FailNow()
}

sm, err := NewSourceManager(naiveAnalyzer{}, cpath)
sm, err := NewSourceManager(cpath)
if err != nil {
t.Errorf("Unexpected error on SourceManager creation: %s", err)
t.FailNow()
Expand All @@ -66,7 +66,7 @@ func remakeNaiveSM(osm *SourceMgr, t *testing.T) (*SourceMgr, func()) {
cpath := osm.cachedir
osm.Release()

sm, err := NewSourceManager(naiveAnalyzer{}, cpath)
sm, err := NewSourceManager(cpath)
if err != nil {
t.Errorf("unexpected error on SourceManager recreation: %s", err)
t.FailNow()
Expand All @@ -91,13 +91,13 @@ func TestSourceManagerInit(t *testing.T) {
if err != nil {
t.Errorf("Failed to create temp dir: %s", err)
}
sm, err := NewSourceManager(naiveAnalyzer{}, cpath)
sm, err := NewSourceManager(cpath)

if err != nil {
t.Errorf("Unexpected error on SourceManager creation: %s", err)
}

_, err = NewSourceManager(naiveAnalyzer{}, cpath)
_, err = NewSourceManager(cpath)
if err == nil {
t.Errorf("Creating second SourceManager should have failed due to file lock contention")
} else if te, ok := err.(CouldNotCreateLockError); !ok {
Expand All @@ -120,7 +120,7 @@ func TestSourceManagerInit(t *testing.T) {
}

// Set another one up at the same spot now, just to be sure
sm, err = NewSourceManager(naiveAnalyzer{}, cpath)
sm, err = NewSourceManager(cpath)
if err != nil {
t.Errorf("Creating a second SourceManager should have succeeded when the first was released, but failed with err %s", err)
}
Expand All @@ -144,7 +144,7 @@ func TestSourceInit(t *testing.T) {
t.FailNow()
}

sm, err := NewSourceManager(naiveAnalyzer{}, cpath)
sm, err := NewSourceManager(cpath)
if err != nil {
t.Errorf("Unexpected error on SourceManager creation: %s", err)
t.FailNow()
Expand Down Expand Up @@ -338,7 +338,7 @@ func TestMgrMethodsFailWithBadPath(t *testing.T) {
if _, err = sm.ListPackages(bad, nil); err == nil {
t.Error("ListPackages() did not error on bad input")
}
if _, _, err = sm.GetManifestAndLock(bad, nil); err == nil {
if _, _, err = sm.GetManifestAndLock(bad, nil, naiveAnalyzer{}); err == nil {
t.Error("GetManifestAndLock() did not error on bad input")
}
if err = sm.ExportProject(bad, nil, ""); err == nil {
Expand Down Expand Up @@ -426,7 +426,7 @@ func TestGetInfoListVersionsOrdering(t *testing.T) {

id := mkPI("github.com/sdboyer/gpkt").normalize()

_, _, err := sm.GetManifestAndLock(id, NewVersion("v1.0.0"))
_, _, err := sm.GetManifestAndLock(id, NewVersion("v1.0.0"), naiveAnalyzer{})
if err != nil {
t.Errorf("Unexpected error from GetInfoAt %s", err)
}
Expand Down Expand Up @@ -689,7 +689,7 @@ func TestErrAfterRelease(t *testing.T) {
t.Errorf("ListPackages errored after Release(), but with unexpected error: %T %s", terr, terr.Error())
}

_, _, err = sm.GetManifestAndLock(id, nil)
_, _, err = sm.GetManifestAndLock(id, nil, naiveAnalyzer{})
if err == nil {
t.Errorf("GetManifestAndLock did not error after calling Release()")
} else if terr, ok := err.(smIsReleased); !ok {
Expand Down
2 changes: 1 addition & 1 deletion result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func BenchmarkCreateVendorTree(b *testing.B) {
tmp := path.Join(os.TempDir(), "vsolvtest")

clean := true
sm, err := NewSourceManager(naiveAnalyzer{}, path.Join(tmp, "cache"))
sm, err := NewSourceManager(path.Join(tmp, "cache"))
if err != nil {
b.Errorf("NewSourceManager errored unexpectedly: %q", err)
clean = false
Expand Down
3 changes: 3 additions & 0 deletions rootdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ type rootdata struct {

// A defensively copied instance of params.RootPackageTree
rpt pkgtree.PackageTree

// The ProjectAnalyzer to use for all GetManifestAndLock calls.
an ProjectAnalyzer
}

// externalImportList returns a list of the unique imports from the root data.
Expand Down
2 changes: 2 additions & 0 deletions rootdata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ func TestRootdataExternalImports(t *testing.T) {
RootDir: string(fix.ds[0].n),
RootPackageTree: fix.rootTree(),
Manifest: fix.rootmanifest(),
ProjectAnalyzer: naiveAnalyzer{},
}

is, err := Prepare(params, newdepspecSM(fix.ds, nil))
Expand Down Expand Up @@ -65,6 +66,7 @@ func TestGetApplicableConstraints(t *testing.T) {
RootDir: string(fix.ds[0].n),
RootPackageTree: fix.rootTree(),
Manifest: fix.rootmanifest(),
ProjectAnalyzer: naiveAnalyzer{},
}

is, err := Prepare(params, newdepspecSM(fix.ds, nil))
Expand Down
2 changes: 1 addition & 1 deletion solve_basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,7 @@ func newdepspecSM(ds []depspec, ignore []string) *depspecSourceManager {
}
}

func (sm *depspecSourceManager) GetManifestAndLock(id ProjectIdentifier, v Version) (Manifest, Lock, error) {
func (sm *depspecSourceManager) GetManifestAndLock(id ProjectIdentifier, v Version, an ProjectAnalyzer) (Manifest, Lock, error) {
// If the input version is a PairedVersion, look only at its top version,
// not the underlying. This is generally consistent with the idea that, for
// this class of lookup, the rev probably DOES exist, but upstream changed
Expand Down
2 changes: 1 addition & 1 deletion solve_bimodal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ func (sm *bmSourceManager) ListPackages(id ProjectIdentifier, v Version) (pkgtre
return pkgtree.PackageTree{}, fmt.Errorf("Project %s at version %s could not be found", id.errString(), v)
}

func (sm *bmSourceManager) GetManifestAndLock(id ProjectIdentifier, v Version) (Manifest, Lock, error) {
func (sm *bmSourceManager) GetManifestAndLock(id ProjectIdentifier, v Version, an ProjectAnalyzer) (Manifest, Lock, error) {
for _, ds := range sm.specs {
if id.normalizedSource() == string(ds.n) && v.Matches(ds.v) {
if l, exists := sm.lm[id.normalizedSource()+" "+v.String()]; exists {
Expand Down
11 changes: 11 additions & 0 deletions solve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func solveBasicsAndCheck(fix basicFixture, t *testing.T) (res Solution, err erro
Downgrade: fix.downgrade,
ChangeAll: fix.changeall,
ToChange: fix.changelist,
ProjectAnalyzer: naiveAnalyzer{},
}

if fix.l != nil {
Expand Down Expand Up @@ -153,6 +154,7 @@ func solveBimodalAndCheck(fix bimodalFixture, t *testing.T) (res Solution, err e
Lock: dummyLock{},
Downgrade: fix.downgrade,
ChangeAll: fix.changeall,
ProjectAnalyzer: naiveAnalyzer{},
}

if fix.l != nil {
Expand Down Expand Up @@ -284,6 +286,7 @@ func TestRootLockNoVersionPairMatching(t *testing.T) {
RootPackageTree: fix.rootTree(),
Manifest: fix.rootmanifest(),
Lock: l2,
ProjectAnalyzer: naiveAnalyzer{},
}

res, err := fixSolve(params, sm)
Expand All @@ -308,6 +311,14 @@ func TestBadSolveOpts(t *testing.T) {
t.Error("Prepare should have given error on nil SourceManager, but gave:", err)
}

_, err = Prepare(params, sm)
if err == nil {
t.Errorf("Prepare should have errored without ProjectAnalyzer")
} else if !strings.Contains(err.Error(), "must provide a ProjectAnalyzer") {
t.Error("Prepare should have given error without ProjectAnalyzer, but gave:", err)
}

params.ProjectAnalyzer = naiveAnalyzer{}
_, err = Prepare(params, sm)
if err == nil {
t.Errorf("Prepare should have errored on empty root")
Expand Down
Loading

0 comments on commit 38bedf4

Please sign in to comment.