The develop command now works with SAT on (tdevelopfeature green) (#…

…1210)

* fixes develop

* removes unnecessary func

* fixes indent

* treversedeps green when SAT on

* removes redundant packages

* Improves the download cache, doenst re-downloads pkgs when installing

* adds cleanDirs to test so it doesnt fail in subsequent runs

* makes another test green

* Fixes ilegal package setup for a test

* Dont error on local package list

src/nimble.nim

@@ -54,51 +54,65 @@ proc checkSatisfied(options: Options, dependencies: seq[PackageInfo]) =
           [pkgInfo.basicInfo.name, $currentVer, $pkgsInPath[pkgInfo.basicInfo.name]])
     pkgsInPath[pkgInfo.basicInfo.name] = currentVer
 
+proc displaySatisfiedMsg(solvedPkgs: seq[SolvedPackage], pkgToInstall: seq[(string, Version)]) =
+  for pkg in solvedPkgs:
+    if pkg.pkgName notin pkgToInstall.mapIt(it[0]):
+      for req in pkg.requirements:
+        displayInfo(pkgDepsAlreadySatisfiedMsg(req))
+
+proc addReverseDeps(solvedPkgs: seq[SolvedPackage], allPkgsInfo: seq[PackageInfo], options: Options) = 
+  for pkg in solvedPkgs:
+    let solvedPkg = getPackageInfo(pkg.pkgName, allPkgsInfo)
+    if solvedPkg.isNone: continue
+    for reverseDepName in pkg.reverseDependencies:
+      var reverseDep = getPackageInfo(reverseDepName, allPkgsInfo)
+      if reverseDep.isNone: continue
+
+      if reverseDep.get.myPath.parentDir.developFileExists:
+        reverseDep.get.isLink = true
+      addRevDep(options.nimbleData, solvedPkg.get.basicInfo, reverseDep.get)
+
 proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo], options: Options): HashSet[PackageInfo] = 
-  result = solveLocalPackages(rootPkgInfo, pkgList)
-  if result.len > 0: return result
-  var reverseDependencies: seq[PackageBasicInfo] = @[]
+  var solvedPkgs = newSeq[SolvedPackage]()
   var pkgsToInstall: seq[(string, Version)] = @[]
-  var output = ""
-  var solved = false #A pgk can be solved and still dont return a set of PackageInfo
-  (solved, result) = solvePackages(rootPkgInfo, pkgList, pkgsToInstall, options, output)
-  if pkgsToInstall.len > 0:
-    for pkg in pkgsToInstall:
-      let dep = (name: pkg[0], ver: pkg[1].toVersionRange)
-      let resolvedDep = dep.resolveAlias(options)
-      display("Installing", $resolvedDep, priority = HighPriority)
-      let toInstall = @[(resolvedDep.name, resolvedDep.ver)]
-      #TODO install here will download the package again. We could use the already downloaded package 
-      #from the cache
-      let (packages, _) = install(toInstall, options,
-        doPrompt = false, first = false, fromLockFile = false, preferredPackages = @[])
+  var allPkgsInfo: seq[PackageInfo] = pkgList & rootPkgInfo
 
-      for pkg in packages:
-        if result.contains pkg:
-          # If the result already contains the newly tried to install package
-          # we had to merge its special versions set into the set of the old
-          # one.
-          result[pkg].metaData.specialVersions.incl(
-            pkg.metaData.specialVersions)
-        else:
-          result.incl pkg
-
-        if not pkg.isLink:
-          reverseDependencies.add(pkg.basicInfo)
-  if result.len > 0: 
-    # We add the reverse deps to the JSON file here because we don't want
-    # them added if the above errorenous condition occurs
-    # (unsatisfiable dependendencies).
-    # N.B. NimbleData is saved in installFromDir.
-    for i in reverseDependencies:
-      addRevDep(options.nimbleData, i, rootPkgInfo)
+  result = solveLocalPackages(rootPkgInfo, pkgList, solvedPkgs)
+  if solvedPkgs.len > 0: 
+    displaySatisfiedMsg(solvedPkgs, pkgsToInstall)
+    addReverseDeps(solvedPkgs, allPkgsInfo, options)
     return result
-  else:
-    if not solved:
-      display("Error", output, Error, priority = HighPriority)
-      raise nimbleError("Unsatisfiable dependencies")
-
 
