The develop command now works with SAT on (tdevelopfeature green) (#…

…1210)

* fixes develop

* removes unnecessary func

* fixes indent

* treversedeps green when SAT on

* removes redundant packages

* Improves the download cache, doenst re-downloads pkgs when installing

* adds cleanDirs to test so it doesnt fail in subsequent runs

* makes another test green

* Fixes ilegal package setup for a test

* Dont error on local package list

src/nimble.nim

@@ -54,51 +54,65 @@ proc checkSatisfied(options: Options, dependencies: seq[PackageInfo]) =
  [pkgInfo.basicInfo.name, $currentVer, $pkgsInPath[pkgInfo.basicInfo.name]])
  pkgsInPath[pkgInfo.basicInfo.name] = currentVer
 
+proc displaySatisfiedMsg(solvedPkgs: seq[SolvedPackage], pkgToInstall: seq[(string, Version)]) =
+ for pkg in solvedPkgs:
+ if pkg.pkgName notin pkgToInstall.mapIt(it[0]):
+ for req in pkg.requirements:
+ displayInfo(pkgDepsAlreadySatisfiedMsg(req))
+
+proc addReverseDeps(solvedPkgs: seq[SolvedPackage], allPkgsInfo: seq[PackageInfo], options: Options) = 
+ for pkg in solvedPkgs:
+ let solvedPkg = getPackageInfo(pkg.pkgName, allPkgsInfo)
+ if solvedPkg.isNone: continue
+ for reverseDepName in pkg.reverseDependencies:
+ var reverseDep = getPackageInfo(reverseDepName, allPkgsInfo)
+ if reverseDep.isNone: continue
+
+ if reverseDep.get.myPath.parentDir.developFileExists:
+ reverseDep.get.isLink = true
+ addRevDep(options.nimbleData, solvedPkg.get.basicInfo, reverseDep.get)
+
 proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo], options: Options): HashSet[PackageInfo] = 
- result = solveLocalPackages(rootPkgInfo, pkgList)
- if result.len > 0: return result
- var reverseDependencies: seq[PackageBasicInfo] = @[]
+ var solvedPkgs = newSeq[SolvedPackage]()
  var pkgsToInstall: seq[(string, Version)] = @[]
- var output = ""
- var solved = false #A pgk can be solved and still dont return a set of PackageInfo
- (solved, result) = solvePackages(rootPkgInfo, pkgList, pkgsToInstall, options, output)
- if pkgsToInstall.len > 0:
- for pkg in pkgsToInstall:
- let dep = (name: pkg[0], ver: pkg[1].toVersionRange)
- let resolvedDep = dep.resolveAlias(options)
- display("Installing", $resolvedDep, priority = HighPriority)
- let toInstall = @[(resolvedDep.name, resolvedDep.ver)]
- #TODO install here will download the package again. We could use the already downloaded package 
- #from the cache
- let (packages, _) = install(toInstall, options,
- doPrompt = false, first = false, fromLockFile = false, preferredPackages = @[])
+ var allPkgsInfo: seq[PackageInfo] = pkgList & rootPkgInfo
 
- for pkg in packages:
- if result.contains pkg:
- # 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
-
- if not pkg.isLink:
- reverseDependencies.add(pkg.basicInfo)
- if result.len > 0: 
- # We add the reverse deps to the JSON file here because we don't want
- # them added if the above errorenous condition occurs
- # (unsatisfiable dependendencies).
- # N.B. NimbleData is saved in installFromDir.
- for i in reverseDependencies:
- addRevDep(options.nimbleData, i, rootPkgInfo)
+ result = solveLocalPackages(rootPkgInfo, pkgList, solvedPkgs)
+ if solvedPkgs.len > 0: 
+ displaySatisfiedMsg(solvedPkgs, pkgsToInstall)
+ addReverseDeps(solvedPkgs, allPkgsInfo, options)
  return result
- else:
- if not solved:
- display("Error", output, Error, priority = HighPriority)
- raise nimbleError("Unsatisfiable dependencies")
-
 
