From 6109ef157907235b2754d7a29fea78368e500fe2 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Wed, 11 Jan 2017 20:50:01 -0500 Subject: [PATCH 1/4] Use an io.Writer to write hashing inputs This provides a convenient way of letting the debugging func inject a newline after each write (for readability in debugging). --- hash.go | 70 +++++++++++++++++++++++++++++++++------------------- hash_test.go | 20 +++++++-------- 2 files changed, 54 insertions(+), 36 deletions(-) diff --git a/hash.go b/hash.go index 5dc5166..b412993 100644 --- a/hash.go +++ b/hash.go @@ -3,6 +3,7 @@ package gps import ( "bytes" "crypto/sha256" + "io" "sort" ) @@ -17,30 +18,38 @@ import ( // // (Basically, this is for memoization.) func (s *solver) HashInputs() (digest []byte) { - buf := new(bytes.Buffer) - s.writeHashingInputs(buf) + h := sha256.New() + s.writeHashingInputs(h) - hd := sha256.Sum256(buf.Bytes()) + hd := h.Sum(nil) digest = hd[:] return } -func (s *solver) writeHashingInputs(buf *bytes.Buffer) { +func (s *solver) writeHashingInputs(w io.Writer) { + writeString := func(s string) { + // All users of writeHashingInputs cannot error on Write(), so just + // ignore it + w.Write([]byte(s)) + } + // Apply overrides to the constraints from the root. Otherwise, the hash // would be computed on the basis of a constraint from root that doesn't // actually affect solving. - p := s.ovr.overrideAll(s.rm.DependencyConstraints().merge(s.rm.TestDependencyConstraints())) + wc := s.ovr.overrideAll(s.rm.DependencyConstraints().merge(s.rm.TestDependencyConstraints())) - for _, pd := range p { - buf.WriteString(string(pd.Ident.ProjectRoot)) - buf.WriteString(pd.Ident.Source) + for _, pd := range wc { + writeString(string(pd.Ident.ProjectRoot)) + writeString(pd.Ident.Source) // FIXME Constraint.String() is a surjective-only transformation - tags // and branches with the same name are written out as the same string. - // This could, albeit rarely, result in input collisions when a real - // change has occurred. - buf.WriteString(pd.Constraint.String()) + // This could, albeit rarely, result in erroneously identical inputs + // when a real change has occurred. + writeString(pd.Constraint.String()) } + // Get the external reach list + // Write each of the packages, or the errors that were found for a // particular subpath, into the hash. We need to do this in a // deterministic order, so expand and sort the map. @@ -51,19 +60,19 @@ func (s *solver) writeHashingInputs(buf *bytes.Buffer) { sort.Sort(sortPackageOrErr(pkgs)) for _, perr := range pkgs { if perr.Err != nil { - buf.WriteString(perr.Err.Error()) + writeString(perr.Err.Error()) } else { - buf.WriteString(perr.P.Name) - buf.WriteString(perr.P.CommentPath) - buf.WriteString(perr.P.ImportPath) + writeString(perr.P.Name) + writeString(perr.P.CommentPath) + writeString(perr.P.ImportPath) for _, imp := range perr.P.Imports { if !isStdLib(imp) { - buf.WriteString(imp) + writeString(imp) } } for _, imp := range perr.P.TestImports { if !isStdLib(imp) { - buf.WriteString(imp) + writeString(imp) } } } @@ -79,7 +88,7 @@ func (s *solver) writeHashingInputs(buf *bytes.Buffer) { sort.Strings(req) for _, reqp := range req { - buf.WriteString(reqp) + writeString(reqp) } } @@ -93,23 +102,32 @@ func (s *solver) writeHashingInputs(buf *bytes.Buffer) { sort.Strings(ig) for _, igp := range ig { - buf.WriteString(igp) + writeString(igp) } } for _, pc := range s.ovr.asSortedSlice() { - buf.WriteString(string(pc.Ident.ProjectRoot)) + writeString(string(pc.Ident.ProjectRoot)) if pc.Ident.Source != "" { - buf.WriteString(pc.Ident.Source) + writeString(pc.Ident.Source) } if pc.Constraint != nil { - buf.WriteString(pc.Constraint.String()) + writeString(pc.Constraint.String()) } } an, av := s.b.AnalyzerInfo() - buf.WriteString(an) - buf.WriteString(av.String()) + writeString(an) + writeString(av.String()) +} + +// bytes.Buffer wrapper that injects newlines after each call to Write(). +type nlbuf bytes.Buffer + +func (buf *nlbuf) Write(p []byte) (n int, err error) { + n, _ = (*bytes.Buffer)(buf).Write(p) + (*bytes.Buffer)(buf).WriteByte('\n') + return n + 1, nil } // HashingInputsAsString returns the raw input data used by Solver.HashInputs() @@ -118,10 +136,10 @@ func (s *solver) writeHashingInputs(buf *bytes.Buffer) { // This is primarily intended for debugging purposes. func HashingInputsAsString(s Solver) string { ts := s.(*solver) - buf := new(bytes.Buffer) + buf := new(nlbuf) ts.writeHashingInputs(buf) - return buf.String() + return (*bytes.Buffer)(buf).String() } type sortPackageOrErr []PackageOrErr diff --git a/hash_test.go b/hash_test.go index d55f2de..4ec28e5 100644 --- a/hash_test.go +++ b/hash_test.go @@ -46,7 +46,7 @@ func TestHashInputs(t *testing.T) { t.Error("Hashes are not equal") } - fixstr, hisstr := strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr := strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -100,7 +100,7 @@ func TestHashInputsReqsIgs(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr := strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr := strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -148,7 +148,7 @@ func TestHashInputsReqsIgs(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr = strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr = strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -190,7 +190,7 @@ func TestHashInputsReqsIgs(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr = strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr = strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -245,7 +245,7 @@ func TestHashInputsOverrides(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr := strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr := strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -283,7 +283,7 @@ func TestHashInputsOverrides(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr = strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr = strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -325,7 +325,7 @@ func TestHashInputsOverrides(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr = strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr = strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -368,7 +368,7 @@ func TestHashInputsOverrides(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr = strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr = strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -412,7 +412,7 @@ func TestHashInputsOverrides(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr = strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr = strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } @@ -458,7 +458,7 @@ func TestHashInputsOverrides(t *testing.T) { t.Errorf("Hashes are not equal") } - fixstr, hisstr = strings.Join(elems, ""), HashingInputsAsString(s) + fixstr, hisstr = strings.Join(elems, "\n")+"\n", HashingInputsAsString(s) if fixstr != hisstr { t.Errorf("Hashing inputs not equal:\n\t(GOT) %s\n\t(WNT) %s", hisstr, fixstr) } From 2488c3ea222c5361f163088a25a87c96b3311c85 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Wed, 11 Jan 2017 21:02:09 -0500 Subject: [PATCH 2/4] Remove blank/newlines from hashing tests --- hash.go | 11 ++++++++--- hash_test.go | 9 --------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/hash.go b/hash.go index b412993..3548524 100644 --- a/hash.go +++ b/hash.go @@ -28,9 +28,14 @@ func (s *solver) HashInputs() (digest []byte) { func (s *solver) writeHashingInputs(w io.Writer) { writeString := func(s string) { - // All users of writeHashingInputs cannot error on Write(), so just - // ignore it - w.Write([]byte(s)) + // Skip zero-length string writes; it doesn't affect the real hash + // calculation, and keeps misleading newlines from showing up in the + // debug output. + if s != "" { + // All users of writeHashingInputs cannot error on Write(), so just + // ignore it + w.Write([]byte(s)) + } } // Apply overrides to the constraints from the root. Otherwise, the hash diff --git a/hash_test.go b/hash_test.go index 4ec28e5..a645697 100644 --- a/hash_test.go +++ b/hash_test.go @@ -82,7 +82,6 @@ func TestHashInputsReqsIgs(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -128,7 +127,6 @@ func TestHashInputsReqsIgs(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -172,7 +170,6 @@ func TestHashInputsReqsIgs(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -227,7 +224,6 @@ func TestHashInputsOverrides(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -263,7 +259,6 @@ func TestHashInputsOverrides(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -302,7 +297,6 @@ func TestHashInputsOverrides(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -343,7 +337,6 @@ func TestHashInputsOverrides(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -387,7 +380,6 @@ func TestHashInputsOverrides(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", @@ -432,7 +424,6 @@ func TestHashInputsOverrides(t *testing.T) { "b", "1.0.0", "root", - "", "root", "a", "b", From 95c24a49ed89bdf9e904de2285805c64a5381241 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Wed, 11 Jan 2017 21:04:05 -0500 Subject: [PATCH 3/4] Remove pointless ifs --- hash.go | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/hash.go b/hash.go index 3548524..8cc9b6d 100644 --- a/hash.go +++ b/hash.go @@ -83,32 +83,26 @@ func (s *solver) writeHashingInputs(w io.Writer) { } } - // Write any require packages given in the root manifest. - if len(s.req) > 0 { - // Dump and sort the reqnores - req := make([]string, 0, len(s.req)) - for pkg := range s.req { - req = append(req, pkg) - } - sort.Strings(req) + // Write any required packages given in the root manifest. + req := make([]string, 0, len(s.req)) + for pkg := range s.req { + req = append(req, pkg) + } + sort.Strings(req) - for _, reqp := range req { - writeString(reqp) - } + for _, reqp := range req { + writeString(reqp) } // Add the ignored packages, if any. - if len(s.ig) > 0 { - // Dump and sort the ignores - ig := make([]string, 0, len(s.ig)) - for pkg := range s.ig { - ig = append(ig, pkg) - } - sort.Strings(ig) + ig := make([]string, 0, len(s.ig)) + for pkg := range s.ig { + ig = append(ig, pkg) + } + sort.Strings(ig) - for _, igp := range ig { - writeString(igp) - } + for _, igp := range ig { + writeString(igp) } for _, pc := range s.ovr.asSortedSlice() { From 53a999a58d2a348e9913d638a4795727c6079d7f Mon Sep 17 00:00:00 2001 From: sam boyer Date: Thu, 12 Jan 2017 00:17:37 -0500 Subject: [PATCH 4/4] Split out 'rootdata' struct from solver This separates a bunch of the static state/rules/information that comes from the root project and input parameters into a discrete subsystem. The only real benefit here is focusing the state tracked by the solver in on the actual algorithm of solving, and less so these static rules - which should make it a bit easier for other people to grok. --- bridge.go | 18 ++- hash.go | 14 +- rootdata.go | 173 ++++++++++++++++++++++++ solve_test.go | 8 +- solver.go | 365 +++++++++++++++++++------------------------------- trace.go | 20 +-- 6 files changed, 343 insertions(+), 255 deletions(-) create mode 100644 rootdata.go diff --git a/bridge.go b/bridge.go index 5d8c4c6..34945dc 100644 --- a/bridge.go +++ b/bridge.go @@ -40,6 +40,9 @@ type bridge struct { // held by the solver that it ends up being easier and saner to do this. s *solver + // Whether to sort version lists for downgrade. + down bool + // Simple, local cache of the root's PackageTree crp *struct { ptree PackageTree @@ -58,17 +61,18 @@ type bridge struct { // Global factory func to create a bridge. This exists solely to allow tests to // override it with a custom bridge and sm. -var mkBridge = func(s *solver, sm SourceManager) sourceBridge { +var mkBridge = func(s *solver, sm SourceManager, down bool) sourceBridge { return &bridge{ sm: sm, s: s, + down: down, vlists: make(map[ProjectIdentifier][]Version), } } func (b *bridge) GetManifestAndLock(id ProjectIdentifier, v Version) (Manifest, Lock, error) { - if id.ProjectRoot == ProjectRoot(b.s.rpt.ImportRoot) { - return b.s.rm, b.s.rl, nil + if b.s.rd.isRoot(id.ProjectRoot) { + return b.s.rd.rm, b.s.rd.rl, nil } b.s.mtr.push("b-gmal") @@ -94,7 +98,7 @@ func (b *bridge) ListVersions(id ProjectIdentifier) ([]Version, error) { return nil, err } - if b.s.params.Downgrade { + if b.down { SortForDowngrade(vl) } else { SortForUpgrade(vl) @@ -120,7 +124,7 @@ func (b *bridge) SourceExists(id ProjectIdentifier) (bool, error) { } func (b *bridge) vendorCodeExists(id ProjectIdentifier) (bool, error) { - fi, err := os.Stat(filepath.Join(b.s.params.RootDir, "vendor", string(id.ProjectRoot))) + fi, err := os.Stat(filepath.Join(b.s.rd.dir, "vendor", string(id.ProjectRoot))) if err != nil { return false, err } else if fi.IsDir() { @@ -279,7 +283,7 @@ func (b *bridge) vtu(id ProjectIdentifier, v Version) versionTypeUnion { // The root project is handled separately, as the source manager isn't // responsible for that code. func (b *bridge) ListPackages(id ProjectIdentifier, v Version) (PackageTree, error) { - if id.ProjectRoot == ProjectRoot(b.s.rpt.ImportRoot) { + if b.s.rd.isRoot(id.ProjectRoot) { panic("should never call ListPackages on root project") } @@ -327,7 +331,7 @@ func (b *bridge) breakLock() { return } - for _, lp := range b.s.rl.Projects() { + for _, lp := range b.s.rd.rl.Projects() { if _, is := b.s.sel.selected(lp.pi); !is { // TODO(sdboyer) use this as an opportunity to detect // inconsistencies between upstream and the lock (e.g., moved tags)? diff --git a/hash.go b/hash.go index 8cc9b6d..d926c71 100644 --- a/hash.go +++ b/hash.go @@ -41,7 +41,7 @@ func (s *solver) writeHashingInputs(w io.Writer) { // Apply overrides to the constraints from the root. Otherwise, the hash // would be computed on the basis of a constraint from root that doesn't // actually affect solving. - wc := s.ovr.overrideAll(s.rm.DependencyConstraints().merge(s.rm.TestDependencyConstraints())) + wc := s.rd.combineConstraints() for _, pd := range wc { writeString(string(pd.Ident.ProjectRoot)) @@ -59,7 +59,7 @@ func (s *solver) writeHashingInputs(w io.Writer) { // particular subpath, into the hash. We need to do this in a // deterministic order, so expand and sort the map. var pkgs []PackageOrErr - for _, perr := range s.rpt.Packages { + for _, perr := range s.rd.rpt.Packages { pkgs = append(pkgs, perr) } sort.Sort(sortPackageOrErr(pkgs)) @@ -84,8 +84,8 @@ func (s *solver) writeHashingInputs(w io.Writer) { } // Write any required packages given in the root manifest. - req := make([]string, 0, len(s.req)) - for pkg := range s.req { + req := make([]string, 0, len(s.rd.req)) + for pkg := range s.rd.req { req = append(req, pkg) } sort.Strings(req) @@ -95,8 +95,8 @@ func (s *solver) writeHashingInputs(w io.Writer) { } // Add the ignored packages, if any. - ig := make([]string, 0, len(s.ig)) - for pkg := range s.ig { + ig := make([]string, 0, len(s.rd.ig)) + for pkg := range s.rd.ig { ig = append(ig, pkg) } sort.Strings(ig) @@ -105,7 +105,7 @@ func (s *solver) writeHashingInputs(w io.Writer) { writeString(igp) } - for _, pc := range s.ovr.asSortedSlice() { + for _, pc := range s.rd.ovr.asSortedSlice() { writeString(string(pc.Ident.ProjectRoot)) if pc.Ident.Source != "" { writeString(pc.Ident.Source) diff --git a/rootdata.go b/rootdata.go new file mode 100644 index 0000000..413e558 --- /dev/null +++ b/rootdata.go @@ -0,0 +1,173 @@ +package gps + +import ( + "sort" + + "github.com/armon/go-radix" +) + +// rootdata holds static data and constraining rules from the root project for +// use in solving. +type rootdata struct { + // Path to the root of the project on which gps is operating. + dir string + + // Map of packages to ignore. + ig map[string]bool + + // Map of packages to require. + req map[string]bool + + // A ProjectConstraints map containing the validated (guaranteed non-empty) + // overrides declared by the root manifest. + ovr ProjectConstraints + + // A map of the ProjectRoot (local names) that should be allowed to change + chng map[ProjectRoot]struct{} + + // Flag indicating all projects should be allowed to change, without regard + // for lock. + chngall bool + + // A map of the project names listed in the root's lock. + rlm map[ProjectRoot]LockedProject + + // A defensively-copied instance of the root manifest. + rm Manifest + + // A defensively-copied instance of the root lock. + rl Lock + + // A defensively-copied instance of params.RootPackageTree + rpt PackageTree +} + +// rootImportList returns a list of the unique imports from the root data. +// Ignores and requires are taken into consideration. +func (rd rootdata) externalImportList() []string { + reach := rd.rpt.ExternalReach(true, true, rd.ig).ListExternalImports() + + // If there are any requires, slide them into the reach list, as well. + if len(rd.req) > 0 { + reqs := make([]string, 0, len(rd.req)) + + // Make a map of both imported and required pkgs to skip, to avoid + // duplication. Technically, a slice would probably be faster (given + // small size and bounds check elimination), but this is a one-time op, + // so it doesn't matter. + skip := make(map[string]bool, len(rd.req)) + for _, r := range reach { + if rd.req[r] { + skip[r] = true + } + } + + for r := range rd.req { + if !skip[r] { + reqs = append(reqs, r) + } + } + + reach = append(reach, reqs...) + } + + return reach +} + +func (rd rootdata) getApplicableConstraints() []workingConstraint { + xt := radix.New() + combined := rd.combineConstraints() + + type wccount struct { + count int + wc workingConstraint + } + for _, wc := range combined { + xt.Insert(string(wc.Ident.ProjectRoot), wccount{wc: wc}) + } + + // Walk all dep import paths we have to consider and mark the corresponding + // wc entry in the trie, if any + for _, im := range rd.externalImportList() { + if isStdLib(im) { + continue + } + + if pre, v, match := xt.LongestPrefix(im); match && isPathPrefixOrEqual(pre, im) { + wcc := v.(wccount) + wcc.count++ + xt.Insert(pre, wcc) + } + } + + var ret []workingConstraint + + xt.Walk(func(s string, v interface{}) bool { + wcc := v.(wccount) + if wcc.count > 0 || wcc.wc.overrNet || wcc.wc.overrConstraint { + ret = append(ret, wcc.wc) + } + return false + }) + + return ret +} + +func (rd rootdata) combineConstraints() []workingConstraint { + return rd.ovr.overrideAll(rd.rm.DependencyConstraints().merge(rd.rm.TestDependencyConstraints())) +} + +// needVersionListFor indicates whether we need a version list for a given +// project root, based solely on general solver inputs (no constraint checking +// required). This will be true if: +// +// - ChangeAll is on +// - The project is not in the lock at all +// - The project is in the lock, but is also in the list of projects to change +func (rd rootdata) needVersionsFor(pr ProjectRoot) bool { + if rd.chngall { + return true + } + + if _, has := rd.rlm[pr]; !has { + // not in the lock + return true + } else if _, has := rd.chng[pr]; has { + // in the lock, but marked for change + return true + } + // in the lock, not marked for change + return false + +} + +func (rd rootdata) isRoot(pr ProjectRoot) bool { + return pr == ProjectRoot(rd.rpt.ImportRoot) +} + +// rootAtom creates an atomWithPackages that represents the root project. +func (rd rootdata) rootAtom() atomWithPackages { + a := atom{ + id: ProjectIdentifier{ + ProjectRoot: ProjectRoot(rd.rpt.ImportRoot), + }, + // This is a hack so that the root project doesn't have a nil version. + // It's sort of OK because the root never makes it out into the results. + // We may need a more elegant solution if we discover other side + // effects, though. + v: rootRev, + } + + list := make([]string, 0, len(rd.rpt.Packages)) + for path, pkg := range rd.rpt.Packages { + if pkg.Err != nil && !rd.ig[path] { + list = append(list, path) + } + } + sort.Strings(list) + + return atomWithPackages{ + a: a, + pl: list, + } +} diff --git a/solve_test.go b/solve_test.go index 2d3de69..9b203f0 100644 --- a/solve_test.go +++ b/solve_test.go @@ -20,7 +20,7 @@ var fixtorun string // TODO(sdboyer) regression test ensuring that locks with only revs for projects don't cause errors func init() { flag.StringVar(&fixtorun, "gps.fix", "", "A single fixture to run in TestBasicSolves or TestBimodalSolves") - mkBridge(nil, nil) + mkBridge(nil, nil, false) overrideMkBridge() overrideIsStdLib() } @@ -29,11 +29,12 @@ func init() { func overrideMkBridge() { // For all tests, override the base bridge with the depspecBridge that skips // verifyRootDir calls - mkBridge = func(s *solver, sm SourceManager) sourceBridge { + mkBridge = func(s *solver, sm SourceManager, down bool) sourceBridge { return &depspecBridge{ &bridge{ sm: sm, s: s, + down: down, vlists: make(map[ProjectIdentifier][]Version), }, } @@ -417,10 +418,11 @@ func TestBadSolveOpts(t *testing.T) { // swap out the test mkBridge override temporarily, just to make sure we get // the right error - mkBridge = func(s *solver, sm SourceManager) sourceBridge { + mkBridge = func(s *solver, sm SourceManager, down bool) sourceBridge { return &bridge{ sm: sm, s: s, + down: down, vlists: make(map[ProjectIdentifier][]Version), } } diff --git a/solver.go b/solver.go index c74e104..1727666 100644 --- a/solver.go +++ b/solver.go @@ -14,7 +14,7 @@ var rootRev = Revision("") // SolveParameters hold all arguments to a solver run. // -// Only RootDir and ImportRoot are absolutely required. A nil Manifest is +// Only RootDir and RootPackageTree are absolutely required. A nil Manifest is // allowed, though it usually makes little sense. // // Of these properties, only Manifest and Ignore are (directly) incorporated in @@ -92,14 +92,6 @@ type solver struct { // starts moving forward again. attempts int - // SolveParameters are the inputs to the solver. They determine both what - // data the solver should operate on, and certain aspects of how solving - // proceeds. - // - // Prepare() validates these, so by the time we have a *solver instance, we - // know they're valid. - params SolveParameters - // Logger used exclusively for trace output, if the trace option is set. tl *log.Logger @@ -128,12 +120,6 @@ type solver struct { // removal. unsel *unselected - // 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 @@ -142,92 +128,57 @@ type solver struct { // added to an existing project. vqs []*versionQueue - // A map of the ProjectRoot (local names) that should be allowed to change - chng map[ProjectRoot]struct{} - - // A ProjectConstraints map containing the validated (guaranteed non-empty) - // overrides declared by the root manifest. - ovr ProjectConstraints - - // A map of the project names listed in the root's lock. - rlm map[ProjectRoot]LockedProject - - // A defensively-copied instance of the root manifest. - rm Manifest - - // A defensively-copied instance of the root lock. - rl Lock - - // A defensively-copied instance of params.RootPackageTree - rpt PackageTree + // Contains data and constraining information from the root project + rd rootdata // metrics for the current solve run. mtr *metrics } -// A Solver is the main workhorse of gps: given a set of project inputs, it -// performs a constraint solving analysis to develop a complete Solution, or -// else fail with an informative error. -// -// If a Solution is found, an implementing tool may persist it - typically into -// a "lock file" - and/or use it to write out a directory tree of dependencies, -// suitable to be a vendor directory, via CreateVendorTree. -type Solver interface { - // HashInputs hashes the unique inputs to this solver, returning the hash - // digest. It is guaranteed that, if the resulting digest is equal to the - // digest returned from a previous Solution.InputHash(), that that Solution - // is valid for this Solver's inputs. - // - // In such a case, it may not be necessary to run Solve() at all. - HashInputs() []byte - - // Solve initiates a solving run. It will either complete successfully with - // a Solution, or fail with an informative error. - Solve() (Solution, error) -} - -// Prepare readies a Solver for use. -// -// This function reads and validates the provided SolveParameters. If a problem -// with the inputs is detected, an error is returned. Otherwise, a Solver is -// returned, ready to hash and check inputs or perform a solving run. -func Prepare(params SolveParameters, sm SourceManager) (Solver, error) { - if sm == nil { - return nil, badOptsFailure("must provide non-nil SourceManager") - } +func (params SolveParameters) toRootdata() (rootdata, error) { if params.RootDir == "" { - return nil, badOptsFailure("params must specify a non-empty root directory") + return rootdata{}, badOptsFailure("params must specify a non-empty root directory") } if params.RootPackageTree.ImportRoot == "" { - return nil, badOptsFailure("params must include a non-empty import root") + return rootdata{}, badOptsFailure("params must include a non-empty import root") } if len(params.RootPackageTree.Packages) == 0 { - return nil, badOptsFailure("at least one package must be present in the PackageTree") - } - if params.Trace && params.TraceLogger == nil { - return nil, badOptsFailure("trace requested, but no logger provided") + return rootdata{}, badOptsFailure("at least one package must be present in the PackageTree") } if params.Lock == nil && len(params.ToChange) != 0 { - return nil, badOptsFailure(fmt.Sprintf("update specifically requested for %s, but no lock was provided to upgrade from", params.ToChange)) + return rootdata{}, badOptsFailure(fmt.Sprintf("update specifically requested for %s, but no lock was provided to upgrade from", params.ToChange)) } if params.Manifest == nil { params.Manifest = simpleRootManifest{} } - s := &solver{ - params: params, - ig: params.Manifest.IgnoredPackages(), - req: params.Manifest.RequiredPackages(), - ovr: params.Manifest.Overrides(), - tl: params.TraceLogger, - rpt: params.RootPackageTree.dup(), + rd := rootdata{ + ig: params.Manifest.IgnoredPackages(), + req: params.Manifest.RequiredPackages(), + ovr: params.Manifest.Overrides(), + rpt: params.RootPackageTree.dup(), + chng: make(map[ProjectRoot]struct{}), + rlm: make(map[ProjectRoot]LockedProject), + chngall: params.ChangeAll, + dir: params.RootDir, + } + + // Ensure the required, ignore and overrides maps are at least initialized + if rd.ig == nil { + rd.ig = make(map[string]bool) + } + if rd.req == nil { + rd.req = make(map[string]bool) + } + if rd.ovr == nil { + rd.ovr = make(ProjectConstraints) } - if len(s.ig) != 0 { + if len(rd.ig) != 0 { var both []string for pkg := range params.Manifest.RequiredPackages() { - if s.ig[pkg] { + if rd.ig[pkg] { both = append(both, pkg) } } @@ -235,23 +186,15 @@ func Prepare(params SolveParameters, sm SourceManager) (Solver, error) { case 0: break case 1: - return nil, badOptsFailure(fmt.Sprintf("%q was given as both a required and ignored package", both[0])) + return rootdata{}, 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: %s", strings.Join(both, ", "))) + return rootdata{}, badOptsFailure(fmt.Sprintf("multiple packages given as both required and ignored: %s", strings.Join(both, ", "))) } } - // Ensure the ignore and overrides maps are at least initialized - if s.ig == nil { - s.ig = make(map[string]bool) - } - if s.ovr == nil { - s.ovr = make(ProjectConstraints) - } - // Validate no empties in the overrides map var eovr []string - for pr, pp := range s.ovr { + for pr, pp := range rd.ovr { if pp.Constraint == nil && pp.Source == "" { eovr = append(eovr, string(pr)) } @@ -263,24 +206,66 @@ func Prepare(params SolveParameters, sm SourceManager) (Solver, error) { // tool/user know there's bad input. Purely as a principle, that seems // preferable to silently allowing progress with icky input. if len(eovr) > 1 { - return nil, badOptsFailure(fmt.Sprintf("Overrides lacked any non-zero properties for multiple project roots: %s", strings.Join(eovr, " "))) + return rootdata{}, badOptsFailure(fmt.Sprintf("Overrides lacked any non-zero properties for multiple project roots: %s", strings.Join(eovr, " "))) } - return nil, badOptsFailure(fmt.Sprintf("An override was declared for %s, but without any non-zero properties", eovr[0])) + return rootdata{}, badOptsFailure(fmt.Sprintf("An override was declared for %s, but without any non-zero properties", eovr[0])) + } + + // Prep safe, normalized versions of root manifest and lock data + rd.rm = prepManifest(params.Manifest) + + if params.Lock != nil { + for _, lp := range params.Lock.Projects() { + rd.rlm[lp.Ident().ProjectRoot] = lp + } + + // Also keep a prepped one, mostly for the bridge. This is probably + // wasteful, but only minimally so, and yay symmetry + rd.rl = prepLock(params.Lock) + } + + for _, p := range params.ToChange { + if _, exists := rd.rlm[p]; !exists { + return rootdata{}, badOptsFailure(fmt.Sprintf("cannot update %s as it is not in the lock", p)) + } + rd.chng[p] = struct{}{} + } + + return rd, nil +} + +// Prepare readies a Solver for use. +// +// This function reads and validates the provided SolveParameters. If a problem +// with the inputs is detected, an error is returned. Otherwise, a Solver is +// returned, ready to hash and check inputs or perform a solving run. +func Prepare(params SolveParameters, sm SourceManager) (Solver, error) { + if sm == nil { + return nil, badOptsFailure("must provide non-nil SourceManager") + } + if params.Trace && params.TraceLogger == nil { + return nil, badOptsFailure("trace requested, but no logger provided") + } + + rd, err := params.toRootdata() + if err != nil { + return nil, err + } + + s := &solver{ + tl: params.TraceLogger, + rd: rd, } // Set up the bridge and ensure the root dir is in good, working order // before doing anything else. (This call is stubbed out in tests, via // overriding mkBridge(), so we can run with virtual RootDir.) - s.b = mkBridge(s, sm) - err := s.b.verifyRootDir(s.params.RootDir) + s.b = mkBridge(s, sm, params.Downgrade) + err = s.b.verifyRootDir(params.RootDir) if err != nil { return nil, err } - // Initialize maps - s.chng = make(map[ProjectRoot]struct{}) - s.rlm = make(map[ProjectRoot]LockedProject) - // Initialize stacks and queues s.sel = &selection{ deps: make(map[ProjectRoot][]dependency), @@ -291,26 +276,28 @@ func Prepare(params SolveParameters, sm SourceManager) (Solver, error) { cmp: s.unselectedComparator, } - // Prep safe, normalized versions of root manifest and lock data - s.rm = prepManifest(s.params.Manifest) - if s.params.Lock != nil { - for _, lp := range s.params.Lock.Projects() { - s.rlm[lp.Ident().ProjectRoot] = lp - } - - // Also keep a prepped one, mostly for the bridge. This is probably - // wasteful, but only minimally so, and yay symmetry - s.rl = prepLock(s.params.Lock) - } + return s, nil +} - for _, p := range s.params.ToChange { - if _, exists := s.rlm[p]; !exists { - return nil, badOptsFailure(fmt.Sprintf("cannot update %s as it is not in the lock", p)) - } - s.chng[p] = struct{}{} - } +// A Solver is the main workhorse of gps: given a set of project inputs, it +// performs a constraint solving analysis to develop a complete Solution, or +// else fail with an informative error. +// +// If a Solution is found, an implementing tool may persist it - typically into +// a "lock file" - and/or use it to write out a directory tree of dependencies, +// suitable to be a vendor directory, via CreateVendorTree. +type Solver interface { + // HashInputs hashes the unique inputs to this solver, returning the hash + // digest. It is guaranteed that, if the resulting digest is equal to the + // digest returned from a previous Solution.InputHash(), that that Solution + // is valid for this Solver's inputs. + // + // In such a case, it may not be necessary to run Solve() at all. + HashInputs() []byte - return s, nil + // Solve initiates a solving run. It will either complete successfully with + // a Solution, or fail with an informative error. + Solve() (Solution, error) } // Solve attempts to find a dependency solution for the given project, as @@ -348,8 +335,8 @@ func (s *solver) Solve() (Solution, error) { } s.traceFinish(soln, err) - if s.params.Trace { - s.mtr.dump(s.params.TraceLogger) + if s.tl != nil { + s.mtr.dump(s.tl) } return soln, err } @@ -467,67 +454,14 @@ func (s *solver) solve() (map[atom]map[string]struct{}, error) { // populate the queues at the beginning of a solve run. func (s *solver) selectRoot() error { s.mtr.push("select-root") - pa := atom{ - id: ProjectIdentifier{ - ProjectRoot: ProjectRoot(s.rpt.ImportRoot), - }, - // This is a hack so that the root project doesn't have a nil version. - // It's sort of OK because the root never makes it out into the results. - // We may need a more elegant solution if we discover other side - // effects, though. - v: rootRev, - } - - list := make([]string, len(s.rpt.Packages)) - k := 0 - for path, pkg := range s.rpt.Packages { - if pkg.Err != nil { - list[k] = path - k++ - } - } - list = list[:k] - sort.Strings(list) - - a := atomWithPackages{ - a: pa, - pl: list, - } - // Push the root project onto the queue. // TODO(sdboyer) maybe it'd just be better to skip this? - s.sel.pushSelection(a, true) + awp := s.rd.rootAtom() + s.sel.pushSelection(awp, true) // 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())) - reach := s.rpt.ExternalReach(true, true, s.ig).ListExternalImports() - - // If there are any requires, slide them into the reach list, as well. - if len(s.req) > 0 { - reqs := make([]string, 0, len(s.req)) - - // Make a map of both imported and required pkgs to skip, to avoid - // duplication. Technically, a slice would probably be faster (given - // small size and bounds check elimination), but this is a one-time op, - // so it doesn't matter. - skip := make(map[string]bool, len(s.req)) - for _, r := range reach { - if s.req[r] { - skip[r] = true - } - } - - for r := range s.req { - if !skip[r] { - reqs = append(reqs, r) - } - } - - reach = append(reach, reqs...) - } - - deps, err := s.intersectConstraintsWithImports(mdeps, reach) + deps, err := s.intersectConstraintsWithImports(s.rd.combineConstraints(), s.rd.externalImportList()) if err != nil { // TODO(sdboyer) this could well happen; handle it with a more graceful error panic(fmt.Sprintf("shouldn't be possible %s", err)) @@ -537,16 +471,16 @@ func (s *solver) selectRoot() error { // If we have no lock, or if this dep isn't in the lock, then prefetch // it. See longer explanation in selectAtom() for how we benefit from // parallelism here. - if s.needVersionsFor(dep.Ident.ProjectRoot) { + if s.rd.needVersionsFor(dep.Ident.ProjectRoot) { go s.b.SyncSourceFor(dep.Ident) } - s.sel.pushDep(dependency{depender: pa, dep: dep}) + s.sel.pushDep(dependency{depender: awp.a, dep: dep}) // Add all to unselected queue heap.Push(s.unsel, bimodalIdentifier{id: dep.Ident, pl: dep.pl, fromRoot: true}) } - s.traceSelectRoot(s.rpt, deps) + s.traceSelectRoot(s.rd.rpt, deps) s.mtr.pop() return nil } @@ -554,7 +488,7 @@ func (s *solver) selectRoot() error { func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]completeDep, error) { var err error - if ProjectRoot(s.rpt.ImportRoot) == a.a.id.ProjectRoot { + if s.rd.isRoot(a.a.id.ProjectRoot) { panic("Should never need to recheck imports/constraints from root during solve") } @@ -570,7 +504,7 @@ func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]completeDep, return nil, err } - allex := ptree.ExternalReach(false, false, s.ig) + allex := ptree.ExternalReach(false, false, s.rd.ig) // Use a map to dedupe the unique external packages exmap := make(map[string]struct{}) // Add to the list those packages that are reached by the packages @@ -603,7 +537,7 @@ func (s *solver) getImportsAndConstraintsOf(a atomWithPackages) ([]completeDep, } sort.Strings(reach) - deps := s.ovr.overrideAll(m.DependencyConstraints()) + deps := s.rd.ovr.overrideAll(m.DependencyConstraints()) return s.intersectConstraintsWithImports(deps, reach) } @@ -629,23 +563,21 @@ func (s *solver) intersectConstraintsWithImports(deps []workingConstraint, reach // Look for a prefix match; it'll be the root project/repo containing // the reached package - if pre, idep, match := xt.LongestPrefix(rp); match { - if isPathPrefixOrEqual(pre, rp) { - // Match is valid; put it in the dmap, either creating a new - // completeDep or appending it to the existing one for this base - // project/prefix. - dep := idep.(workingConstraint) - if cdep, exists := dmap[dep.Ident.ProjectRoot]; exists { - cdep.pl = append(cdep.pl, rp) - dmap[dep.Ident.ProjectRoot] = cdep - } else { - dmap[dep.Ident.ProjectRoot] = completeDep{ - workingConstraint: dep, - pl: []string{rp}, - } + if pre, idep, match := xt.LongestPrefix(rp); match && isPathPrefixOrEqual(pre, rp) { + // Match is valid; put it in the dmap, either creating a new + // completeDep or appending it to the existing one for this base + // project/prefix. + dep := idep.(workingConstraint) + if cdep, exists := dmap[dep.Ident.ProjectRoot]; exists { + cdep.pl = append(cdep.pl, rp) + dmap[dep.Ident.ProjectRoot] = cdep + } else { + dmap[dep.Ident.ProjectRoot] = completeDep{ + workingConstraint: dep, + pl: []string{rp}, } - continue } + continue } // No match. Let the SourceManager try to figure out the root @@ -656,7 +588,7 @@ func (s *solver) intersectConstraintsWithImports(deps []workingConstraint, reach } // Make a new completeDep with an open constraint, respecting overrides - pd := s.ovr.override(root, ProjectProperties{Constraint: Any()}) + pd := s.rd.ovr.override(root, ProjectProperties{Constraint: Any()}) // Insert the pd into the trie so that further deps from this // project get caught by the prefix search @@ -682,7 +614,7 @@ func (s *solver) intersectConstraintsWithImports(deps []workingConstraint, reach func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error) { id := bmi.id // If on the root package, there's no queue to make - if ProjectRoot(s.rpt.ImportRoot) == id.ProjectRoot { + if s.rd.isRoot(id.ProjectRoot) { return newVersionQueue(id, nil, nil, s.b) } @@ -704,7 +636,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error } var lockv Version - if len(s.rlm) > 0 { + if len(s.rd.rlm) > 0 { lockv, err = s.getLockVersionIfValid(id) if err != nil { // Can only get an error here if an upgrade was expressly requested on @@ -722,7 +654,7 @@ func (s *solver) createVersionQueue(bmi bimodalIdentifier) (*versionQueue, error // TODO(sdboyer) nested loop; prime candidate for a cache somewhere for _, dep := range s.sel.getDependenciesOn(bmi.id) { // Skip the root, of course - if ProjectRoot(s.rpt.ImportRoot) == dep.depender.id.ProjectRoot { + if s.rd.isRoot(dep.depender.id.ProjectRoot) { continue } @@ -857,7 +789,7 @@ func (s *solver) findValidVersion(q *versionQueue, pl []string) error { func (s *solver) getLockVersionIfValid(id ProjectIdentifier) (Version, error) { // If the project is specifically marked for changes, then don't look for a // locked version. - if _, explicit := s.chng[id.ProjectRoot]; explicit || s.params.ChangeAll { + if _, explicit := s.rd.chng[id.ProjectRoot]; explicit || s.rd.chngall { // For projects with an upstream or cache repository, it's safe to // ignore what's in the lock, because there's presumably more versions // to be found and attempted in the repository. If it's only in vendor, @@ -880,7 +812,7 @@ func (s *solver) getLockVersionIfValid(id ProjectIdentifier) (Version, error) { } } - lp, exists := s.rlm[id.ProjectRoot] + lp, exists := s.rd.rlm[id.ProjectRoot] if !exists { return nil, nil } @@ -922,29 +854,6 @@ func (s *solver) getLockVersionIfValid(id ProjectIdentifier) (Version, error) { return v, nil } -// needVersionListFor indicates whether we need a version list for a given -// project root, based solely on general solver inputs (no constraint checking -// required). This will be true if: -// -// - ChangeAll is on -// - The project is not in the lock at all -// - The project is in the lock, but is also in the list of projects to change -func (s *solver) needVersionsFor(pr ProjectRoot) bool { - if s.params.ChangeAll { - return true - } - - if _, has := s.rlm[pr]; !has { - // not in the lock - return true - } else if _, has := s.chng[pr]; has { - // in the lock, but marked for change - return true - } - // in the lock, not marked for change - return false -} - // backtrack works backwards from the current failed solution to find the next // solution to try. func (s *solver) backtrack() bool { @@ -1059,8 +968,8 @@ func (s *solver) unselectedComparator(i, j int) bool { return false } - _, ilock := s.rlm[iname.ProjectRoot] - _, jlock := s.rlm[jname.ProjectRoot] + _, ilock := s.rd.rlm[iname.ProjectRoot] + _, jlock := s.rd.rlm[jname.ProjectRoot] switch { case ilock && !jlock: @@ -1105,7 +1014,7 @@ func (s *solver) fail(id ProjectIdentifier) { // selection? // skip if the root project - if ProjectRoot(s.rpt.ImportRoot) != id.ProjectRoot { + if !s.rd.isRoot(id.ProjectRoot) { // just look for the first (oldest) one; the backtracker will necessarily // traverse through and pop off any earlier ones for _, vq := range s.vqs { @@ -1167,7 +1076,7 @@ func (s *solver) selectAtom(a atomWithPackages, pkgonly bool) { // few microseconds before blocking later. Best case, the dep doesn't // come up next, but some other dep comes up that wasn't prefetched, and // both fetches proceed in parallel. - if s.needVersionsFor(dep.Ident.ProjectRoot) { + if s.rd.needVersionsFor(dep.Ident.ProjectRoot) { go s.b.SyncSourceFor(dep.Ident) } diff --git a/trace.go b/trace.go index 6baf3f4..db0ff2e 100644 --- a/trace.go +++ b/trace.go @@ -15,7 +15,7 @@ const ( ) func (s *solver) traceCheckPkgs(bmi bimodalIdentifier) { - if !s.params.Trace { + if s.tl == nil { return } @@ -24,7 +24,7 @@ func (s *solver) traceCheckPkgs(bmi bimodalIdentifier) { } func (s *solver) traceCheckQueue(q *versionQueue, bmi bimodalIdentifier, cont bool, offset int) { - if !s.params.Trace { + if s.tl == nil { return } @@ -49,7 +49,7 @@ func (s *solver) traceCheckQueue(q *versionQueue, bmi bimodalIdentifier, cont bo // traceStartBacktrack is called with the bmi that first failed, thus initiating // backtracking func (s *solver) traceStartBacktrack(bmi bimodalIdentifier, err error, pkgonly bool) { - if !s.params.Trace { + if s.tl == nil { return } @@ -67,7 +67,7 @@ func (s *solver) traceStartBacktrack(bmi bimodalIdentifier, err error, pkgonly b // traceBacktrack is called when a package or project is poppped off during // backtracking func (s *solver) traceBacktrack(bmi bimodalIdentifier, pkgonly bool) { - if !s.params.Trace { + if s.tl == nil { return } @@ -84,7 +84,7 @@ func (s *solver) traceBacktrack(bmi bimodalIdentifier, pkgonly bool) { // Called just once after solving has finished, whether success or not func (s *solver) traceFinish(sol solution, err error) { - if !s.params.Trace { + if s.tl == nil { return } @@ -101,15 +101,15 @@ func (s *solver) traceFinish(sol solution, err error) { // traceSelectRoot is called just once, when the root project is selected func (s *solver) traceSelectRoot(ptree PackageTree, cdeps []completeDep) { - if !s.params.Trace { + if s.tl == nil { return } // This duplicates work a bit, but we're in trace mode and it's only once, // so who cares - rm := ptree.ExternalReach(true, true, s.ig) + rm := ptree.ExternalReach(true, true, s.rd.ig) - s.tl.Printf("Root project is %q", s.rpt.ImportRoot) + s.tl.Printf("Root project is %q", s.rd.rpt.ImportRoot) var expkgs int for _, cdep := range cdeps { @@ -124,7 +124,7 @@ func (s *solver) traceSelectRoot(ptree PackageTree, cdeps []completeDep) { // traceSelect is called when an atom is successfully selected func (s *solver) traceSelect(awp atomWithPackages, pkgonly bool) { - if !s.params.Trace { + if s.tl == nil { return } @@ -140,7 +140,7 @@ func (s *solver) traceSelect(awp atomWithPackages, pkgonly bool) { } func (s *solver) traceInfo(args ...interface{}) { - if !s.params.Trace { + if s.tl == nil { return }