+  var output = ""
+  result = solvePackages(rootPkgInfo, pkgList, pkgsToInstall, options, output, solvedPkgs)
+  displaySatisfiedMsg(solvedPkgs, pkgsToInstall)
+  var solved = solvedPkgs.len > 0 #A pgk can be solved and still dont return a set of PackageInfo
+  let toInstall = pkgsToInstall
+    .mapIt((name: it[0], ver: it[1].toVersionRange))
+    .mapIt(it.resolveAlias(options))
+    .mapIt((name: it.name, ver: it.ver))
+
+  if toInstall.len > 0:
+    let (packages, _) = install(toInstall, options,
+      doPrompt = false, first = false, fromLockFile = false, preferredPackages = @[])
+    for pkg in packages:
+      if result.contains pkg:
+        # If the result already contains the newly tried to install package
+        # we had to merge its special versions set into the set of the old
+        # one.
+        result[pkg].metaData.specialVersions.incl(
+          pkg.metaData.specialVersions)
+      else:
+        result.incl pkg
+
+  for pkg in result:
+    allPkgsInfo.add pkg
+  addReverseDeps(solvedPkgs, allPkgsInfo, options)
+
+  if not solved:
+    display("Error", output, Error, priority = HighPriority)
+    raise nimbleError("Unsatisfiable dependencies")
+
 proc processFreeDependencies(pkgInfo: PackageInfo,
                              requirements: seq[PkgTuple],
                              options: Options,
@@ -463,7 +477,7 @@ proc installFromDir(dir: string, requestedVer: VersionRange, options: Options,
     priority = HighPriority)
 
   let oldPkg = pkgInfo.packageExists(options)
-  if oldPkg.isSome:
+  if oldPkg.isSome and not options.useSatSolver:
     # In the case we already have the same package in the cache then only merge
     # the new package special versions to the old one.
     displayWarning(pkgAlreadyExistsInTheCacheMsg(pkgInfo))
@@ -764,9 +778,12 @@ proc install(packages: seq[PkgTuple], options: Options,
     for pv in packages:
       let (meth, url, metadata) = getDownloadInfo(pv, options, doPrompt)
       let subdir = metadata.getOrDefault("subdir")
+      var downloadPath = ""
+      if options.useSatSolver:
+          downloadPath =  getCacheDownloadDir(url, pv.ver, options)
       let (downloadDir, downloadVersion, vcsRevision) =
          downloadPkg(url, pv.ver, meth, subdir, options,
-                    downloadPath = "", vcsRevision = notSetSha1Hash)
+                    downloadPath = downloadPath, vcsRevision = notSetSha1Hash)
       try:
         var opt = options
         if pv.name.isNim:

src/nimblepkg/download.nim

@@ -463,18 +463,10 @@ proc downloadPkg*(url: string, verRange: VersionRange,
     raise nimbleError("Cannot download in offline mode.")
   let downloadDir =
     if downloadPath == "":
-      let dir = if options.pkgCachePath != "":
-        options.pkgCachePath
-      else:
-        getNimbleTempDir()
-      (dir / getDownloadDirName(url, verRange, vcsRevision))
+      (getNimbleTempDir() / getDownloadDirName(url, verRange, vcsRevision))
     else:
       downloadPath
-
-  if options.pkgCachePath != "" and dirExists(downloadDir):
-    #TODO test integrity of the package
-    return (dir: downloadDir, version: newVersion getSimpleString(verRange), vcsRevision: notSetSha1Hash)    
-
+
   createDir(downloadDir)
   var modUrl =
     if url.startsWith("git://") and options.config.cloneUsingHttps:

src/nimblepkg/nimblesat.nim

@@ -3,9 +3,9 @@ when defined(nimNimbleBootstrap):
 else:
   import sat/[sat, satvars] 
 import version, packageinfotypes, download, packageinfo, packageparser, options, 
-  sha1hashes
+  sha1hashes, tools
 
-import std/[tables, sequtils, algorithm, sets, strutils, options, strformat]
+import std/[tables, sequtils, algorithm, sets, strutils, options, strformat, os]
 
 
 type  
@@ -54,12 +54,21 @@ type
     reqs*: seq[Requirements]
     packageToDependency*: Table[string, int] #package.name -> index into nodes
     # reqsByDeps: Table[Requirements, int]
+  SolvedPackage* = object
+    pkgName*: string
+    version*: Version
+    requirements*: seq[PkgTuple] 
+    reverseDependencies*: seq[string]  
+
   GetPackageMinimal* = proc (pv: PkgTuple, options: Options): Option[PackageMinimalInfo]
 
+proc isNim*(pv: PkgTuple): bool =
+  pv.name == "nim" or pv.name == "nimrod"
+
 proc getMinimalInfo*(pkg: PackageInfo): PackageMinimalInfo =
   result.name = pkg.basicInfo.name
   result.version = pkg.basicInfo.version
-  result.requires = pkg.requires
+  result.requires = pkg.requires.filterIt(not it.isNim())
 
 proc hasVersion*(packageVersions: PackageVersions, pv: PkgTuple): bool =
   for pkg in packageVersions.versions:
@@ -101,7 +110,7 @@ proc findDependencyForDep(g: DepGraph; dep: string): int {.inline.} =
   result = g.packageToDependency.getOrDefault(dep)
 
 proc createRequirements(pkg: PackageMinimalInfo): Requirements =
-  result.deps = pkg.requires.filterIt(it.name != "nim")
+  result.deps = pkg.requires.filterIt(not it.isNim())
   result.version = pkg.version
   result.nimVersion = pkg.requires.getNimVersion()
 
@@ -280,28 +289,50 @@ proc solve*(g: var DepGraph; f: Form, packages: var Table[string, Version], outp
     output = generateUnsatisfiableMessage(g, f, s)
     false
 
-proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output: var string): Table[string, Version] =
+proc collectReverseDependencies*(targetPkgName: string, graph: DepGraph): seq[string] =
+  var reverseDeps: HashSet[string] = initHashSet[string]()
+  for node in graph.nodes:
+    for version in node.versions:
+      for (depName, _) in graph.reqs[version.req].deps:
+        if depName == targetPkgName:
+          reverseDeps.incl(node.pkgName)  #
+  reverseDeps.toSeq()
+
+proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output: var string): seq[SolvedPackage] =
   var graph = pkgVersionTable.toDepGraph()
   #Make sure all references are in the graph before calling toFormular
   for p in graph.nodes:
     for ver in p.versions.items:
       for dep, q in items graph.reqs[ver.req].deps:
         if dep notin graph.packageToDependency:
           output.add &"Dependency {dep} not found in the graph \n"
-          return initTable[string, Version]()
+          return newSeq[SolvedPackage]()
 
   let form = toFormular(graph)
   var packages = initTable[string, Version]()
   discard solve(graph, form, packages, output)
-  packages
+
+  for pkg, ver in packages:
+    let nodeIdx = graph.packageToDependency[pkg]
+    for dep in graph.nodes[nodeIdx].versions:
+      if dep.version == ver:
+        let reqIdx = dep.req
+        let deps =  graph.reqs[reqIdx].deps
+        let solvedPkg = SolvedPackage(pkgName: pkg, version: ver, 
+          requirements: deps, reverseDependencies: collectReverseDependencies(pkg, graph))
+        result.add solvedPkg
+
+proc getCacheDownloadDir*(url: string, ver: VersionRange, options: Options): string =
+  options.pkgCachePath / getDownloadDirName(url, ver, notSetSha1Hash)
 
 proc downloadPkInfoForPv*(pv: PkgTuple, options: Options): PackageInfo  =
   let (meth, url, metadata) = 
-    getDownloadInfo(pv, options, doPrompt = true)
+    getDownloadInfo(pv, options, doPrompt = false, ignorePackageCache = false)
   let subdir = metadata.getOrDefault("subdir")
+  let downloadDir =  getCacheDownloadDir(url, pv.ver, options)
   let res = 
     downloadPkg(url, pv.ver, meth, subdir, options,
-                  "", vcsRevision = notSetSha1Hash)
+                  downloadDir, vcsRevision = notSetSha1Hash)
   return getPkgInfo(res.dir, options)
 
 proc downloadMinimalPackage*(pv: PkgTuple, options: Options): Option[PackageMinimalInfo] =
