Enumerate tagged versions upfront (nim-lang#1289)

* implements getPackageMinimalVersionsFromRepo

* SAT solver now can fallback to a previous version of a compatible dependency

* progress

* rename test

* fixes test compilation

* progress

* collectAllVersion now collects requirements from releases discovered through tags. minimal from git also restore the current package even if it isnt released

* fix test

* Dont traverse additional versions when the dep is fixed

* retrigger ci

src/nimblepkg/download.nim

@@ -19,7 +19,7 @@ proc updateSubmodules(dir: string) =
   discard tryDoCmdEx(
     &"git -C {dir} submodule update --init --recursive --depth 1")
 
-proc doCheckout(meth: DownloadMethod, downloadDir, branch: string) =
+proc doCheckout*(meth: DownloadMethod, downloadDir, branch: string) =
   case meth
   of DownloadMethod.git:
     # Force is used here because local changes may appear straight after a clone
@@ -46,7 +46,15 @@ proc doClone(meth: DownloadMethod, url, downloadDir: string, branch = "",
       branchArg = if branch == "": "" else: &"-b {branch}"
     discard tryDoCmdEx(&"hg clone {tipArg} {branchArg} {url} {downloadDir}")
 
-proc getTagsList(dir: string, meth: DownloadMethod): seq[string] =
+proc gitFetchTags*(repoDir: string, downloadMethod: DownloadMethod) =
+  case downloadMethod:
+    of DownloadMethod.git:
+      tryDoCmdEx(&"git -C {repoDir} fetch --tags")
+    of DownloadMethod.hg:
+      assert false, "hg not supported"
+
+
+proc getTagsList*(dir: string, meth: DownloadMethod): seq[string] =
   var output: string
   cd dir:
     case meth

src/nimblepkg/nimblesat.nim

@@ -1,6 +1,6 @@
 import sat/[sat, satvars] 
 import version, packageinfotypes, download, packageinfo, packageparser, options, 
-  sha1hashes, tools, downloadnim
+  sha1hashes, tools, downloadnim, cli
 
 import std/[tables, sequtils, algorithm, sets, strutils, options, strformat, os]
 
@@ -315,10 +315,33 @@ proc findMinimalFailingSet*(g: var DepGraph): tuple[failingSet: seq[PkgTuple], o
 
   (minimalFailingSet, output)
 
-#It may be better to just use result here
-proc solve*(g: var DepGraph; f: Form, packages: var Table[string, Version], output: var string): bool =
+proc filterSatisfiableDeps(g: DepGraph, node: Dependency): seq[DependencyVersion] =
+  ## Returns a sequence of versions from the node that have satisfiable dependencies
+  result = @[]
+  for v in node.versions:
+    let reqs = g.reqs[v.req].deps
+    var hasUnsatisfiableDep = false
+    for req in reqs:
+      let depIdx = findDependencyForDep(g, req.name)
+      if depIdx >= 0:
+        var canSatisfy = false
+        for depVer in g.nodes[depIdx].versions:
+          if depVer.version.withinRange(req.ver):
+            canSatisfy = true
+            break
+        if not canSatisfy:
+          hasUnsatisfiableDep = true
+          break
+    if not hasUnsatisfiableDep:
+      result.add(v)
+
+const MaxSolverRetries = 100
+
+proc solve*(g: var DepGraph; f: Form, packages: var Table[string, Version], output: var string, 
+           retryCount = 0): bool =
   let m = f.idgen
   var s = createSolution(m)
+
   if satisfiable(f.f, s):
     for n in mitems g.nodes:
       if n.isRoot: n.active = true
@@ -330,18 +353,33 @@ proc solve*(g: var DepGraph; f: Form, packages: var Table[string, Version], outp
         g.nodes[idx].activeVersion = m.index
 
     for n in items g.nodes:
-        for v in items(n.versions):
-          let item = f.mapping[v.v]
-          if s.isTrue(v.v):
-            packages[item.pkg] = item.version
-            output.add &"item.pkg  [x]  {toString item} \n"
-          else:
-            output.add &"item.pkg  [ ]  {toString item} \n"
-    # echo output
-    true
+      for v in items(n.versions):
+        let item = f.mapping[v.v]
+        if s.isTrue(v.v):
+          packages[item.pkg] = item.version
+          output.add &"item.pkg  [x]  {toString item} \n"
+        else:
+          output.add &"item.pkg  [ ]  {toString item} \n"
+    return true
   else:
     let (failingSet, errorMsg) = findMinimalFailingSet(g)
+    if retryCount >= MaxSolverRetries:
+      output = &"Max retry attempts ({MaxSolverRetries}) exceeded while trying to resolve dependencies \n"
+      output.add errorMsg
+      return false
+
     if failingSet.len > 0:
+      var newGraph = g
+      for pkg in failingSet:
+        let idx = findDependencyForDep(newGraph, pkg.name)
+        if idx >= 0:
+          # echo "Retry #", retryCount + 1, ": Checking package ", pkg.name, " version ", pkg.ver
+          let newVersions = filterSatisfiableDeps(newGraph, newGraph.nodes[idx])
+          if newVersions.len > 0 and newVersions != newGraph.nodes[idx].versions:
+            newGraph.nodes[idx].versions = newVersions
+            let newForm = toFormular(newGraph)
+            return solve(newGraph, newForm, packages, output, retryCount + 1)
+
       output = errorMsg
     else:
       output = generateUnsatisfiableMessage(g, f, s)
@@ -362,7 +400,12 @@ proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output:
     for ver in p.versions.items:
       for dep, q in items graph.reqs[ver.req].deps:
         if dep notin graph.packageToDependency:
+          #debug print. show all packacges in the graph
           output.add &"Dependency {dep} not found in the graph \n"
+          for k, v in pkgVersionTable:
+            output.add &"Package {k} \n"
+            for v in v.versions:
+              output.add &"\t \t Version {v.version} requires: {v.requires} \n" 
           return newSeq[SolvedPackage]()
 
   let form = toFormular(graph)
@@ -384,27 +427,55 @@ proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output:
 proc getCacheDownloadDir*(url: string, ver: VersionRange, options: Options): string =
   options.pkgCachePath / getDownloadDirName(url, ver, notSetSha1Hash)
 
-proc downloadPkInfoForPv*(pv: PkgTuple, options: Options): PackageInfo  =
+proc downloadPkgFromUrl*(pv: PkgTuple, options: Options): (DownloadPkgResult, DownloadMethod) = 
   let (meth, url, metadata) = 
-    getDownloadInfo(pv, options, doPrompt = false, ignorePackageCache = false)
+      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,
-                  downloadDir, vcsRevision = notSetSha1Hash)
-  return getPkgInfo(res.dir, options)
+  let downloadRes = downloadPkg(url, pv.ver, meth, subdir, options,
+                downloadDir, vcsRevision = notSetSha1Hash)
+  (downloadRes, meth)
+
+proc downloadPkInfoForPv*(pv: PkgTuple, options: Options): PackageInfo  =
+  downloadPkgFromUrl(pv, options)[0].dir.getPkgInfo(options)
 
 proc getAllNimReleases(options: Options): seq[PackageMinimalInfo] =
   let releases = getOfficialReleases(options)
   for release in releases:
     result.add PackageMinimalInfo(name: "nim", version: release)
+
+proc getPackageMinimalVersionsFromRepo*(repoDir, pkgName: string, downloadMethod: DownloadMethod, options: Options): seq[PackageMinimalInfo] =
+  #This is expensive. We need to cache it. Potentially it could be also run in parallel
+  # echo &"Discovering version for {pkgName}"
+  gitFetchTags(repoDir, downloadMethod)       
+  #First package must be the current one
+  result.add getPkgInfo(repoDir, options).getMinimalInfo(options)
+  let tags = getTagsList(repoDir, downloadMethod).getVersionList()
+  var checkedTags = 0
+  for (ver, tag) in tags.pairs:    
+    if options.maxTaggedVersions > 0 and checkedTags >= options.maxTaggedVersions:
+      # echo &"Tag limit reached for {pkgName}"
+      break
+    inc checkedTags
+    #For each version, we need to parse the requires so we need to checkout and initialize the repo
+    try:
+      doCheckout(downloadMethod, repoDir, tag)
+      let nimbleFile = findNimbleFile(repoDir, true, options)
+      let pkgInfo = getPkgInfoFromFile(nimbleFile, options, useCache=false)
+      let minimalInfo = pkgInfo.getMinimalInfo(options)
+      result.addUnique minimalInfo
+    except CatchableError as e:
+      displayWarning(&"Error reading tag {tag}: for package {pkgName}. This may not be relevant as it could be an old version of the package. \n {e.msg}", HighPriority)
 
 proc downloadMinimalPackage*(pv: PkgTuple, options: Options): seq[PackageMinimalInfo] =
   if pv.name == "": return newSeq[PackageMinimalInfo]()
   if pv.isNim and not options.disableNimBinaries: return getAllNimReleases(options)
-
-  let pkgInfo = downloadPkInfoForPv(pv, options)
-  return @[pkgInfo.getMinimalInfo(options)]
+  if pv.ver.kind in [verSpecial, verEq]: #if special or equal, we dont retrieve more versions as we only need one.
+    result = @[downloadPkInfoForPv(pv, options).getMinimalInfo(options)]
+  else:
+    let (downloadRes, downloadMeth) = downloadPkgFromUrl(pv, options)
+    result = getPackageMinimalVersionsFromRepo(downloadRes.dir, pv.name, downloadMeth, options)
+  # echo "Downloading minimal package for ", pv.name, " ", $pv.ver, result
 
 proc fillPackageTableFromPreferred*(packages: var Table[string, PackageVersions], preferredPackages: seq[PackageMinimalInfo]) =
   for pkg in preferredPackages:
@@ -417,20 +488,17 @@ proc fillPackageTableFromPreferred*(packages: var Table[string, PackageVersions]
 proc getInstalledMinimalPackages*(options: Options): seq[PackageMinimalInfo] =
   getInstalledPkgsMin(options.getPkgsDir(), options).mapIt(it.getMinimalInfo(options))
 
-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 collectAllVersions*(versions: var Table[string, PackageVersions], package: PackageMinimalInfo, options: Options, getMinimalPackage: GetPackageMinimal, preferredPackages: seq[PackageMinimalInfo] = newSeq[PackageMinimalInfo]()) =
   proc getMinimalFromPreferred(pv: PkgTuple): seq[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 @[pp]
+    # echo "Getting minimal from getMinimalPackage for ", pv.name, " ", $pv.ver
     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
+  proc processRequirements(versions: var Table[string, PackageVersions], pv: PkgTuple) =
+    if not hasVersion(versions, pv):
       var pkgMins = getMinimalFromPreferred(pv)
       for pkgMin in pkgMins.mitems:
         if pv.ver.kind == verSpecial:
@@ -439,8 +507,14 @@ proc collectAllVersions*(versions: var Table[string, PackageVersions], package:
           versions[pv.name] = PackageVersions(pkgName: pv.name, versions: @[pkgMin])
         else:
           versions[pv.name].versions.addUnique pkgMin
-        #TODO Note for when implementing "enumerate all versions": do not enter in the loop until we have collected all the versions
-        collectAllVersions(versions, pkgMin, options, getMinimalPackage, preferredPackages)
+
+        # Process requirements from both the package and GetMinimalPackage results
+        for req in pkgMin.requires:
+          # echo "Processing requirement: ", req.name, " ", $req.ver
+          processRequirements(versions, req)
+
+  for pv in package.requires:
+    processRequirements(versions, pv)
 
 proc topologicalSort*(solvedPkgs: seq[SolvedPackage]): seq[SolvedPackage] =
   var inDegree = initTable[string, int]()
@@ -510,4 +584,4 @@ proc getPackageInfo*(name: string, pkgs: seq[PackageInfo], version: Option[Versi
           if pkg.basicInfo.version == version.get:
             return some pkg
         else: #No version passed over first match
-          return some pkg
+          return some pkg

src/nimblepkg/options.nim

@@ -63,6 +63,7 @@ type
     extraRequires*: seq[PkgTuple] # extra requires parsed from the command line
     nimBinariesDir*: string # Directory where nim binaries are stored. Separated from nimbleDir as it can be changed by the user/tests
     disableNimBinaries*: bool # Whether to disable the use of nim binaries
+    maxTaggedVersions*: int # Maximum number of tags to check for a package when discovering versions in a local repo
 
   ActionType* = enum
     actionNil, actionRefresh, actionInit, actionDump, actionPublish, actionUpgrade
@@ -747,7 +748,8 @@ proc initOptions*(): Options =
     verbosity: HighPriority,
     noColor: not isatty(stdout),
     startDir: getCurrentDir(),
-    nimBinariesDir: getHomeDir() / ".nimble" / "nimbinaries"
+    nimBinariesDir: getHomeDir() / ".nimble" / "nimbinaries", 
+    maxTaggedVersions: 2 #TODO increase once we have a cache
   )
 
 proc handleUnknownFlags(options: var Options) =

src/nimblepkg/packageparser.nim

@@ -272,7 +272,7 @@ proc inferInstallRules(pkgInfo: var PackageInfo, options: Options) =
     if fileExists(pkgInfo.getRealDir() / pkgInfo.basicInfo.name.addFileExt("nim")):
       pkgInfo.installFiles.add(pkgInfo.basicInfo.name.addFileExt("nim"))
 
-proc readPackageInfo(pkgInfo: var PackageInfo, nf: NimbleFile, options: Options, onlyMinimalInfo=false) =
+proc readPackageInfo(pkgInfo: var PackageInfo, nf: NimbleFile, options: Options, onlyMinimalInfo=false, useCache=true) =
   ## Reads package info from the specified Nimble file.
   ##
   ## Attempts to read it using the "old" Nimble ini format first, if that
@@ -289,7 +289,7 @@ proc readPackageInfo(pkgInfo: var PackageInfo, nf: NimbleFile, options: Options,
   assert fileExists(nf)
 
   # Check the cache.
-  if options.pkgInfoCache.hasKey(nf):
+  if useCache and options.pkgInfoCache.hasKey(nf):
     pkgInfo = options.pkgInfoCache[nf]
     return
   pkgInfo = initPackageInfo(options, nf)
@@ -369,12 +369,12 @@ proc readPackageInfo(pkgInfo: var PackageInfo, nf: NimbleFile, options: Options,
     validatePackageInfo(pkgInfo, options)
 
 proc getPkgInfoFromFile*(file: NimbleFile, options: Options,
-                         forValidation = false): PackageInfo =
+                         forValidation = false, useCache = true): PackageInfo =
   ## Reads the specified .nimble file and returns its data as a PackageInfo
   ## object. Any validation errors are handled and displayed as warnings.
   result = initPackageInfo()
   try:
-    readPackageInfo(result, file, options)
+    readPackageInfo(result, file, options, useCache= useCache)
   except ValidationError:
     let exc = (ref ValidationError)(getCurrentException())
     if exc.warnAll and not forValidation:

tests/oldnimble/oldnimble.nimble

@@ -0,0 +1,13 @@
+# Package
+
+version       = "0.1.0"
+author        = "jmgomez"
+description   = "A new awesome nimble package"
+license       = "MIT"
+srcDir        = "src"
+
+
+# Dependencies
+
+requires "nim >= 2.0.11"
+requires "nimble <= 0.16.2" #We know this nimble version has additional requirements (new nimble use submodules)

tests/oldnimble/src/oldnimble.nim

@@ -0,0 +1,7 @@
+# This is just an example to get you started. A typical library package
+# exports the main API in this file. Note that you cannot rename this file
+# but you can remove it if you wish.
+
+proc add*(x, y: int): int =
+  ## Adds two numbers together.
+  return x + y

tests/oldnimble/src/oldnimble/submodule.nim

@@ -0,0 +1,12 @@
+# This is just an example to get you started. Users of your library will
+# import this file by writing ``import oldnimble/submodule``. Feel free to rename or
+# remove this file altogether. You may create additional modules alongside
+# this file as required.
+
+type
+  Submodule* = object
+    name*: string
+
+proc initSubmodule*(): Submodule =
+  ## Initialises a new ``Submodule`` object.
+  Submodule(name: "Anonymous")

tests/oldnimble/tests/test1.nim

@@ -0,0 +1,12 @@
+# This is just an example to get you started. You may wish to put all of your
+# tests into a single file, or separate them into multiple `test1`, `test2`
+# etc. files (better names are recommended, just make sure the name starts with
+# the letter 't').
+#
+# To run these tests, simply execute `nimble test`.
+
+import unittest
+
+import oldnimble
+test "can add":
+  check add(5, 5) == 10