Preserve special versions in package installations. Ref #88.

src/nimble.nim

@@ -263,7 +263,7 @@ proc processDeps(pkginfo: PackageInfo, options: Options): seq[string] =
   result = @[]
   assert(not pkginfo.isMinimal, "processDeps needs pkginfo.requires")
   display("Verifying",
-          "dependencies for $1 v$2" % [pkginfo.name, pkginfo.version],
+          "dependencies for $1@$2" % [pkginfo.name, pkginfo.version],
           priority = HighPriority)
 
   let pkglist = getInstalledPkgs(options.getPkgsDir(), options)
@@ -275,7 +275,7 @@ proc processDeps(pkginfo: PackageInfo, options: Options): seq[string] =
         let msg = "Unsatisfied dependency: " & dep.name & " (" & $dep.ver & ")"
         raise newException(NimbleError, msg)
     else:
-      let depDesc = "$1 ($2)" % [dep.name, $dep.ver]
+      let depDesc = "$1@$2" % [dep.name, $dep.ver]
       display("Checking", "for $1" % depDesc, priority = MediumPriority)
       var pkg: PackageInfo
       if not findPkg(pkglist, dep, pkg):
@@ -390,7 +390,7 @@ proc vcsRevisionInDir(dir: string): string =
     except:
       discard
 