@@ -321,14 +352,38 @@ proc fillPackageTableFromPreferred*(packages: var Table[string, PackageVersions]
 proc getInstalledMinimalPackages*(options: Options): seq[PackageMinimalInfo] =
   getInstalledPkgsMin(options.getPkgsDir(), options).mapIt(it.getMinimalInfo())
 
-proc collectAllVersions*(versions: var Table[string, PackageVersions], package: PackageMinimalInfo, options: Options, getMinimalPackage: GetPackageMinimal) =
+#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
+  proc getMinimalFromPreferred(pv: PkgTuple): Option[PackageMinimalInfo] =
+    #Before proceding to download we check if the package is in the preferred packages
+    for pp in preferredPackages:
+      if pp.name == pv.name and pp.version.withinRange(pv.ver):
+        return some pp
+    getMinimalPackage(pv, options)
+
   for pv in package.requires:
     # echo "Collecting versions for ", pv.name, " and Version: ", $pv.ver, " via ", package.name
     var pv = pv
     if not hasVersion(versions, pv):  # Not found, meaning this package-version needs to be explored
-      var pkgMin = getMinimalPackage(pv, options).get() #TODO elegantly fail here
+      var pkgMin = getMinimalFromPreferred(pv).get()
       if pv.ver.kind == verSpecial:
         pkgMin.version = newVersion $pv.ver
       if not versions.hasKey(pv.name):
@@ -337,32 +392,37 @@ 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]): HashSet[PackageInfo] = 
+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 = ""
-  var solvedPkgs = pkgVersionTable.getSolvedPackages(output)
-  for pkg, ver in solvedPkgs:
+  solvedPkgs = pkgVersionTable.getSolvedPackages(output)
+  for solvedPkg in solvedPkgs:
     for pkgInfo in pkgList:
