Buncha docs

bestiary_test.go

@@ -990,134 +990,5 @@ func backtracking() {
     "myapp from root": "0.0.0",
     "a": "1.0.0"
   }, maxTries: 2);
-
-  // Tests that the backjumper will jump past unrelated selections when a
-  // source conflict occurs. This test selects, in order:
-  // - myapp -> a
-  // - myapp -> b
-  // - myapp -> c (1 of 5)
-  // - b -> a
-  // It selects a and b first because they have fewer versions than c. It
-  // traverses b"s dependency on a after selecting a version of c because
-  // dependencies are traversed breadth-first (all of myapps"s immediate deps
-  // before any other their deps).
-  //
-  // This means it doesn"t discover the source conflict until after selecting
-  // c. When that happens, it should backjump past c instead of trying older
-  // versions of it since they aren"t related to the conflict.
-  testResolve("backjump to conflicting source", {
-    "myapp 0.0.0": {
-      "a": "any",
-      "b": "any",
-      "c": "any"
-    },
-    "a 1.0.0": {},
-    "a 1.0.0 from mock2": {},
-    "b 1.0.0": {
-      "a": "any"
-    },
-    "b 2.0.0": {
-      "a from mock2": "any"
-    },
-    "c 1.0.0": {},
-    "c 2.0.0": {},
-    "c 3.0.0": {},
-    "c 4.0.0": {},
-    "c 5.0.0": {},
-  }, result: {
-    "myapp from root": "0.0.0",
-    "a": "1.0.0",
-    "b": "1.0.0",
-    "c": "5.0.0"
-  }, maxTries: 2);
-
-  // Like the above test, but for a conflicting description.
-  testResolve("backjump to conflicting description", {
-    "myapp 0.0.0": {
-      "a-x": "any",
-      "b": "any",
-      "c": "any"
-    },
-    "a-x 1.0.0": {},
-    "a-y 1.0.0": {},
-    "b 1.0.0": {
-      "a-x": "any"
-    },
-    "b 2.0.0": {
-      "a-y": "any"
-    },
-    "c 1.0.0": {},
-    "c 2.0.0": {},
-    "c 3.0.0": {},
-    "c 4.0.0": {},
-    "c 5.0.0": {},
-  }, result: {
-    "myapp from root": "0.0.0",
-    "a": "1.0.0",
-    "b": "1.0.0",
-    "c": "5.0.0"
-  }, maxTries: 2);
-
-  // Similar to the above two tests but where there is no solution. It should
-  // fail in this case with no backtracking.
-  testResolve("backjump to conflicting source", {
-    "myapp 0.0.0": {
-      "a": "any",
-      "b": "any",
-      "c": "any"
-    },
-    "a 1.0.0": {},
-    "a 1.0.0 from mock2": {},
-    "b 1.0.0": {
-      "a from mock2": "any"
-    },
-    "c 1.0.0": {},
-    "c 2.0.0": {},
-    "c 3.0.0": {},
-    "c 4.0.0": {},
-    "c 5.0.0": {},
-  }, error: sourceMismatch("a", "myapp", "b"), maxTries: 1);
-
-  testResolve("backjump to conflicting description", {
-    "myapp 0.0.0": {
-      "a-x": "any",
-      "b": "any",
-      "c": "any"
-    },
-    "a-x 1.0.0": {},
-    "a-y 1.0.0": {},
-    "b 1.0.0": {
-      "a-y": "any"
-    },
-    "c 1.0.0": {},
-    "c 2.0.0": {},
-    "c 3.0.0": {},
-    "c 4.0.0": {},
-    "c 5.0.0": {},
-  }, error: descriptionMismatch("a", "myapp", "b"), maxTries: 1);
-
-  // This is a regression test for #18666. It was possible for the solver to
-  // "forget" that a package had previously led to an error. In that case, it
-  // would backtrack over the failed package instead of trying different
-  // versions of it.
-  testResolve("finds solution with less strict constraint", {
-    "myapp 1.0.0": {
-      "a": "any",
-      "c": "any",
-      "d": "any"
-    },
-    "a 2.0.0": {},
-    "a 1.0.0": {},
-    "b 1.0.0": {"a": "1.0.0"},
-    "c 1.0.0": {"b": "any"},
-    "d 2.0.0": {"myapp": "any"},
-    "d 1.0.0": {"myapp": "<1.0.0"}
-  }, result: {
-    "myapp from root": "1.0.0",
-    "a": "1.0.0",
-    "b": "1.0.0",
-    "c": "1.0.0",
-    "d": "2.0.0"
-  }, maxTries: 3);
 }
 */

satisfy.go

@@ -2,8 +2,8 @@ package vsolver
 
 import "github.com/Sirupsen/logrus"
 