-proc installFromDir(dir: string, latest: bool, options: Options,
+proc installFromDir(dir: string, requestedVer: VersionRange, options: Options,
                     url: string): tuple[paths: seq[string], pkg: PackageInfo] =
   ## Returns where package has been installed to, together with paths
   ## to the packages this package depends on.
@@ -400,24 +400,29 @@ proc installFromDir(dir: string, latest: bool, options: Options,
   let realDir = pkgInfo.getRealDir()
   let binDir = options.getBinDir()
   let pkgsDir = options.getPkgsDir()
-  var deps_options = options
+  var depsOptions = options
-  deps_options.depsOnly = false
+  depsOptions.depsOnly = false
+
+  # Overwrite the version if the requested version is "#head" or similar.
+  if requestedVer.kind == verSpecial:
+    pkgInfo.version = $requestedVer.spe
 
   # Dependencies need to be processed before the creation of the pkg dir.
-  result.paths = processDeps(pkginfo, deps_options)
+  result.paths = processDeps(pkginfo, depsOptions)
 
   if options.depsOnly:
     result.pkg = pkgInfo
     return result
 
-  display("Installing", "$1 v$2" % [pkginfo.name, pkginfo.version],
+  display("Installing", "$1 $2" % [pkginfo.name, pkginfo.version],
           priority = HighPriority)
 
   # Build before removing an existing package (if one exists). This way
   # if the build fails then the old package will still be installed.
   if pkgInfo.bin.len > 0: buildFromDir(pkgInfo, result.paths, true)
 
-  let versionStr = (if latest: "" else: '-' & pkgInfo.version)
+  let versionStr = '-' & pkgInfo.version
+
   let pkgDestDir = pkgsDir / (pkgInfo.name & versionStr)
   if existsDir(pkgDestDir) and existsFile(pkgDestDir / "nimblemeta.json"):
     if not options.prompt(pkgInfo.name & versionStr &
@@ -527,7 +532,7 @@ proc getNimbleTempDir(): string =
 
 proc downloadPkg(url: string, verRange: VersionRange,
                  downMethod: DownloadMethod,
-                 options: Options): (string, VersionRange) =
+                 options: Options): (string, Version) =
   ## Downloads the repository as specified by ``url`` and ``verRange`` using
   ## the download method specified.
   ##
@@ -580,7 +585,7 @@ proc install(packages: seq[PkgTuple],
              options: Options,
              doPrompt = true): tuple[paths: seq[string], pkg: PackageInfo] =
   if packages == @[]:
-    result = installFromDir(getCurrentDir(), false, options, "")
+    result = installFromDir(getCurrentDir(), newVRAny(), options, "")
   else:
     # If packages.json is not present ask the user if they want to download it.
     if needsRefresh(options):
@@ -597,11 +602,11 @@ proc install(packages: seq[PkgTuple],
       let (downloadDir, downloadVersion) =
           downloadPkg(url, pv.ver, meth, options)
       try:
-        result = installFromDir(downloadDir, false, options, url)
+        result = installFromDir(downloadDir, pv.ver, options, url)
       except BuildFailed:
         # The package failed to build.
         # Check if we tried building a tagged version of the package.
-        let headVer = parseVersionRange("#" & getHeadName(meth))
+        let headVer = getHeadName(meth)
         if pv.ver.kind != verSpecial and downloadVersion != headVer:
           # If we tried building a tagged version of the package then
           # ask the user whether they want to try building #head.
@@ -610,8 +615,8 @@ proc install(packages: seq[PkgTuple],
                   " like to try installing '$1@#head' (latest unstable)?") %
                   [pv.name, $downloadVersion])
           if promptResult:
-
+            let toInstall = @[(pv.name, headVer.toVersionRange())]
-            result = install(@[(pv.name, headVer)], options, doPrompt)
+            result = install(toInstall, options, doPrompt)
           else:
             raise newException(BuildFailed,
               "Aborting installation due to build failure")

src/nimblepkg/download.nim

@@ -116,12 +116,12 @@ proc getDownloadMethod*(meth: string): DownloadMethod =
   else:
     raise newException(NimbleError, "Invalid download method: " & meth)
 
-proc getHeadName*(meth: DownloadMethod): string =
+proc getHeadName*(meth: DownloadMethod): Version =
   ## Returns the name of the download method specific head. i.e. for git
   ## it's ``head`` for hg it's ``tip``.
   case meth
-  of DownloadMethod.git: "head"
+  of DownloadMethod.git: newVersion("#head")
-  of DownloadMethod.hg: "tip"
+  of DownloadMethod.hg: newVersion("#tip")
 
 proc checkUrlType*(url: string): DownloadMethod =
   ## Determines the download method based on the URL.
@@ -136,7 +136,7 @@ proc isURL*(name: string): bool =
   name.startsWith(peg" @'://' ")
 
 proc doDownload*(url: string, downloadDir: string, verRange: VersionRange,
-                 downMethod: DownloadMethod, options: Options): VersionRange =
+                 downMethod: DownloadMethod, options: Options): Version =
   ## Downloads the repository specified by ``url`` using the specified download
   ## method.
   ##
@@ -152,7 +152,7 @@ proc doDownload*(url: string, downloadDir: string, verRange: VersionRange,
     # https://github.com/nim-lang/nimble/issues/22
     meth
     if $latest.ver != "":
-      result = parseVersionRange($latest.ver)
+      result = latest.ver
     else:
       # Result should already be set to #head here.
       assert(not result.isNil)
@@ -170,20 +170,20 @@ proc doDownload*(url: string, downloadDir: string, verRange: VersionRange,
   removeDir(downloadDir)
   if verRange.kind == verSpecial:
     # We want a specific commit/branch/tag here.
-    if verRange.spe == newSpecial(getHeadName(downMethod)):
+    if verRange.spe == getHeadName(downMethod):
       doClone(downMethod, url, downloadDir) # Grab HEAD.
     else:
       # Grab the full repo.
       doClone(downMethod, url, downloadDir, tip = false)
       # Then perform a checkout operation to get the specified branch/commit.
       doCheckout(downMethod, downloadDir, $verRange.spe)
-    result = verRange
+    result = verRange.spe
   else:
     case downMethod
     of DownloadMethod.git:
       # For Git we have to query the repo remotely for its tags. This is
       # necessary as cloning with a --depth of 1 removes all tag info.
-      result = parseVersionRange("#head")
+      result = getHeadName(downMethod)
       let versions = getTagsListRemote(url, downMethod).getVersionList()
       if versions.len > 0:
         getLatestByTag:
@@ -197,7 +197,7 @@ proc doDownload*(url: string, downloadDir: string, verRange: VersionRange,
       verifyClone()
     of DownloadMethod.hg:
       doClone(downMethod, url, downloadDir)
-      result = parseVersionRange("#tip")
+      result = getHeadName(downMethod)
       let versions = getTagsList(downloadDir, downMethod).getVersionList()
 
       if versions.len > 0:

src/nimblepkg/packageinfo.nim

@@ -4,7 +4,7 @@ import parsecfg, json, streams, strutils, parseutils, os, sets, tables
 import version, tools, common, options, cli
 
 type
-  Package* = object
+  Package* = object ## Definition of package from packages.json.
     # Required fields in a package.
     name*: string
     url*: string # Download location.
@@ -57,7 +57,6 @@ proc getNameVersion*(pkgpath: string): tuple[name, version: string] =
   ##
   ## Also works for file paths like:
   ##   ``/home/user/.nimble/pkgs/package-0.1/package.nimble``
-
   if pkgPath.splitFile.ext == ".nimble" or pkgPath.splitFile.ext == ".babel":
     return getNameVersion(pkgPath.splitPath.head)
 
@@ -288,14 +287,8 @@ when isMainModule:
       ("package-a", "0.1")
   doAssert getNameVersion("/home/user/.nimble/libs/package-a-0.1/package.nimble") ==
       ("package-a", "0.1")
-
+  doAssert getNameVersion("/home/user/.nimble/libs/package-#head") ==
-  validatePackageName("foo_bar")
+      ("package", "#head")
-  validatePackageName("f_oo_b_a_r")
-  try:
-    validatePackageName("foo__bar")
-    assert false
-  except NimbleError:
-    assert true
 
   doAssert toValidPackageName("foo__bar") == "foo_bar"
   doAssert toValidPackageName("jhbasdh!£$@%#^_&*_()qwe") == "jhbasdh_qwe"

src/nimblepkg/packageparser.nim

@@ -193,6 +193,7 @@ proc readPackageInfo*(nf: NimbleFile, options: Options,
     return options.pkgInfoCache[nf]
 
   result = initPackageInfo(nf)
+  let minimalInfo = getNameVersion(nf)
 
   validatePackageName(nf.splitFile.name)
 
@@ -208,9 +209,8 @@ proc readPackageInfo*(nf: NimbleFile, options: Options,
 
   if not success:
     if onlyMinimalInfo:
-      let tmp = getNameVersion(nf)
+      result.name = minimalInfo.name
-      result.name = tmp.name
+      result.version = minimalInfo.version
-      result.version = tmp.version
       result.isNimScript = true
       result.isMinimal = true
     else:
@@ -226,6 +226,14 @@ proc readPackageInfo*(nf: NimbleFile, options: Options,
         raise newException(NimbleError, msg)
 
   validatePackageInfo(result, nf)
+
+  # The package directory name may include a "special" version
+  # (example #head). If so, it is given higher priority and therefore
+  # overwrites the .nimble file's version.
+  let version = parseVersionRange(minimalInfo.version)
+  if version.kind == verSpecial:
+    result.version = minimalInfo.version
+
   if not result.isMinimal:
     options.pkgInfoCache[nf] = result
 
@@ -240,7 +248,7 @@ proc getInstalledPkgs*(libsDir: string, options: Options):
   ##
   ## ``libsDir`` is in most cases: ~/.nimble/pkgs/
   const
-    readErrorMsg = "Installed package $1 v$2 is outdated or corrupt."
+    readErrorMsg = "Installed package '$1@$2' is outdated or corrupt."
     validationErrorMsg = readErrorMsg & "\nPackage did not pass validation: $3"
     hintMsg = "The corrupted package will need to be removed manually. To fix" &
               " this error message, remove $1."
@@ -278,3 +286,14 @@ proc getInstalledPkgs*(libsDir: string, options: Options):
 
 proc isNimScript*(nf: string, options: Options): bool =
   result = readPackageInfo(nf, options).isNimScript
+
+when isMainModule:
+  validatePackageName("foo_bar")
+  validatePackageName("f_oo_b_a_r")
+  try:
+    validatePackageName("foo__bar")
+    assert false
+  except NimbleError:
+    assert true
+
+  echo("Everything passed!")

src/nimblepkg/version.nim

@@ -5,7 +5,6 @@
 import strutils, tables, hashes, parseutils
 type
   Version* = distinct string
-  Special* = distinct string
 
   VersionRangeEnum* = enum
     verLater, # > V
@@ -23,7 +22,7 @@ type
     of verLater, verEarlier, verEqLater, verEqEarlier, verEq:
       ver*: Version
     of verSpecial:
-      spe*: Special
+      spe*: Version
     of verIntersect:
       verILeft, verIRight: VersionRange
     of verAny:
@@ -36,18 +35,25 @@ type
   NimbleError* = object of Exception
     hint*: string
 
-proc newVersion*(ver: string): Version = return Version(ver)
+proc newVersion*(ver: string): Version =
-proc newSpecial*(spe: string): Special = return Special(spe)
+  doAssert(ver[0] in {'#', '\0'} + Digits)
+  return Version(ver)
 
 proc `$`*(ver: Version): string {.borrow.}
 
 proc hash*(ver: Version): Hash {.borrow.}
 
-proc `$`*(ver: Special): string {.borrow.}
+proc isNil*(ver: Version): bool {.borrow.}
 
-proc hash*(ver: Special): Hash {.borrow.}
+proc isSpecial*(ver: Version): bool =
+  return ($ver)[0] == '#'
 
 proc `<`*(ver: Version, ver2: Version): bool =
+  # Handling for special versions such as "#head" or "#branch".
+  if ver.isSpecial or ver2.isSpecial:
+    return false
+
+  # Handling for normal versions such as "0.1.0" or "1.0".
   var sVer = string(ver).split('.')
   var sVer2 = string(ver2).split('.')
   for i in 0..max(sVer.len, sVer2.len)-1:
@@ -65,6 +71,9 @@ proc `<`*(ver: Version, ver2: Version): bool =
       return false
 
 proc `==`*(ver: Version, ver2: Version): bool =
+  if ver.isSpecial or ver2.isSpecial:
+    return ($ver).toLowerAscii() == ($ver2).toLowerAscii()
+
   var sVer = string(ver).split('.')
   var sVer2 = string(ver2).split('.')
   for i in 0..max(sVer.len, sVer2.len)-1:
@@ -79,9 +88,6 @@ proc `==`*(ver: Version, ver2: Version): bool =
     else:
       return false
 
-proc `==`*(spe: Special, spe2: Special): bool =
-  return ($spe).toLowerAscii() == ($spe2).toLowerAscii()
-
 proc `<=`*(ver: Version, ver2: Version): bool =
   return (ver == ver2) or (ver < ver2)
 
@@ -97,39 +103,36 @@ proc `==`*(range1: VersionRange, range2: VersionRange): bool =
   of verAny: true
 
 proc withinRange*(ver: Version, ran: VersionRange): bool =
-  case ran.kind
+  if ver.isSpecial:
-  of verLater:
+    case ran.kind
-    return ver > ran.ver
+    of verLater, verEarlier, verEqLater, verEqEarlier, verEq, verIntersect:
-  of verEarlier:
+      return false
-    return ver < ran.ver
+    of verSpecial:
-  of verEqLater:
+      return ver == ran.spe
-    return ver >= ran.ver
+    of verAny:
-  of verEqEarlier:
+      return true
-    return ver <= ran.ver
+  else:
-  of verEq:
+    case ran.kind
-    return ver == ran.ver
+    of verLater:
-  of verSpecial:
+      return ver > ran.ver
-    return false
+    of verEarlier:
-  of verIntersect:
+      return ver < ran.ver
-    return withinRange(ver, ran.verILeft) and withinRange(ver, ran.verIRight)
+    of verEqLater:
-  of verAny:
+      return ver >= ran.ver
-    return true
+    of verEqEarlier:
-
+      return ver <= ran.ver
-proc withinRange*(spe: Special, ran: VersionRange): bool =
+    of verEq:
-  case ran.kind
+      return ver == ran.ver
-  of verLater, verEarlier, verEqLater, verEqEarlier, verEq, verIntersect:
+    of verSpecial:
-    return false
+      return false
-  of verSpecial:
+    of verIntersect:
-    return spe == ran.spe
+      return withinRange(ver, ran.verILeft) and withinRange(ver, ran.verIRight)
-  of verAny:
+    of verAny:
-    return true
+      return true
 
 proc contains*(ran: VersionRange, ver: Version): bool =
   return withinRange(ver, ran)
 
-proc contains*(ran: VersionRange, spe: Special): bool =
-  return withinRange(spe, ran)
-
 proc makeRange*(version: string, op: string): VersionRange =
   new(result)
   if version == "":
@@ -153,9 +156,13 @@ proc makeRange*(version: string, op: string): VersionRange =
 proc parseVersionRange*(s: string): VersionRange =
   # >= 1.5 & <= 1.8
   new(result)
+  if s.len == 0:
+    result.kind = verAny
+    return
+
   if s[0] == '#':
     result.kind = verSpecial
-    result.spe = s[1 .. s.len-1].Special
+    result.spe = s.Version
     return
 
   var i = 0
@@ -200,6 +207,15 @@ proc parseVersionRange*(s: string): VersionRange =
           "Unexpected char in version range: " & s[i])
     inc(i)
 
+proc toVersionRange*(ver: Version): VersionRange =
+  ## Converts a version to either a verEq or verSpecial VersionRange.
+  new(result)
+  if ver.isSpecial:
+    result.kind = verSpecial
+    result.spe = ver
+  else:
+    result.kind = verEq
+    result.ver = ver
 
 proc parseRequires*(req: string): PkgTuple =
   try:
@@ -231,7 +247,7 @@ proc `$`*(verRange: VersionRange): string =
   of verEq:
     result = ""
   of verSpecial:
-    return "#" & $verRange.spe
+    return $verRange.spe
   of verIntersect:
     return $verRange.verILeft & " & " & $verRange.verIRight
   of verAny:
@@ -312,13 +328,26 @@ when isMainModule:
   #doAssert newVersion("0.1-rc1") < newVersion("0.1")
 
   # Special tests
-  doAssert newSpecial("ab26sgdt362") != newSpecial("ab26saggdt362")
+  doAssert newVersion("#ab26sgdt362") != newVersion("#qwersaggdt362")
-  doAssert newSpecial("ab26saggdt362") == newSpecial("ab26saggdt362")
+  doAssert newVersion("#ab26saggdt362") == newVersion("#ab26saggdt362")
-  doAssert newSpecial("head") == newSpecial("HEAD")
+  doAssert newVersion("#head") == newVersion("#HEAD")
-  doAssert newSpecial("head") == newSpecial("head")
+  doAssert newVersion("#head") == newVersion("#head")
 
   var sp = parseVersionRange("#ab26sgdt362")
-  doAssert newSpecial("ab26sgdt362") in sp
+  doAssert newVersion("#ab26sgdt362") in sp
-  doAssert newSpecial("ab26saggdt362") notin sp
+  doAssert newVersion("#ab26saggdt362") notin sp
+
+  doAssert newVersion("#head") in parseVersionRange("#head")
+
+  # TODO: It may be worth changing this in the future, although we can't be
+  # certain that #head is in fact newer than v0.1.0.
+  doAssert(not(newVersion("#head") > newVersion("0.1.0")))
+
+  # An empty version range should give verAny
+  doAssert parseVersionRange("").kind == verAny
+
+  # toVersionRange tests
+  doAssert toVersionRange(newVersion("#head")).kind == verSpecial
+  doAssert toVersionRange(newVersion("0.2.0")).kind == verEq
 
   echo("Everything works!")

tests/tester.nim

@@ -2,6 +2,10 @@
 # BSD License. Look at license.txt for more info.
 import osproc, streams, unittest, strutils, os, sequtils, future
 
+# TODO: Each test should start off with a clean slate. Currently installed
+# packages are shared between each test which causes a multitude of issues
+# and is really fragile.
+
 var rootDir = getCurrentDir().parentDir()
 var nimblePath = rootDir / "src" / "nimble" & ExeExt
 var installDir = rootDir / "tests" / "nimbleDir"
@@ -41,6 +45,11 @@ test "issue 129 (installing commit hash)":
   let arguments = @["install", "-y",
                    "https://github.com/nimble-test/packagea.git@#1f9cb289c89"]
   check execNimble(arguments).exitCode == QuitSuccess
+  # Verify that it was installed correctly.
+  check dirExists(installDir / "pkgs" / "packagea-#1f9cb289c89")
+  # Remove it so that it doesn't interfere with the uninstall tests.
+  check execNimble("uninstall", "-y", "packagea@#1f9cb289c89").exitCode ==
+        QuitSuccess
 
 test "issue 113 (uninstallation problems)":
   cd "issue113/c":
@@ -247,6 +256,6 @@ test "can uninstall":
 
   check execNimble("uninstall", "-y", "PackageA@0.2", "issue27b").exitCode ==
       QuitSuccess
-  check(not dirExists(getHomeDir() / ".nimble" / "pkgs" / "PackageA-0.2.0"))
+  check(not dirExists(installDir / "pkgs" / "PackageA-0.2.0"))
 
   check execNimble("uninstall", "-y", "nimscript").exitCode == QuitSuccess