-      if pkgInfo.basicInfo.name == pkg and pkgInfo.basicInfo.version == ver:
+      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): (bool, HashSet[PackageInfo]) =
-  var root = rootPkg.getMinimalInfo()
+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()
   root.isRoot = true
   var pkgVersionTable = initTable[string, PackageVersions]()
   pkgVersionTable[root.name] = PackageVersions(pkgName: root.name, versions: @[root])
-  collectAllVersions(pkgVersionTable, root, options, downloadMinimalPackage)
-  var solvedPkgs = pkgVersionTable.getSolvedPackages(output)
-  result[0] = solvedPkgs.len > 0
-  var pkgsToInstall: seq[(string, Version)] = @[]
-  for solvedPkg, ver in solvedPkgs:
-    if solvedPkg == root.name: continue
+  collectAllVersions(pkgVersionTable, root, options, downloadMinimalPackage, pkgList.map(getMinimalInfo))
+  solvedPkgs = pkgVersionTable.getSolvedPackages(output)
+  for solvedPkg in solvedPkgs:
+    if solvedPkg.pkgName == root.name: continue
+    var foundInList = false
     for pkgInfo in pkgList:
-      if pkgInfo.basicInfo.name == solvedPkg: # and pkgInfo.basicInfo.version.withinRange(ver):
-        result[1].incl pkgInfo
-      else:
-        pkgsToInstall.addUnique((solvedPkg, ver))
+      if pkgInfo.basicInfo.name == solvedPkg.pkgName and pkgInfo.basicInfo.version == solvedPkg.version:
+        result.incl pkgInfo
+        foundInList = true
+    if not foundInList:
+      pkgsToInstall.addUnique((solvedPkg.pkgName, solvedPkg.version))
+
+proc getPackageInfo*(dep: string, pkgs: seq[PackageInfo]): Option[PackageInfo] =
+    for pkg in pkgs:
+      if pkg.basicInfo.name.tolower == dep.tolower or pkg.metadata.url == dep:
+        return some pkg 

src/nimblepkg/packageinfo.nim

@@ -180,7 +180,10 @@ proc fetchList*(list: PackageList, options: Options) =
       display("Success", "Package list copied.", Success, HighPriority)
 
   if lastError.len != 0:
-    raise nimbleError("Refresh failed\n" & lastError)
+    if list.name == "local":
+      display("Warning:", lastError & ", discarding.", Warning)
+    else:
+      raise nimbleError("Refresh failed\n" & lastError)
 
   if copyFromPath.len > 0:
     copyFile(copyFromPath,

tests/issue289/issue289.nimble

@@ -10,5 +10,4 @@ bin = @["issue289"]
 # Dependencies
 
 requires "nim >= 0.15.0", "https://github.com/nimble-test/packagea.git 0.6.0"
-requires "https://github.com/nimble-test/packagea.git#head"
 

tests/tissues.nim

@@ -135,6 +135,7 @@ suite "issues":
   test "issues #308 and #515":
     let
       ext = when defined(Windows): ExeExt else: "out"
+    cleanDir(installDir)
     cd "issue308515" / "v1":
       var (output, exitCode) = execNimble(["run", "binname", "--silent"])
       check exitCode == QuitSuccess

tests/tsat.nim

@@ -231,8 +231,10 @@ suite "SAT solver":
     collectAllVersions(pkgVersionTable, root, options, downloadMinimalPackage)
     var output = ""
     let solvedPkgs = pkgVersionTable.getSolvedPackages(output)
-    check solvedPkgs["b"] == newVersion "0.1.4"
-    check solvedPkgs["c"] == newVersion "0.1.0"
+    let pkgB = solvedPkgs.filterIt(it.pkgName == "b")[0]
+    let pkgC = solvedPkgs.filterIt(it.pkgName == "c")[0]
+    check pkgB.pkgName == "b" and pkgB.version == newVersion "0.1.4"
+    check pkgC.pkgName == "c" and pkgC.version == newVersion "0.1.0"
     check "random" in pkgVersionTable
 
     removeDir(options.pkgCachePath)