-// satisfiable is the main checking method - it determines if introducing a new
+// satisfiable is the main checking method. It determines if introducing a new
-// project atom would result in a graph where all requirements are still
+// project atom would result in a state where all solver requirements are still
 // satisfied.
 func (s *solver) satisfiable(pa ProjectAtom) error {
 	if emptyProjectAtom == pa {
@@ -169,6 +169,12 @@ func (s *solver) checkDepsDisallowsSelected(pa ProjectAtom, dep ProjectDep) erro
 	return nil
 }
 
+// checkIdentMatches ensures that the LocalName of a dep introduced by an atom,
+// has the same NetworkName as what's already been selected (assuming anything's
+// been selected).
+//
+// In other words, this ensures that the solver never simultaneously selects two
+// identifiers that disagree about where their upstream source is.
 func (s *solver) checkIdentMatches(pa ProjectAtom, dep ProjectDep) error {
 	if cur, exists := s.names[dep.Ident.LocalName]; exists {
 		if cur != dep.Ident.netName() {

solver.go

@@ -23,12 +23,37 @@ type Solver interface {
 // SolveOpts holds both options that govern solving behavior, and the actual
 // inputs to the solving process.
 type SolveOpts struct {
-	Root                 string
+	// The path to the root of the project on which the solver is working.
-	N                    ProjectName
+	Root string
-	M                    Manifest
+	// The 'name' of the project. Required. This should (must?) correspond to subpath of
-	L                    Lock
+	// Root that exists under a GOPATH.
-	Downgrade, ChangeAll bool
+	N ProjectName
-	ToChange             []ProjectName
+	// The root manifest. Required. This contains all the dependencies, constraints, and
+	// other controls available to the root project.
+	M Manifest
+	// The root lock. Optional. Generally, this lock is the output of a previous solve run.
+	//
+	// If provided, the solver will attempt to preserve the versions specified
+	// in the lock, unless ToChange or ChangeAll settings indicate otherwise.
+	L Lock
+	// Downgrade indicates whether the solver will attempt to upgrade (false) or
+	// downgrade (true) projects that are not locked, or are marked for change.
+	//
+	// Upgrading is, by far, the most typical case. The field is named
+	// 'Downgrade' so that the bool's zero value corresponds to that most
+	// typical case.
+	Downgrade bool
+	// ChangeAll indicates that all projects should be changed - that is, any
+	// versions specified in the root lock file should be ignored.
+	ChangeAll bool
+	// ToChange is a list of project names that should be changed - that is, any
+	// versions specified for those projects in the root lock file should be
+	// ignored.
+	//
+	// Passing ChangeAll has subtly different behavior from enumerating all
+	// projects into ToChange. In general, ToChange should *only* be used if the
+	// user expressly requested an upgrade for a specific project.
+	ToChange []ProjectName
 }
 
 func NewSolver(sm SourceManager, l *logrus.Logger) Solver {
@@ -46,15 +71,43 @@ func NewSolver(sm SourceManager, l *logrus.Logger) Solver {
 // conditions hardcoded to the needs of the Go package management problem space.
 type solver struct {
 	attempts int
-	o        SolveOpts
+	// SolveOpts are the configuration options provided to the solver. The
-	l        *logrus.Logger
+	// solver will abort early if certain options are not appropriately set.
-	sm       *smAdapter
+	o SolveOpts
-	sel      *selection
+	l *logrus.Logger
-	unsel    *unselected
+	// An adapter around a standard SourceManager. The adapter does some local
+	// caching of pre-sorted version lists, as well as translation between the
+	// full-on ProjectIdentifiers that the solver deals with and the simplified
+	// names a SourceManager operates on.
+	sm *smAdapter
+	// The list of projects currently "selected" - that is, they have passed all
+	// satisfiability checks, and are part of the current solution.
+	//
+	// The *selection type is mostly just a dumb data container; the solver
+	// itself is responsible for maintaining that invariant.
+	sel *selection
+	// The current list of projects that we need to incorporate into the solution in
+	// order for the solution to be complete. This list is implemented as a
+	// priority queue that places projects least likely to induce errors at the
+	// front, in order to minimize the amount of backtracking required to find a
+	// solution.
+	//
+	// Entries are added to and removed from this list by the solver at the same
+	// time that the selected queue is updated, either with an addition or
+	// removal.
+	unsel *unselected
+	// A list 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.versions never
+	// will.
 	versions []*versionQueue
-	latest   map[ProjectName]struct{}
+	// A map of the ProjectName (local names) that should be allowed to change
-	names    map[ProjectName]string
+	chng map[ProjectName]struct{}
-	rlm      map[ProjectName]LockedProject
+	// A map of the ProjectName (local names) that are currently selected, and
+	// the network name to which they currently correspond.
+	names map[ProjectName]string
+	// A map of the names listed in the root's lock.
+	rlm map[ProjectName]LockedProject
 }
 
 // Solve attempts to find a dependency solution for the given project, as
@@ -90,8 +143,7 @@ func (s *solver) Solve(opts SolveOpts) (Result, error) {
 	s.o = opts
 
 	// Initialize maps
-
+	s.chng = make(map[ProjectName]struct{})
-	s.latest = make(map[ProjectName]struct{})
 	s.rlm = make(map[ProjectName]LockedProject)
 	s.names = make(map[ProjectName]string)
 
@@ -102,7 +154,7 @@ func (s *solver) Solve(opts SolveOpts) (Result, error) {
 	}
 
 	for _, v := range s.o.ToChange {
-		s.latest[v] = struct{}{}
+		s.chng[v] = struct{}{}
 	}
 
 	// Initialize queues
@@ -339,10 +391,19 @@ func (s *solver) findValidVersion(q *versionQueue) error {
 	}
 }
 
+// getLockVersionIfValid finds an atom for the given ProjectIdentifier from the
+// root lock, assuming:
+//
+// 1. A root lock was provided
+// 2. The general flag to change all projects was not passed
+// 3. A flag to change this particular ProjectIdentifier was not passed
+//
+// If any of these three conditions are true (or if the id cannot be found in
+// the root lock), then no atom will be returned.
 func (s *solver) getLockVersionIfValid(id ProjectIdentifier) (ProjectAtom, error) {
 	// If the project is specifically marked for changes, then don't look for a
 	// locked version.
-	if _, explicit := s.latest[id.LocalName]; explicit || s.o.ChangeAll {
+	if _, explicit := s.chng[id.LocalName]; explicit || s.o.ChangeAll {
 		// 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,