All tests are green when SAT is on (#1218)

* all green

* enables the satcache

src/nimble.nim

@@ -62,12 +62,11 @@ proc displaySatisfiedMsg(solvedPkgs: seq[SolvedPackage], pkgToInstall: seq[(stri
 
 proc addReverseDeps(solvedPkgs: seq[SolvedPackage], allPkgsInfo: seq[PackageInfo], options: Options) = 
   for pkg in solvedPkgs:
-    let solvedPkg = getPackageInfo(pkg.pkgName, allPkgsInfo)
+    let solvedPkg = getPackageInfo(pkg.pkgName, allPkgsInfo, some pkg.version)
     if solvedPkg.isNone: continue
-    for reverseDepName in pkg.reverseDependencies:
-      var reverseDep = getPackageInfo(reverseDepName, allPkgsInfo)
+    for (reverseDepName, ver) in pkg.reverseDependencies:
+      var reverseDep = getPackageInfo(reverseDepName, allPkgsInfo, some ver)
       if reverseDep.isNone: continue
-
       if reverseDep.get.myPath.parentDir.developFileExists:
         reverseDep.get.isLink = true
       addRevDep(options.nimbleData, solvedPkg.get.basicInfo, reverseDep.get)
@@ -78,23 +77,32 @@ proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): Has
     return satProccesedPackages
   var solvedPkgs = newSeq[SolvedPackage]()
   var pkgsToInstall: seq[(string, Version)] = @[]
-  var pkgList = initPkgList(rootPkgInfo, options)
+  var pkgList = initPkgList(rootPkgInfo, options).mapIt(it.toFullInfo(options))
   var allPkgsInfo: seq[PackageInfo] = pkgList & rootPkgInfo
-  var rootPkgInfo = rootPkgInfo
-  #Replace requirements so they are updated as needed 
-  if options.action.typ == actionUpgrade:
-    let toUpgradeNames = options.action.packages.mapIt(it[0])
+  #Remove from the pkglist the packages that exists in lock file and has a different vcsRevision
+  var toUpgradeNames: seq[string]
+  var isUpgrading = options.action.typ == actionUpgrade
+  if isUpgrading:
+    toUpgradeNames = options.action.packages.mapIt(it[0])
     pkgList = pkgList.filterIt(it.basicInfo.name notin toUpgradeNames)
-
-    # rootPkgInfo.requires = rootPkgInfo.requires.filterIt(it.name notin toUpgradeNames)
-    # rootPkgInfo.requires &= options.action.packages
+
+  var toRemoveFromLocked = newSeq[PackageInfo]()
+  if rootPkgInfo.lockedDeps.hasKey(""):
+    for name, lockedPkg in rootPkgInfo.lockedDeps[""]:
+      for pkg in pkgList:
+        if name notin toUpgradeNames and name == pkg.basicInfo.name and 
+         (isUpgrading and lockedPkg.vcsRevision != pkg.metaData.vcsRevision or 
+          not isUpgrading and lockedPkg.vcsRevision == pkg.metaData.vcsRevision):
+              toRemoveFromLocked.add pkg
 
   result = solveLocalPackages(rootPkgInfo, pkgList, solvedPkgs)
   if solvedPkgs.len > 0: 
     displaySatisfiedMsg(solvedPkgs, pkgsToInstall)
     addReverseDeps(solvedPkgs, allPkgsInfo, options)
     for pkg in allPkgsInfo:
       result.incl pkg
+    for nonLocked in toRemoveFromLocked:
+      result.excl nonLocked
     result = 
       result.toSeq
       .deleteStaleDependencies(rootPkgInfo, options)
@@ -124,12 +132,16 @@ proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): Has
     allPkgsInfo.add pkg
   addReverseDeps(solvedPkgs, allPkgsInfo, options)
 
+  for nonLocked in toRemoveFromLocked:
+    result.excl nonLocked
+
   result = deleteStaleDependencies(result.toSeq, rootPkgInfo, options).toHashSet  
   satProccesedPackages = result
-
+ 
   if not solved:
     display("Error", output, Error, priority = HighPriority)
     raise nimbleError("Unsatisfiable dependencies")
+
 proc processFreeDependencies(pkgInfo: PackageInfo,
                              requirements: seq[PkgTuple],
                              options: Options,
@@ -796,7 +808,7 @@ proc install(packages: seq[PkgTuple], options: Options,
       let (meth, url, metadata) = getDownloadInfo(pv, options, doPrompt)
       let subdir = metadata.getOrDefault("subdir")
       var downloadPath = ""
-      if options.useSatSolver:
+      if options.useSatSolver and subdir == "": #Ignore the cache if subdir is set
           downloadPath =  getCacheDownloadDir(url, pv.ver, options)
       let (downloadDir, downloadVersion, vcsRevision) =
          downloadPkg(url, pv.ver, meth, subdir, options,

src/nimblepkg/nimblesat.nim

@@ -3,7 +3,7 @@ when defined(nimNimbleBootstrap):
 else:
   import sat/[sat, satvars] 
 import version, packageinfotypes, download, packageinfo, packageparser, options, 
-  sha1hashes#, tools
+  sha1hashes, tools
 
 import std/[tables, sequtils, algorithm, sets, strutils, options, strformat, os]
 
@@ -58,10 +58,27 @@ type
     pkgName*: string
     version*: Version
     requirements*: seq[PkgTuple] 
-    reverseDependencies*: seq[string]  
+    reverseDependencies*: seq[(string, Version)] 
 
   GetPackageMinimal* = proc (pv: PkgTuple, options: Options): Option[PackageMinimalInfo]
 
+#From the STD as it is not available in older Nim versions
+func addUnique*[T](s: var seq[T], x: sink T) =
+  ## Adds `x` to the container `s` if it is not already present. 
+  ## Uses `==` to check if the item is already present.
+  runnableExamples:
+    var a = @[1, 2, 3]
+    a.addUnique(4)
+    a.addUnique(4)
+    assert a == @[1, 2, 3, 4]
+
+  for i in 0..high(s):
+    if s[i] == x: return
+  when declared(ensureMove):
+    s.add ensureMove(x)
+  else:
+    s.add x
+
 proc isNim*(pv: PkgTuple): bool =
   pv.name == "nim" or pv.name == "nimrod"
 
@@ -289,14 +306,13 @@ proc solve*(g: var DepGraph; f: Form, packages: var Table[string, Version], outp
     output = generateUnsatisfiableMessage(g, f, s)
     false
 
-proc collectReverseDependencies*(targetPkgName: string, graph: DepGraph): seq[string] =
-  var reverseDeps: HashSet[string] = initHashSet[string]()
+proc collectReverseDependencies*(targetPkgName: string, graph: DepGraph): seq[(string, Version)] =
   for node in graph.nodes:
     for version in node.versions:
-      for (depName, _) in graph.reqs[version.req].deps:
+      for (depName, ver) in graph.reqs[version.req].deps:
         if depName == targetPkgName:
-          reverseDeps.incl(node.pkgName)  #
-  reverseDeps.toSeq()
+          let revDep = (node.pkgName, version.version)
+          result.addUnique revDep
 
 proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output: var string): seq[SolvedPackage] =
   var graph = pkgVersionTable.toDepGraph()
@@ -319,12 +335,13 @@ proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output:
         let reqIdx = dep.req
         let deps =  graph.reqs[reqIdx].deps
         let solvedPkg = SolvedPackage(pkgName: pkg, version: ver, 
-          requirements: deps, reverseDependencies: collectReverseDependencies(pkg, graph))
+          requirements: deps, 
+          reverseDependencies: collectReverseDependencies(pkg, graph),
+        )
         result.add solvedPkg
 
 proc getCacheDownloadDir*(url: string, ver: VersionRange, options: Options): string =
-  return ""
-  # options.pkgCachePath / getDownloadDirName(url, ver, notSetSha1Hash)
+  options.pkgCachePath / getDownloadDirName(url, ver, notSetSha1Hash)
 
 proc downloadPkInfoForPv*(pv: PkgTuple, options: Options): PackageInfo  =
   let (meth, url, metadata) = 
@@ -353,23 +370,6 @@ proc fillPackageTableFromPreferred*(packages: var Table[string, PackageVersions]
 proc getInstalledMinimalPackages*(options: Options): seq[PackageMinimalInfo] =
   getInstalledPkgsMin(options.getPkgsDir(), options).mapIt(it.getMinimalInfo())
 
-#From the STD as it is not available in older Nim versions
-func addUnique*[T](s: var seq[T], x: sink T) =
-  ## Adds `x` to the container `s` if it is not already present. 
-  ## Uses `==` to check if the item is already present.
-  runnableExamples:
-    var a = @[1, 2, 3]
-    a.addUnique(4)
-    a.addUnique(4)
-    assert a == @[1, 2, 3, 4]
-
-  for i in 0..high(s):
-    if s[i] == x: return
-  when declared(ensureMove):
-    s.add ensureMove(x)
-  else:
-    s.add x
-
 proc collectAllVersions*(versions: var Table[string, PackageVersions], package: PackageMinimalInfo, options: Options, getMinimalPackage: GetPackageMinimal,  preferredPackages: seq[PackageMinimalInfo] = newSeq[PackageMinimalInfo]()) =
   ### Collects all the versions of a package and its dependencies and stores them in the versions table
   ### A getMinimalPackage function is passed to get the package
@@ -393,19 +393,6 @@ proc collectAllVersions*(versions: var Table[string, PackageVersions], package:
         versions[pv.name].versions.addUnique pkgMin
       collectAllVersions(versions, pkgMin, options, getMinimalPackage)
 
-proc solveLocalPackages*(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo], solvedPkgs: var seq[SolvedPackage]): HashSet[PackageInfo] = 
-  var root = rootPkgInfo.getMinimalInfo()
-  root.isRoot = true
-  var pkgVersionTable = initTable[string, PackageVersions]()
-  pkgVersionTable[root.name] = PackageVersions(pkgName: root.name, versions: @[root])
-  fillPackageTableFromPreferred(pkgVersionTable, pkgList.map(getMinimalInfo))
-  var output = ""
-  solvedPkgs = pkgVersionTable.getSolvedPackages(output)
-  for solvedPkg in solvedPkgs:
-    for pkgInfo in pkgList:
-      if pkgInfo.basicInfo.name == solvedPkg.pkgName and pkgInfo.basicInfo.version == solvedPkg.version:
-        result.incl pkgInfo
-
 proc topologicalSort*(solvedPkgs: seq[SolvedPackage]): seq[SolvedPackage] =
   var inDegree = initTable[string, int]()
   var adjList = initTable[string, seq[string]]()
@@ -436,6 +423,18 @@ proc topologicalSort*(solvedPkgs: seq[SolvedPackage]): seq[SolvedPackage] =
       if inDegree[neighbor] == 0:
         zeroInDegree.add(neighbor) 
 
+proc solveLocalPackages*(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo], solvedPkgs: var seq[SolvedPackage]): HashSet[PackageInfo] = 
+  var root = rootPkgInfo.getMinimalInfo()
+  root.isRoot = true
+  var pkgVersionTable = initTable[string, PackageVersions]()
+  pkgVersionTable[root.name] = PackageVersions(pkgName: root.name, versions: @[root])
+  fillPackageTableFromPreferred(pkgVersionTable, pkgList.map(getMinimalInfo))
+  var output = ""
+  solvedPkgs = pkgVersionTable.getSolvedPackages(output)
+  for solvedPkg in solvedPkgs:
+    for pkgInfo in pkgList:
+      if pkgInfo.basicInfo.name == solvedPkg.pkgName and pkgInfo.basicInfo.version == solvedPkg.version:
+        result.incl pkgInfo
 
 proc solvePackages*(rootPkg: PackageInfo, pkgList: seq[PackageInfo], pkgsToInstall: var seq[(string, Version)], options: Options, output: var string, solvedPkgs: var seq[SolvedPackage]): HashSet[PackageInfo] =
   var root: PackageMinimalInfo = rootPkg.getMinimalInfo()
@@ -455,7 +454,11 @@ proc solvePackages*(rootPkg: PackageInfo, pkgList: seq[PackageInfo], pkgsToInsta
     if not foundInList:
       pkgsToInstall.addUnique((solvedPkg.pkgName, solvedPkg.version))
 
-proc getPackageInfo*(dep: string, pkgs: seq[PackageInfo]): Option[PackageInfo] =
+proc getPackageInfo*(name: string, pkgs: seq[PackageInfo], version: Option[Version] = none(Version)): Option[PackageInfo] =
     for pkg in pkgs:
-      if pkg.basicInfo.name.tolower == dep.tolower or pkg.metadata.url == dep:
-        return some pkg 
+      if pkg.basicInfo.name.tolower == name.tolower or pkg.metadata.url == name:
+        if version.isSome:
+          if pkg.basicInfo.version == version.get:
+            return some pkg
+        else: #No version passed over first match
+          return some pkg