SAT Progress (#1217)

* *Most* tests are green now * removes unused param * fix test * Prevents CI failing due to an unused module in Win * Update nimblesat.nim * Update src/nimble.nim Co-authored-by: Andreas Rumpf <rumpf_a@web.de> * Update src/nimble.nim Co-authored-by: Andreas Rumpf <rumpf_a@web.de> * Update nimble.nim * Apply suggestions from code review --------- Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
nim-lang · May 2, 2024 · 7aea7dd · 7aea7dd
1 parent 59dc6c9
commit 7aea7dd
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 33 deletions.
diff --git a/src/nimble.nim b/src/nimble.nim
@@ -72,47 +72,64 @@ proc addReverseDeps(solvedPkgs: seq[SolvedPackage], allPkgsInfo: seq[PackageInfo
         reverseDep.get.isLink = true
       addRevDep(options.nimbleData, solvedPkg.get.basicInfo, reverseDep.get)
 
-proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo], options: Options): HashSet[PackageInfo] = 
+var satProccesedPackages: HashSet[PackageInfo]
+proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): HashSet[PackageInfo] = 
+  if satProccesedPackages.len > 0:
+    return satProccesedPackages
   var solvedPkgs = newSeq[SolvedPackage]()
   var pkgsToInstall: seq[(string, Version)] = @[]
+  var pkgList = initPkgList(rootPkgInfo, 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])
+    pkgList = pkgList.filterIt(it.basicInfo.name notin toUpgradeNames)
+
+    # rootPkgInfo.requires = rootPkgInfo.requires.filterIt(it.name notin toUpgradeNames)
+    # rootPkgInfo.requires &= options.action.packages
 
   result = solveLocalPackages(rootPkgInfo, pkgList, solvedPkgs)
   if solvedPkgs.len > 0: 
     displaySatisfiedMsg(solvedPkgs, pkgsToInstall)
     addReverseDeps(solvedPkgs, allPkgsInfo, options)
+    for pkg in allPkgsInfo:
+      result.incl pkg
+    result = 
+      result.toSeq
+      .deleteStaleDependencies(rootPkgInfo, options)
+      .toHashSet
+    satProccesedPackages = result
     return result
 
   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,
+  for (name, ver) in pkgsToInstall:
+    let resolvedDep = ((name: name, ver: ver.toVersionRange)).resolveAlias(options)
+    let (packages, _) = install(@[resolvedDep], options,
       doPrompt = false, first = false, fromLockFile = false, preferredPackages = @[])
     for pkg in packages:
-      if result.contains pkg:
+      if pkg in result:
         # 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)
 
+  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,
@@ -126,11 +143,10 @@ proc processFreeDependencies(pkgInfo: PackageInfo,
          "processFreeDependencies needs pkgInfo.requires"
 
   var pkgList {.global.}: seq[PackageInfo]
-
   once: 
     pkgList = initPkgList(pkgInfo, options)
     if options.useSatSolver:
-      return processFreeDependenciesSAT(pkgInfo, pkgList, options)
+      return processFreeDependenciesSAT(pkgInfo, options)
 
   display("Verifying", "dependencies for $1@$2" %
           [pkgInfo.basicInfo.name, $pkgInfo.basicInfo.version],
@@ -477,19 +493,20 @@ proc installFromDir(dir: string, requestedVer: VersionRange, options: Options,
     priority = HighPriority)
 
   let oldPkg = pkgInfo.packageExists(options)
-  if oldPkg.isSome and not options.useSatSolver:
+  if oldPkg.isSome:
     # 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))
-    var oldPkg = oldPkg.get
-    oldPkg.metaData.specialVersions.incl pkgInfo.metaData.specialVersions
-    saveMetaData(oldPkg.metaData, oldPkg.getNimbleFileDir, changeRoots = false)
-    if result.deps.contains oldPkg:
-      result.deps[oldPkg].metaData.specialVersions.incl(
-        oldPkg.metaData.specialVersions)
-    result.deps.incl oldPkg
-    result.pkg = oldPkg
-    return
+    if not options.useSatSolver: #The dep path is not created when using the sat solver as packages are collected upfront
+      var oldPkg = oldPkg.get
+      oldPkg.metaData.specialVersions.incl pkgInfo.metaData.specialVersions
+      saveMetaData(oldPkg.metaData, oldPkg.getNimbleFileDir, changeRoots = false)
+      if result.deps.contains oldPkg:
+        result.deps[oldPkg].metaData.specialVersions.incl(
+          oldPkg.metaData.specialVersions)
+      result.deps.incl oldPkg
+      result.pkg = oldPkg
+      return result
 
   # nim is intended only for local project local usage, so avoid installing it
   # in .nimble/bin
@@ -1792,7 +1809,6 @@ proc getDependenciesForLocking(pkgInfo: PackageInfo, options: Options):
 
       allRequiredPackages = pkgInfo.processFreeDependencies(toUpgrade, options, res).toSeq
       allRequiredNames = allRequiredPackages.mapIt(it.name)
-
     res = res.filterIt(it.name notin allRequiredNames)
     res.add allRequiredPackages
 
@@ -1805,8 +1821,12 @@ proc lock(options: Options) =
     currentDir = getCurrentDir()
     pkgInfo = getPkgInfo(currentDir, options)
     currentLockFile = options.lockFile(currentDir)
-    lockExists = displayLockOperationStart(currentLockFile)
-    baseDeps = pkgInfo.getDependenciesForLocking(options) # Deps shared by base and tasks
+    lockExists = displayLockOperationStart(currentLockFile)      
+    baseDeps = 
+      if options.useSATSolver:
+        processFreeDependenciesSAT(pkgInfo, options).toSeq
+      else:
+        pkgInfo.getDependenciesForLocking(options) # Deps shared by base and tasks  
     baseDepNames: HashSet[string] = baseDeps.mapIt(it.name).toHashSet
 
   pkgInfo.validateDevelopDependenciesVersionRanges(baseDeps, options)

diff --git a/src/nimblepkg/nimblesat.nim b/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]
 
@@ -323,7 +323,8 @@ proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output:
         result.add solvedPkg
 
 proc getCacheDownloadDir*(url: string, ver: VersionRange, options: Options): string =
-  options.pkgCachePath / getDownloadDirName(url, ver, notSetSha1Hash)
+  return ""
+  # options.pkgCachePath / getDownloadDirName(url, ver, notSetSha1Hash)
 
 proc downloadPkInfoForPv*(pv: PkgTuple, options: Options): PackageInfo  =
   let (meth, url, metadata) = 
@@ -405,13 +406,45 @@ proc solveLocalPackages*(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo], so
       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]]()
