Fixes nimble hanging when there is a cycle in the requirements #Fixes #1307 (#1312)

* Fixes nimble hanging when there is a cycle in the requirements #Fixes #1307

* hashes the actual req

Author: jmgomez

src/nimblepkg/nimblesat.nim

@@ -598,33 +598,37 @@ proc fillPackageTableFromPreferred*(packages: var Table[string, PackageVersions]
 proc getInstalledMinimalPackages*(options: Options): seq[PackageMinimalInfo] =
   getInstalledPkgsMin(options.getPkgsDir(), options).mapIt(it.getMinimalInfo(options))
 
+proc getMinimalFromPreferred(pv: PkgTuple,  getMinimalPackage: GetPackageMinimal, preferredPackages: seq[PackageMinimalInfo], options: Options): seq[PackageMinimalInfo] =
+  for pp in preferredPackages:
+    if pp.name == pv.name and pp.version.withinRange(pv.ver):
+      return @[pp]
+  getMinimalPackage(pv, options)
+
+proc processRequirements(versions: var Table[string, PackageVersions], pv: PkgTuple, visited: var HashSet[PkgTuple],  getMinimalPackage: GetPackageMinimal, preferredPackages: seq[PackageMinimalInfo] = newSeq[PackageMinimalInfo](), options: Options) =
+  if pv in visited:
+    return
+  
+  visited.incl pv
+  
+  if not hasVersion(versions, pv):
+    var pkgMins = getMinimalFromPreferred(pv, getMinimalPackage, preferredPackages, options)
+    for pkgMin in pkgMins.mitems:
+      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
+      
+      # Process requirements
+      for req in pkgMin.requires:
+        processRequirements(versions, req, visited, getMinimalPackage, preferredPackages, options)
 
 proc collectAllVersions*(versions: var Table[string, PackageVersions], package: PackageMinimalInfo, options: Options, getMinimalPackage: GetPackageMinimal, preferredPackages: seq[PackageMinimalInfo] = newSeq[PackageMinimalInfo]()) =
-  proc getMinimalFromPreferred(pv: PkgTuple): seq[PackageMinimalInfo] =
-    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)
-
-  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:
-          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
-        
-        # Process requirements from both the package and GetMinimalPackage results
-        for req in pkgMin.requires:
-          # echo "Processing requirement: ", req.name, " ", $req.ver
-          processRequirements(versions, req)
 
+  var visited = initHashSet[PkgTuple]()
   for pv in package.requires:
-    processRequirements(versions, pv)
+    processRequirements(versions, pv, visited, getMinimalPackage, preferredPackages, options)
 
 proc topologicalSort*(solvedPkgs: seq[SolvedPackage]): seq[SolvedPackage] =
   var inDegree = initTable[string, int]()

src/nimblepkg/version.nim

@@ -465,3 +465,5 @@ when isMainModule:
     test "convert version to version range":
       check toVersionRange(newVersion("#head")).kind == verSpecial
       check toVersionRange(newVersion("0.2.0")).kind == verEq
+
+proc hash*(pv: PkgTuple): Hash = hash($pv)

tests/sattests/cycletest/cycletest.nimble

@@ -0,0 +1,12 @@
+# Package
+
+version       = "0.1.0"
+author        = "jmgomez"
+description   = "A new awesome nimble package"
+license       = "MIT"
+srcDir        = "src"
+
+
+# Dependencies
+
+requires "https://github.com/disruptek/frosty"

tests/sattests/cycletest/src/cycletest.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/sattests/cycletest/src/cycletest/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 cycletest/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/sattests/cycletest/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 cycletest
+test "can add":
+  check add(5, 5) == 10

tests/tsat.nim

@@ -440,3 +440,9 @@ suite "SAT solver":
       removeDir("nimbledeps")
       let (_, exitCode) = execNimbleYes("install", "-l")
       check exitCode == QuitSuccess
+
+  test "should be able to install packages with cycles in the requirements":
+    cd "sattests" / "cycletest":
+      removeDir("nimbledeps")
+      let (_, exitCode) = execNimbleYes("install", "-l")
+      check exitCode == QuitSuccess