+ 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,
+ doPrompt = false, first = false, fromLockFile = false, preferredPackages = @[])
+ for pkg in packages:
+ if result.contains pkg:
+ # 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)
+
+ if not solved:
+ display("Error", output, Error, priority = HighPriority)
+ raise nimbleError("Unsatisfiable dependencies")
+
 proc processFreeDependencies(pkgInfo: PackageInfo,
  requirements: seq[PkgTuple],
  options: Options,
@@ -463,7 +477,7 @@ proc installFromDir(dir: string, requestedVer: VersionRange, options: Options,
  priority = HighPriority)
 
  let oldPkg = pkgInfo.packageExists(options)
- if oldPkg.isSome:
+ if oldPkg.isSome and not options.useSatSolver:
  # 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))
@@ -764,9 +778,12 @@ proc install(packages: seq[PkgTuple], options: Options,
  for pv in packages:
  let (meth, url, metadata) = getDownloadInfo(pv, options, doPrompt)
  let subdir = metadata.getOrDefault("subdir")
+ var downloadPath = ""
+ if options.useSatSolver:
+ downloadPath = getCacheDownloadDir(url, pv.ver, options)
  let (downloadDir, downloadVersion, vcsRevision) =
  downloadPkg(url, pv.ver, meth, subdir, options,
- downloadPath = "", vcsRevision = notSetSha1Hash)
+ downloadPath = downloadPath, vcsRevision = notSetSha1Hash)
  try:
  var opt = options
  if pv.name.isNim:

src/nimblepkg/download.nim

@@ -463,18 +463,10 @@ proc downloadPkg*(url: string, verRange: VersionRange,
  raise nimbleError("Cannot download in offline mode.")
  let downloadDir =
  if downloadPath == "":
- let dir = if options.pkgCachePath != "":
- options.pkgCachePath
- else:
- getNimbleTempDir()
- (dir / getDownloadDirName(url, verRange, vcsRevision))
+ (getNimbleTempDir() / getDownloadDirName(url, verRange, vcsRevision))
  else:
  downloadPath
-
- if options.pkgCachePath != "" and dirExists(downloadDir):
- #TODO test integrity of the package
- return (dir: downloadDir, version: newVersion getSimpleString(verRange), vcsRevision: notSetSha1Hash) 
-
+
  createDir(downloadDir)
  var modUrl =
  if url.startsWith("git:https://") and options.config.cloneUsingHttps:

src/nimblepkg/nimblesat.nim

@@ -3,9 +3,9 @@ when defined(nimNimbleBootstrap):
 else:
  import sat/[sat, satvars] 
 import version, packageinfotypes, download, packageinfo, packageparser, options, 
- sha1hashes
+ sha1hashes, tools
 
-import std/[tables, sequtils, algorithm, sets, strutils, options, strformat]
+import std/[tables, sequtils, algorithm, sets, strutils, options, strformat, os]
 
 
 type 
@@ -54,12 +54,21 @@ type
  reqs*: seq[Requirements]
  packageToDependency*: Table[string, int] #package.name -> index into nodes
  # reqsByDeps: Table[Requirements, int]
+ SolvedPackage* = object
+ pkgName*: string
+ version*: Version
+ requirements*: seq[PkgTuple] 
+ reverseDependencies*: seq[string] 
+
  GetPackageMinimal* = proc (pv: PkgTuple, options: Options): Option[PackageMinimalInfo]
 
+proc isNim*(pv: PkgTuple): bool =
+ pv.name == "nim" or pv.name == "nimrod"
+
 proc getMinimalInfo*(pkg: PackageInfo): PackageMinimalInfo =
  result.name = pkg.basicInfo.name
  result.version = pkg.basicInfo.version
- result.requires = pkg.requires
+ result.requires = pkg.requires.filterIt(not it.isNim())
 
 proc hasVersion*(packageVersions: PackageVersions, pv: PkgTuple): bool =
  for pkg in packageVersions.versions:
@@ -101,7 +110,7 @@ proc findDependencyForDep(g: DepGraph; dep: string): int {.inline.} =
  result = g.packageToDependency.getOrDefault(dep)
 
 proc createRequirements(pkg: PackageMinimalInfo): Requirements =
- result.deps = pkg.requires.filterIt(it.name != "nim")
+ result.deps = pkg.requires.filterIt(not it.isNim())
  result.version = pkg.version
  result.nimVersion = pkg.requires.getNimVersion()
 
@@ -280,28 +289,50 @@ proc solve*(g: var DepGraph; f: Form, packages: var Table[string, Version], outp
  output = generateUnsatisfiableMessage(g, f, s)
  false
 
-proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output: var string): Table[string, Version] =
+proc collectReverseDependencies*(targetPkgName: string, graph: DepGraph): seq[string] =
+ var reverseDeps: HashSet[string] = initHashSet[string]()
+ for node in graph.nodes:
+ for version in node.versions:
+ for (depName, _) in graph.reqs[version.req].deps:
+ if depName == targetPkgName:
+ reverseDeps.incl(node.pkgName) #
+ reverseDeps.toSeq()
+
+proc getSolvedPackages*(pkgVersionTable: Table[string, PackageVersions], output: var string): seq[SolvedPackage] =
  var graph = pkgVersionTable.toDepGraph()
  #Make sure all references are in the graph before calling toFormular
  for p in graph.nodes:
  for ver in p.versions.items:
  for dep, q in items graph.reqs[ver.req].deps:
  if dep notin graph.packageToDependency:
  output.add &"Dependency {dep} not found in the graph \n"
- return initTable[string, Version]()
+ return newSeq[SolvedPackage]()
 
  let form = toFormular(graph)
  var packages = initTable[string, Version]()
  discard solve(graph, form, packages, output)
- packages
+
+ for pkg, ver in packages:
+ let nodeIdx = graph.packageToDependency[pkg]
+ for dep in graph.nodes[nodeIdx].versions:
+ if dep.version == ver:
+ let reqIdx = dep.req
+ let deps = graph.reqs[reqIdx].deps
+ let solvedPkg = SolvedPackage(pkgName: pkg, version: ver, 
+ requirements: deps, reverseDependencies: collectReverseDependencies(pkg, graph))
+ result.add solvedPkg
+
+proc getCacheDownloadDir*(url: string, ver: VersionRange, options: Options): string =
+ options.pkgCachePath / getDownloadDirName(url, ver, notSetSha1Hash)
 
 proc downloadPkInfoForPv*(pv: PkgTuple, options: Options): PackageInfo =
  let (meth, url, metadata) = 
- getDownloadInfo(pv, options, doPrompt = true)
+ 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,
- "", vcsRevision = notSetSha1Hash)
+ downloadDir, vcsRevision = notSetSha1Hash)
  return getPkgInfo(res.dir, options)
 
 proc downloadMinimalPackage*(pv: PkgTuple, options: Options): Option[PackageMinimalInfo] =
