when in solver:sat sets the right nim version (#1267)

* when in `solver:sat` sets the right `nim` version

* adds missing file

* a more elegant way to retrieve the nim right version

Note we may want to have the solvedPackages inside Options but a refactor would be needed for this and first we should remove the support for the `legacy` solver. Everything is build around it ( i.e. the `legacy` solver doesnt have enough information to split the download stage from the build one)

* improvement

* fixes ci

* indent

* remove unused imports

* remove comment

src/nimble.nim

@@ -71,7 +71,6 @@ proc addReverseDeps(solvedPkgs: seq[SolvedPackage], allPkgsInfo: seq[PackageInfo
         reverseDep.get.isLink = true
       addRevDep(options.nimbleData, solvedPkg.get.basicInfo, reverseDep.get)
 
-var satProccesedPackages: HashSet[PackageInfo]
 proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): HashSet[PackageInfo] = 
   if satProccesedPackages.len > 0:
     return satProccesedPackages
@@ -139,7 +138,7 @@ proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): Has
 
   result = deleteStaleDependencies(result.toSeq, rootPkgInfo, options).toHashSet  
   satProccesedPackages = result
- 
+
   if not solved:
     display("Error", output, Error, priority = HighPriority)
     raise nimbleError("Unsatisfiable dependencies")
@@ -1142,17 +1141,16 @@ proc getNimDir(options: Options): string =
   var nimPkgTupl = projectPkg.requires.filterIt(it.name == "nim")
   if nimPkgTupl.len > 0:
     let reqNimVersion = nimPkgTupl[0].ver
-    let nimBinVersion = options.nimBin.getNimVersionFromBin()
-    if nimBinVersion.isSome and nimBinVersion.get.withinRange(reqNimVersion):
-      return options.nimBin.parentDir
+    if options.nimBin.isSome and options.nimBin.get.version.withinRange(reqNimVersion):
+      return options.nimBin.get.path.parentDir
     var nimPkgInfo = 
       getInstalledPkgsMin(options.getPkgsDir(), options)
         .filterIt(it.basicInfo.name == "nim" and it.withinRange(reqNimVersion))
     nimPkgInfo.sort(proc (a, b: PackageInfo): int = cmp(a.basicInfo.version, b.basicInfo.version), Descending)
     if nimPkgInfo.len > 0:
       let nimBin = nimPkgInfo[0].getNimBin(options)
       return nimBin.parentDir
-  options.nimBin.parentDir
+  return options.nimBin.get(NimBin()).path.parentDir
 
 proc getEntryPoints(pkgInfo: PackageInfo, options: Options): seq[string] =
   ## Returns the entry points for a package. 
@@ -2204,8 +2202,8 @@ proc getAlteredPath(options: Options): string =
       let folder = fullInfo.getOutputDir(bin).parentDir.quoteShell
       paths.add folder
   paths.reverse
-
-  result = fmt "{options.nimBin.parentDir}{separator}{paths.join(separator)}{separator}{getEnv(\"PATH\")}"
+  let parentDir = options.nimBin.get.path.parentDir
+  result = fmt "{parentDir}{separator}{paths.join(separator)}{separator}{getEnv(\"PATH\")}"
 
 proc shellenv(options: var Options) =
   setVerbosity(SilentPriority)
@@ -2369,18 +2367,17 @@ proc doAction(options: var Options) =
 
 proc setNimBin*(options: var Options) =
   # Find nim binary and set into options
-  if options.nimBin.len != 0:
+  if options.nimBin.isSome:
+    let nimBin = options.nimBin.get.path
     # --nim:<path> takes priority...
-    if options.nimBin.splitPath().head.len == 0:
+    if nimBin.splitPath().head.len == 0:
       # Just filename, search in PATH - nim_temp shortcut
-      let pnim = findExe(options.nimBin)
-      if pnim.len != 0: options.nimBin = pnim
-    elif not options.nimBin.isAbsolute():
-      # Relative path
-      options.nimBin = expandTilde(options.nimBin).absolutePath()
+      let pnim = findExe(nimBin)
+      if pnim.len != 0: 
+        options.nimBin = some makeNimBin(pnim)
 
-    if not fileExists(options.nimBin):
-      raise nimbleError("Unable to find `$1`" % options.nimBin)
+    if not fileExists(options.nimBin.get.path):
+      raise nimbleError("Unable to find `$1`" % options.nimBin.get.path)
 
     # when nim is forced via command like don't try to be smart and just return
     # it.
@@ -2407,13 +2404,15 @@ proc setNimBin*(options: var Options) =
         break
 
   # Search PATH to find nim to continue with
-  if options.nimBin.len == 0:
-    options.nimBin = findExe("nim")
+  if options.nimBin.isNone or options.useSystemNim:
+    options.nimBin = some makeNimBin(findExe("nim"))
+    if options.useSystemNim:
+      return
 
   proc install(package: PkgTuple, options: Options): HashSet[PackageInfo] =
     result = install(@[package], options, doPrompt = false, first = false, fromLockFile = false).deps
 
-  if options.nimBin.len == 0:
+  if options.nimBin.isNone:
     # Search installed packages to continue with
     let nimVersion = ("nim", VersionRange(kind: verAny))
     let installedPkgs = getInstalledPkgsMin(options.getPkgsDir(), options)
@@ -2423,11 +2422,11 @@ proc setNimBin*(options: var Options) =
     else:
       # It still no nim found then download and install one to allow parsing of
       # other packages.
-      if options.nimBin.len == 0 and not options.offline and options.prompt("No nim found. Download it now?"):
+      if options.nimBin.isNone and not options.offline and options.prompt("No nim found. Download it now?"):
         for pkg in install(nimVersion, options):
           options.useNimFromDir(pkg.getRealDir, pkg.basicInfo.version.toVersionRange())
 
-  if options.nimBin.len == 0:
+  if options.nimBin.isNone:
     raise nimbleError("Unable to find nim")
 
   # try to switch to the version that is in the develop file

src/nimblepkg/nimblesat.nim

@@ -127,7 +127,7 @@ proc findDependencyForDep(g: DepGraph; dep: string): int {.inline.} =
   result = g.packageToDependency.getOrDefault(dep)
 
 proc createRequirements(pkg: PackageMinimalInfo): Requirements =
-  result.deps = pkg.requires.filterIt(not it.isNim())
+  result.deps = pkg.requires
   result.version = pkg.version
   result.nimVersion = pkg.requires.getNimVersion()
 

src/nimblepkg/nimenv.nim

@@ -1,4 +1,4 @@
-import std/[strscans, os, osproc, strutils, strformat, options]
+import std/[strscans, os, strutils, strformat, options]
 import version, nimblesat, cli, common, options
 
 when defined(windows):
@@ -105,7 +105,7 @@ proc useNimFromDir*(options: var Options, realDir: string, v: VersionRange, tryC
 
   let
     nim = realDir / "bin" / binaryName
-    fileExists = fileExists(options.nimBin)
+    fileExists = fileExists(options.nimBin.get(NimBin()).path)
 
   if not fileExists(nim):
     if tryCompiling and options.prompt("Develop version of nim was found but it is not compiled. Compile it now?"):
@@ -114,21 +114,11 @@ proc useNimFromDir*(options: var Options, realDir: string, v: VersionRange, tryC
       raise nimbleError("Trying to use nim from $1 " % realDir,
                         "If you are using develop mode nim make sure to compile it.")
 
-  options.nimBin = nim
+  options.nimBin = some makeNimBin(nim)
   let separator = when defined(windows): ";" else: ":"
 
   putEnv("PATH", realDir / "bin" & separator & getEnv("PATH"))
   if fileExists:
     display("Info:", "switching to $1 for compilation" % options.nim, priority = HighPriority)
   else:
     display("Info:", "using $1 for compilation" % options.nim, priority = HighPriority)
-
-proc getNimVersionFromBin*(nimBin: string): Option[Version] =
-  let cmd = nimBin & " --version"
-  if nimBin.fileExists:
-    let info = execProcess(cmd)
-    var major, minor, patch: int
-    for line in info.splitLines:
-      if scanf(line, "Nim Compiler Version $i.$i.$i", major, minor, patch):
-        let ver = $major & "." & $minor & "." & $patch
-        return some newVersion(ver)

src/nimblepkg/options.nim

@@ -1,7 +1,7 @@
 # Copyright (C) Dominik Picheta. All rights reserved.
 # BSD License. Look at license.txt for more info.
 
-import json, strutils, os, parseopt, uri, tables, terminal
+import json, strutils, os, parseopt, uri, tables, terminal, osproc, strscans, strformat, sets
 import sequtils, sugar
 import std/options as std_opt
 from httpclient import Proxy, newProxy
@@ -13,6 +13,9 @@ const
   defaultLockFileName* = "nimble.lock"
 
 type
+  NimBin* = object
+    path*: string
+    version*: Version
   DumpMode* = enum kdumpIni, kdumpJson
   Options* = object
     forcePrompts*: ForcePrompt
@@ -41,7 +44,7 @@ type
     dumpMode*: DumpMode
     startDir*: string # Current directory on startup - is top level pkg dir for
                       # some commands, useful when processing deps
-    nimBin*: string # Nim compiler location. Typically accessed via options.nim.
+    nimBin*: Option[NimBin]
     localdeps*: bool # True if project local deps mode
     developLocaldeps*: bool # True if local deps + nimble develop pkg1 ...
     disableSslCertCheck*: bool
@@ -375,10 +378,10 @@ proc promptList*(options: Options, question: string, args: openarray[string]): s
   return promptList(options.forcePrompts, question, args)
 
 proc nim*(options: Options): string =
-  if options.nimBin.len == 0:
+  if options.nimBin.isNone:
     raise nimbleError(
       "Unable to find `nim` binary - add to $PATH or use `--nim`")
-  return options.nimBin
+  return options.nimBin.get().path
 
 proc getNimbleDir*(options: Options): string =
   return options.nimbleDir
@@ -459,16 +462,35 @@ proc parseCommand*(key: string, result: var Options) =
 proc getNimbleFileDir*(pkgInfo: PackageInfo): string =
   pkgInfo.myPath.splitFile.dir
 
+proc getRequiredNimVersion*(pkgInfo: PackageInfo): VersionRange =
+  let nimPkgTupl = pkgInfo.requires.filterIt(it.name == "nim")
+  if nimPkgTupl.len > 0:
+    return nimPkgTupl[0].ver
+  return VersionRange(kind: verAny)
+
 proc getNimBin*(pkgInfo: PackageInfo, options: Options): string =
   if pkgInfo.basicInfo.name == "nim":
     var binaryPath = "bin" / "nim"
     when defined(windows):
       binaryPath &= ".exe"      
-
     result = pkgInfo.getNimbleFileDir() / binaryPath
-    display("Info:", "compiling nim package using $1" % result, priority = HighPriority)
-  else:
+  else: 
+    if options.useSatSolver:
+      #Try to first use nim from the solved packages
+      #TODO add the solved packages to the options (we need to remove the legacy solver first otherwise it will be messy)
+      for pkg in satProccesedPackages:
+        if pkg.basicInfo.name == "nim":
+          return pkg.getNimBin(options)  
+
+    assert options.nimBin.isSome, "Nim binary not set"
+    #Check if the current nim satisfais the pacakge 
+    let nimVer = options.nimBin.get.version
+    let reqNimVer = pkgInfo.getRequiredNimVersion()
+    if not nimVer.withinRange(reqNimVer):
+      display("Warning:", &"Package requires nim {reqNimVer} but {nimVer} found. Attempting to compile with the current nim version.", Warning, HighPriority)
     result = options.nim
+  display("Info:", "compiling nim package using $1" % result, priority = HighPriority)
+
 
 proc getNimBin*(options: Options): string =
   return options.nim
@@ -530,6 +552,29 @@ proc getFlagString(kind: CmdLineKind, flag, val: string): string =
   else:
     return prefix & flag & ":" & val
 
+proc getNimVersionFromBin*(nimBin: string): Option[Version] =
+  let cmd = nimBin & " --version"
+  if nimBin.fileExists:
+    let info = execProcess(cmd)
+    var major, minor, patch: int
+    for line in info.splitLines:
+      if scanf(line, "Nim Compiler Version $i.$i.$i", major, minor, patch):
+        let ver = $major & "." & $minor & "." & $patch
+        return some newVersion(ver)
+
+proc makeNimBin*(path: string, version: Option[Version] = none(Version)): NimBin =
+  var path = path
+  if path == "nim":
+    path = findExe("nim")
+  if path == "nim":
+    raise nimbleError("Unable to find `nim` binary - add to $PATH or use `--nim`")
+  if not path.isAbsolute():
+    path = expandTilde(path).absolutePath()  
+  var version = version
+  if version.isNone:
+    version = getNimVersionFromBin(path)
+  return NimBin(path: path, version: version.get())
+
 proc parseFlag*(flag, val: string, result: var Options, kind = cmdLongOption) =
 
   let f = flag.normalize().replace("-", "")
@@ -548,7 +593,7 @@ proc parseFlag*(flag, val: string, result: var Options, kind = cmdLongOption) =
   of "offline": result.offline = true
   of "nocolor": result.noColor = true
   of "disablevalidation": result.disableValidation = true
-  of "nim": result.nimBin = val
+  of "nim": result.nimBin = some makeNimBin(val)
   of "localdeps", "l": result.localdeps = true
   of "nosslcheck": result.disableSslCertCheck = true
   of "nolockfile": result.disableLockFile = true

src/nimblepkg/packageinfotypes.nim

@@ -89,3 +89,4 @@ type
   PackageDependenciesInfo* = tuple[deps: HashSet[PackageInfo], pkg: PackageInfo]
 
 const noTask* = "" # Means that noTask is being ran. Use this as key for base dependencies
+var satProccesedPackages*: HashSet[PackageInfo]

tests/tsat.nim

@@ -2,7 +2,7 @@
 import unittest, os
 import testscommon
 # from nimblepkg/common import cd Used in the commented tests
-import std/[tables, sequtils, json, jsonutils, strutils, times]
+import std/[tables, sequtils, json, jsonutils, strutils, times, options]
 import nimblepkg/[version, nimblesat, options, config]
 from nimblepkg/common import cd
 
@@ -41,7 +41,7 @@ proc downloadAndStorePackageVersionTableFor(pkgName: string, options: Options) =
 
 proc downloadAllPackages() {.used.} = 
   var options = initOptions()
-  options.nimBin = "nim"
+  options.nimBin = some makeNimBin("nim")
   # options.config.packageLists["uing"] = PackageList(name: pkgName, urls: @[pkgUrl])
   options.config.packageLists["official"] = PackageList(name: "Official", urls: @[
     "https://raw.githubusercontent.com/nim-lang/packages/master/packages.json",
@@ -158,7 +158,7 @@ suite "SAT solver":
     let pkgName: string = "nimlangserver"
     let pv: PkgTuple = (pkgName, VersionRange(kind: verAny))
     var options = initOptions()
-    options.nimBin = "nim"
+    options.nimBin = some makeNimBin("nim")
     options.config.packageLists["official"] = PackageList(name: "Official", urls: @[
       "https://raw.githubusercontent.com/nim-lang/packages/master/packages.json",
       "https://nim-lang.org/nimble/packages.json"
@@ -222,7 +222,7 @@ suite "SAT solver":
       "https://nim-lang.org/nimble/packages.json"
     ])
     options.nimbleDir = getCurrentDir() / "conflictingdepres" / "nimbledeps" 
-    options.nimBin = "nim"
+    options.nimBin = some makeNimBin("nim")
     options.pkgCachePath = getCurrentDir() / "conflictingdepres" / "download"
     let pkgs = getInstalledMinimalPackages(options)
     var pkgVersionTable = initTable[string, PackageVersions]()