+  var zeroInDegree: seq[string] = @[]  
+  # Initialize in-degree and adjacency list using requirements
+  for pkg in solvedPkgs:
+    if not inDegree.hasKey(pkg.pkgName):
+      inDegree[pkg.pkgName] = 0  # Ensure every package is in the inDegree table
+    for dep in pkg.requirements:
+      if dep.name notin adjList:
+        adjList[dep.name] = @[pkg.pkgName]  
+      else:
+        adjList[dep.name].add(pkg.pkgName)  
+      inDegree[pkg.pkgName].inc  # Increase in-degree of this pkg since it depends on dep
+
+  # Find all nodes with zero in-degree
+  for (pkgName, degree) in inDegree.pairs:
+    if degree == 0:
+      zeroInDegree.add(pkgName)
+
+  # Perform the topological sorting
+  while zeroInDegree.len > 0:
+    let current = zeroInDegree.pop()
+    let currentPkg = solvedPkgs.filterIt(it.pkgName == current)[0]
+    result.add(currentPkg)
+    for neighbor in adjList.getOrDefault(current, @[]):
+      inDegree[neighbor] -= 1
+      if inDegree[neighbor] == 0:
+        zeroInDegree.add(neighbor) 
+
+
 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, pkgList.map(getMinimalInfo))
-  solvedPkgs = pkgVersionTable.getSolvedPackages(output)
+  solvedPkgs = pkgVersionTable.getSolvedPackages(output).topologicalSort()
+
   for solvedPkg in solvedPkgs:
     if solvedPkg.pkgName == root.name: continue
     var foundInList = false
@@ -425,4 +458,4 @@ proc solvePackages*(rootPkg: PackageInfo, pkgList: seq[PackageInfo], pkgsToInsta
 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 
+        return some pkg 
diff --git a/tests/tlockfile.nim b/tests/tlockfile.nim
@@ -325,7 +325,9 @@ requires "nim >= 1.5.1"
                                 mainPkgName, mainPkgRepoPath, "< 0.1.0")
           ]
         check output.processOutput.inLines(
-          invalidDevelopDependenciesVersionsMsg(errors))
+          invalidDevelopDependenciesVersionsMsg(errors)) or
+          output.processOutput.inLines(
+          "Downloaded package's version does not satisfy requested version range: wanted > 0.1.0 got 0.1.0.")
 
   test "can download locked dependencies":
     cleanUp()

diff --git a/tests/tshellenv.nim b/tests/tshellenv.nim
@@ -6,14 +6,19 @@
 import unittest, os, osproc, strutils
 import testscommon
 from nimblepkg/common import cd
+when not defined windows:
+  import std/sequtils
 
 const
   separator = when defined(windows): ";" else: ":"
 
 suite "Shell env":
   test "Shell env":
     cd "shellenv":
-      let (output, exitCode) = execCmdEx(nimblePath & " shellenv")
+      var (output, exitCode) = execCmdEx(nimblePath & " shellenv")
+      when not defined windows:
+        #Skips potential linker warning in some MacOs versions 
+        output = output.splitLines.toSeq.filterIt("export" in it)[0]
       check exitCode == QuitSuccess
       let
         prefixValPair = split(output, "=")

diff --git a/tests/tuninstall.nim b/tests/tuninstall.nim
@@ -34,7 +34,8 @@ suite "uninstall":
     let ls = outp.strip.processOutput()
     check exitCode != QuitSuccess
     check ls.inLines(cannotSatisfyMsg("0.2.0", "0.5.0")) or
-          ls.inLines(cannotSatisfyMsg("0.5.0", "0.2.0"))
+          ls.inLines(cannotSatisfyMsg("0.5.0", "0.2.0")) or
+          ls.inLines("Unsatisfiable dependencies")
 
   proc setupIssue27Packages() =
     # Install b