decouples download from collecting packages. re-enables local fix to "issue nim-lang#1162"

Author: jmgomez

src/nimble.nim

@@ -81,6 +81,8 @@ proc checkSatisfied(options: Options, dependencies: seq[PackageInfo]) =
           [pkgInfo.basicInfo.name, $currentVer, $pkgsInPath[pkgInfo.basicInfo.name]])
     pkgsInPath[pkgInfo.basicInfo.name] = currentVer
 
+import nimblepkg/nimblesat
+
 proc processFreeDependencies(pkgInfo: PackageInfo,
                              requirements: seq[PkgTuple],
                              options: Options,
@@ -98,6 +100,31 @@ proc processFreeDependencies(pkgInfo: PackageInfo,
     pkgList = initPkgList(pkgInfo, options)
     #At this point we have all the installed packages
     #If we can solve them all here, we just do it and call it a day. 
+    #TODO refactor extract function
+    var root = pkgInfo.getMinimalInfo()
+    root.isRoot = true
+    var pkgVersionTable = initTable[string, PackageVersions]()
+    pkgVersionTable[root.name] = PackageVersions(pkgName: root.name, versions: @[root])
+
+    proc getPackageFromList(pv: PkgTuple, options: Options): Option[PackageMinimalInfo] =
+      for pkg in pkgList:
+        if pkg.basicInfo.name == pv.name and pkg.basicInfo.version.withinRange(pv.ver):
+          return some pkg.getMinimalInfo()
+
+    collectAllVersions(pkgVersionTable, root, options, getPackageFromList)
+
+    var graph = pkgVersionTable.toDepGraph()
+    let form = graph.toFormular()
+    var packages = initTable[string, Version]()
+    solve(graph, form, packages, listVersions= true)
+    for pkg, ver in packages:
+      for pkgInfo in pkgList:
+        if pkgInfo.basicInfo.name == pkg and pkgInfo.basicInfo.version == ver:
+          result.incl pkgInfo
+    if result.len > 0: return result
+      
+      
+
     #TODO REDO with the GraphDep (need to fill the minimal info from the installed packages first)
     # let allPackages = pkgList.mapIt(it.basicInfo)
     # if requirements.areRequirementsWithinPackages(allPackages):

src/nimblepkg/nimblesat.nim

@@ -1,7 +1,7 @@
 import sat/[sat, satvars] 
 import version, packageinfotypes, download, packageinfo, packageparser, options, sha1hashes
 
-import std/[tables, sequtils, algorithm, json, jsonutils, strutils]
+import std/[tables, sequtils, algorithm, json, jsonutils, strutils, options]
 
 
 type  
@@ -50,6 +50,7 @@ type
     reqs*: seq[Requirements]
     packageToDependency*: Table[string, int] #package.name -> index into nodes
     # reqsByDeps: Table[Requirements, int]
+  GetPackageMinimal* = proc (pv: PkgTuple, options: Options): Option[PackageMinimalInfo]
 
 proc getMinimalInfo*(pkg: PackageInfo): PackageMinimalInfo =
   result.name = pkg.basicInfo.name
@@ -80,7 +81,6 @@ proc getNimVersion*(pvs: seq[PkgTuple]): Version =
       else:
         #TODO range
         discard
-      
 
 proc findDependencyForDep(g: DepGraph; dep: string): int {.inline.} =
   assert g.packageToDependency.hasKey(dep), dep & " not found"
@@ -237,4 +237,47 @@ proc solve*(g: var DepGraph; f: Form, packages: var Table[string, Version], list
             if listVersions:
               echo item.pkg, "[ ] " & toString item
   else:
-    debugFormular(g, f, s)
+    debugFormular(g, f, s)
+
+
+#TODO REVIEW this
+proc getDownloadInfo(pv: PkgTuple, options: Options,
+                      doPrompt: bool, ignorePackageCache = false): (DownloadMethod, string,
+                                        Table[string, string]) =
+  if pv.name.isURL:
+    let (url, metadata) = getUrlData(pv.name)
+    return (checkUrlType(url), url, metadata)
+  else:
+    var pkg = initPackage()
+    if getPackage(pv.name, options, pkg, ignorePackageCache):
+      let (url, metadata) = getUrlData(pkg.url)
+      return (pkg.downloadMethod, url, metadata)
+
+proc downloadPkInfoForPv*(pv: PkgTuple, options: Options): PackageInfo  =
+  let (meth, url, metadata) = 
+    getDownloadInfo(pv, options, doPrompt = true)
+  let subdir = metadata.getOrDefault("subdir")
+  let res = 
+    downloadPkg(url, pv.ver, meth, subdir, options,
+                  downloadPath = "", vcsRevision = notSetSha1Hash)
+  return getPkgInfo(res.dir, options)
+
+proc downloadMinimalPackage*(pv: PkgTuple, options: Options): Option[PackageMinimalInfo] =
+  let pkgInfo = downloadPkInfoForPv(pv, options)
+  some pkgInfo.getMinimalInfo()
+
+proc collectAllVersions*(versions: var Table[string, PackageVersions], package: PackageMinimalInfo, options: Options, getMinimalPackage: GetPackageMinimal) =
+  ### 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
+  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
+      if pv.ver.kind == verSpecial:
+        pkgMin.version = newVersion $pv.ver
+      if not versions.hasKey(pv.name):
+        versions[pv.name] = PackageVersions(pkgName: pv.name, versions: @[pkgMin])
+      else:
+        versions[pv.name].versions.addUnique pkgMin
+      collectAllVersions(versions, pkgMin, options, getMinimalPackage)

tests/conflictingdepres/nimbledeps/pkgs2/b-0.1.4-16deae68214d584169ab5461ee97aee557e047fb/b.nimble

@@ -1,6 +1,7 @@
 version       = "0.1.4"
 description   = "c"
 license       = "MIT"
+author        = "Juan M Gomez"
 
 # Dependencies
 

tests/conflictingdepres/nimbledeps/pkgs2/c-0.1.0-16deae68214d584169ab5461ee97aee557e047fb/c.nimble

@@ -1,6 +1,7 @@
 version       = "0.1.0"
 description   = "c"
 license       = "MIT"
+author        = "Juan M Gomez"
 
 # Dependencies
 

tests/conflictingdepres/nimbledeps/pkgs2/c-0.2.1-16deae68214d584169ab5461ee97aee557e047fb/c.nimble

@@ -1,6 +1,7 @@
 version       = "0.2.1"
 description   = "c"
 license       = "MIT"
+author        = "Juan M Gomez"
 
 # Dependencies
 

tests/tsat.nim

@@ -6,17 +6,9 @@ import std/[tables, sequtils, algorithm, json, jsonutils, strutils]
 import nimblepkg/[version, sha1hashes, packageinfotypes, nimblesat, options, 
   download, packageinfo, packageparser, config]
 import nimble
+import times
 
 
-proc downloadPkInfoForPv(pv: PkgTuple, options: Options): PackageInfo  =
-  let (meth, url, metadata) = 
-    getDownloadInfo(pv, options, doPrompt = true)
-  let subdir = metadata.getOrDefault("subdir")
-  let res = 
-    downloadPkg(url, pv.ver, meth, subdir, options,
-                  downloadPath = "", vcsRevision = notSetSha1Hash)
-  return getPkgInfo(res.dir, options)
-
 proc initFromJson*(dst: var PkgTuple, jsonNode: JsonNode, jsonPath: var string) =
   dst = parseRequires(jsonNode.str)
 
@@ -29,21 +21,6 @@ proc toJsonHook*(src: PkgTuple): JsonNode =
   else:
     newJString(src.name & " " & ver)
 
-proc collectAllVersions(versions: var Table[string, PackageVersions], package: PackageInfo, options: 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
-      let pkgInfo = downloadPkInfoForPv(pv, options)
-      var minimalInfo = pkgInfo.getMinimalInfo()
-      if pv.ver.kind == verSpecial:
-        echo "Special version ", pv, " but it was ", minimalInfo.version
-        minimalInfo.version = newVersion $pv.ver
-      if not versions.hasKey(pv.name):
-        versions[pv.name] = PackageVersions(pkgName: pv.name, versions: @[minimalInfo])
-      else:
-        versions[pv.name].versions.addUnique minimalInfo
-      collectAllVersions(versions, pkgInfo, options)
 
 
 #Test utils:
@@ -62,7 +39,7 @@ proc downloadAndStorePackageVersionTableFor(pkgName: string, options: Options) =
   var root = pkgInfo.getMinimalInfo()
   root.isRoot = true
   var pkgVersionTable = initTable[string, PackageVersions]()
-  collectAllVersions(pkgVersionTable, pkgInfo, options)
+  collectAllVersions(pkgVersionTable, root, options, downloadMinimalPackage)
   pkgVersionTable[pkgName] = PackageVersions(pkgName: pkgName, versions: @[root])
   let json = pkgVersionTable.toJson()
   writeFile(path, json.pretty())
@@ -166,22 +143,12 @@ suite "SAT solver":
     echo packages
     check packages.len == 0
 
-  # test "issue #1162":
-  #   cd "conflictingdepres":
-  #     #integration version of the test above
-  #     #TODO document folder structure setup so others know how to run similar tests
-  #     let (_, exitCode) = execNimble("install", "-l")
-  #     check exitCode == QuitSuccess
-
-#[
-  Next download all packages store it in a json and solve the dependencies one by one. 
-
-  TODO
-    - Create the table from already downloaded packages.
-    - See if downloads can be cached and reused. 
-    - Review if when downloading a package we could just navigate it to get all versions without triggering another download
-    
-]#
+  test "issue #1162":
+    cd "conflictingdepres":
+      #integration version of the test above
+      #TODO document folder structure setup so others know how to run similar tests
+      let (_, exitCode) = execNimble("install", "-l")
+      check exitCode == QuitSuccess
 
   # test "should be able to download a package and select its deps":
 
@@ -198,7 +165,7 @@ suite "SAT solver":
   #   var root = pkgInfo.getMinimalInfo()
   #   root.isRoot = true
   #   var pkgVersionTable = initTable[string, PackageVersions]()
-  #   collectAllVersions(pkgVersionTable, pkgInfo, options)
+  #   collectAllVersions(pkgVersionTable, root, options, downloadMinimalPackage)
   #   pkgVersionTable[pkgName] = PackageVersions(pkgName: pkgName, versions: @[root])
 
   #   var graph = pkgVersionTable.toDepGraph()
@@ -209,14 +176,19 @@ suite "SAT solver":
 
 
   test "should be solve all nimble packages":
-    downloadAllPackages() #uncomment this to download all packages. It's better to just keep them cached as it takes a while.
-
+    # downloadAllPackages() #uncomment this to download all packages. It's better to just keep them cached as it takes a while.
+    let now = now()
+    var pks = 0
     for jsonFile in walkPattern("packageMinimal/*.json"):
+      inc pks
       var pkgVersionTable = parseJson(readFile(jsonFile)).to(Table[string, PackageVersions])
       var graph = pkgVersionTable.toDepGraph()
       let form = toFormular(graph)
       var packages = initTable[string, Version]()
       solve(graph, form, packages, listVersions= false)
-      echo "Solved ", jsonFile.extractFilename, " with ", packages.len, " packages"
+      # echo "Solved ", jsonFile.extractFilename, " with ", packages.len, " packages"
 
-      check packages.len > 0
+      check packages.len > 0
+    
+    let ends = now()
+    echo "Solved ", pks, " packages in ", ends - now, " seconds"