@@ -321,14 +352,38 @@ proc fillPackageTableFromPreferred*(packages: var Table[string, PackageVersions]
 proc getInstalledMinimalPackages*(options: Options): seq[PackageMinimalInfo] =
  getInstalledPkgsMin(options.getPkgsDir(), options).mapIt(it.getMinimalInfo())
 
-proc collectAllVersions*(versions: var Table[string, PackageVersions], package: PackageMinimalInfo, options: Options, getMinimalPackage: GetPackageMinimal) =
+#From the STD as it is not available in older Nim versions
+func addUnique*[T](s: var seq[T], x: sink T) =
+ ## Adds `x` to the container `s` if it is not already present. 
+ ## Uses `==` to check if the item is already present.
+ runnableExamples:
+ var a = @[1, 2, 3]
+ a.addUnique(4)
+ a.addUnique(4)
+ assert a == @[1, 2, 3, 4]
+
+ for i in 0..high(s):
+ if s[i] == x: return
+ when declared(ensureMove):
+ s.add ensureMove(x)
+ else:
+ s.add x
+
+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 getMinimalFromPreferred(pv: PkgTuple): Option[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 some pp
+ 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
- var pkgMin = getMinimalPackage(pv, options).get() #TODO elegantly fail here
+ var pkgMin = getMinimalFromPreferred(pv).get()
  if pv.ver.kind == verSpecial:
  pkgMin.version = newVersion $pv.ver
  if not versions.hasKey(pv.name):
@@ -337,32 +392,37 @@ proc collectAllVersions*(versions: var Table[string, PackageVersions], package:
  versions[pv.name].versions.addUnique pkgMin
  collectAllVersions(versions, pkgMin, options, getMinimalPackage)
 
-proc solveLocalPackages*(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo]): HashSet[PackageInfo] = 
+proc solveLocalPackages*(rootPkgInfo: PackageInfo, pkgList: seq[PackageInfo], solvedPkgs: var seq[SolvedPackage]): HashSet[PackageInfo] = 
  var root = rootPkgInfo.getMinimalInfo()
  root.isRoot = true
  var pkgVersionTable = initTable[string, PackageVersions]()
  pkgVersionTable[root.name] = PackageVersions(pkgName: root.name, versions: @[root])
  fillPackageTableFromPreferred(pkgVersionTable, pkgList.map(getMinimalInfo))
  var output = ""
- var solvedPkgs = pkgVersionTable.getSolvedPackages(output)
- for pkg, ver in solvedPkgs:
+ solvedPkgs = pkgVersionTable.getSolvedPackages(output)
+ for solvedPkg in solvedPkgs:
  for pkgInfo in pkgList:
- if pkgInfo.basicInfo.name == pkg and pkgInfo.basicInfo.version == ver:
+ if pkgInfo.basicInfo.name == solvedPkg.pkgName and pkgInfo.basicInfo.version == solvedPkg.version:
  result.incl pkgInfo
 
-proc solvePackages*(rootPkg: PackageInfo, pkgList: seq[PackageInfo], pkgsToInstall: var seq[(string, Version)], options: Options, output: var string): (bool, HashSet[PackageInfo]) =
- var root = rootPkg.getMinimalInfo()
+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)
- var solvedPkgs = pkgVersionTable.getSolvedPackages(output)
- result[0] = solvedPkgs.len > 0
- var pkgsToInstall: seq[(string, Version)] = @[]
- for solvedPkg, ver in solvedPkgs:
- if solvedPkg == root.name: continue
+ collectAllVersions(pkgVersionTable, root, options, downloadMinimalPackage, pkgList.map(getMinimalInfo))
+ solvedPkgs = pkgVersionTable.getSolvedPackages(output)
+ for solvedPkg in solvedPkgs:
+ if solvedPkg.pkgName == root.name: continue
+ var foundInList = false
  for pkgInfo in pkgList:
- if pkgInfo.basicInfo.name == solvedPkg: # and pkgInfo.basicInfo.version.withinRange(ver):
- result[1].incl pkgInfo
- else:
- pkgsToInstall.addUnique((solvedPkg, ver))
+ if pkgInfo.basicInfo.name == solvedPkg.pkgName and pkgInfo.basicInfo.version == solvedPkg.version:
+ result.incl pkgInfo
+ foundInList = true
+ if not foundInList:
+ pkgsToInstall.addUnique((solvedPkg.pkgName, solvedPkg.version))
+
+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 

src/nimblepkg/packageinfo.nim

@@ -180,7 +180,10 @@ proc fetchList*(list: PackageList, options: Options) =
  display("Success", "Package list copied.", Success, HighPriority)
 
  if lastError.len != 0:
- raise nimbleError("Refresh failed\n" & lastError)
+ if list.name == "local":
+ display("Warning:", lastError & ", discarding.", Warning)
+ else:
+ raise nimbleError("Refresh failed\n" & lastError)
 
  if copyFromPath.len > 0:
  copyFile(copyFromPath,

tests/issue289/issue289.nimble

@@ -10,5 +10,4 @@ bin = @["issue289"]
 # Dependencies
 
 requires "nim >= 0.15.0", "https://github.com/nimble-test/packagea.git 0.6.0"
-requires "https://github.com/nimble-test/packagea.git#head"
 

tests/tissues.nim

@@ -135,6 +135,7 @@ suite "issues":
  test "issues #308 and #515":
  let
  ext = when defined(Windows): ExeExt else: "out"
+ cleanDir(installDir)
  cd "issue308515" / "v1":
  var (output, exitCode) = execNimble(["run", "binname", "--silent"])
  check exitCode == QuitSuccess

tests/tsat.nim

@@ -231,8 +231,10 @@ suite "SAT solver":
  collectAllVersions(pkgVersionTable, root, options, downloadMinimalPackage)
  var output = ""
  let solvedPkgs = pkgVersionTable.getSolvedPackages(output)
- check solvedPkgs["b"] == newVersion "0.1.4"
- check solvedPkgs["c"] == newVersion "0.1.0"
+ let pkgB = solvedPkgs.filterIt(it.pkgName == "b")[0]
+ let pkgC = solvedPkgs.filterIt(it.pkgName == "c")[0]
+ check pkgB.pkgName == "b" and pkgB.version == newVersion "0.1.4"
+ check pkgC.pkgName == "c" and pkgC.version == newVersion "0.1.0"
  check "random" in pkgVersionTable
 
  removeDir(options.pkgCachePath)