From d9a6246c666c6b0fe6078adc194d3f101e9765e9 Mon Sep 17 00:00:00 2001
From: Ivan Bobev <bobeff@gmail.com>
Date: Wed, 21 Aug 2019 14:45:22 +0300
Subject: [PATCH] Add changes required by Nimble lock file support

Implemented support for Nimble local cache with package directories with
a checksum of the package at the end of their names. Now the compiler
supports package paths in the form:

 * /path_to_nimble_cache_dir/pkgs/package_name-1.2.3-
FEBADEAEA2345E777F0F6F8433F7F0A52EDD5D1B

 * /path_to_nimble_cache_dir/pkgs/package_name-#head-
042D4BE2B90ED0672E717D71850ABDB0A2D19CD2

 * /path_to_nimble_cache_dir/pkgs/package_name-#branch-name-
DBC1F902CB79946E990E38AF51F0BAD36ACFABD9

Related to nim-lang/nimble#127
---
 compiler/modulepaths.nim      |  2 +-
 compiler/nimblecmd.nim        | 82 ++++++++++++++++++-----------
 lib/std/sha1.nim              | 11 ++++
 tests/compiler/tnimblecmd.nim | 97 ++++++++++++++++++++++++++---------
 tests/stdlib/tsha1.nim        | 10 ++++
 5 files changed, 148 insertions(+), 54 deletions(-)

diff --git a/compiler/modulepaths.nim b/compiler/modulepaths.nim
index 8511b15921b5a..a16b669c45910 100644
--- a/compiler/modulepaths.nim
+++ b/compiler/modulepaths.nim
@@ -21,7 +21,7 @@ when false:
     for k, p in os.walkDir(dir, relative=true):
       if k == pcDir and p.len > pkg.len+1 and
           p[pkg.len] == '-' and p.startsWith(pkg):
-        let (_, a) = getPathVersion(p)
+        let (_, a, _) = getPathVersionChecksum(p)
         if bestv.len == 0 or bestv < a:
           bestv = a
           best = dir / p
diff --git a/compiler/nimblecmd.nim b/compiler/nimblecmd.nim
index 28398289ced85..18be8b3aa2ad4 100644
--- a/compiler/nimblecmd.nim
+++ b/compiler/nimblecmd.nim
@@ -9,8 +9,8 @@
 
 ## Implements some helper procs for Nimble (Nim's package manager) support.
 
-import parseutils, strutils, strtabs, os, options, msgs, sequtils,
-  lineinfos, pathutils
+import parseutils, strutils, os, options, msgs, sequtils, lineinfos, pathutils,
+  std/sha1, tables
 
 proc addPath*(conf: ConfigRef; path: AbsoluteDir, info: TLineInfo) =
   if not conf.searchPaths.contains(path):
@@ -18,6 +18,7 @@ proc addPath*(conf: ConfigRef; path: AbsoluteDir, info: TLineInfo) =
 
 type
   Version* = distinct string
+  PackageInfo = Table[string, tuple[version, checksum: string]]
 
 proc `$`*(ver: Version): string {.borrow.}
 
@@ -62,43 +63,64 @@ proc `<`*(ver: Version, ver2: Version): bool =
     else:
       return false
 
-proc getPathVersion*(p: string): tuple[name, version: string] =
-  ## Splits path ``p`` in the format ``/home/user/.nimble/pkgs/package-0.1``
-  ## into ``(/home/user/.nimble/pkgs/package, 0.1)``
-  result.name = ""
-  result.version = ""
-
-  const specialSeparator = "-#"
-  let last = p.rfind(p.lastPathPart) # the index where the last path part begins
-  var sepIdx = p.find(specialSeparator, start = last)
-  if sepIdx == -1:
-    sepIdx = p.rfind('-', start = last)
-
-  if sepIdx == -1:
-    result.name = p
-    return
+proc getPathVersionChecksum*(p: string): tuple[name, version, checksum: string] =
+  ## Splits path ``p`` in the format
+  ## ``/home/user/.nimble/pkgs/package-0.1-febadeaea2345e777f0f6f8433f7f0a52edd5d1b`` into
+  ## ``("/home/user/.nimble/pkgs/package", "0.1", "febadeaea2345e777f0f6f8433f7f0a52edd5d1b")``
+
+  const checksumSeparator = '-'
+  const versionSeparator = '-'
+  const specialVersionSeparator = "-#"
+  const separatorNotFound = -1
+
+  var checksumSeparatorIndex = p.rfind(checksumSeparator)
+  if checksumSeparatorIndex != separatorNotFound:
+    result.checksum = p.substr(checksumSeparatorIndex + 1)
+    if not result.checksum.isValidSha1Hash():
+      result.checksum = ""
+      checksumSeparatorIndex = p.len()
+  else:
+    checksumSeparatorIndex = p.len()
 
-  for i in sepIdx..<p.len:
-    if p[i] in {DirSep, AltSep}:
-      result.name = p
-      return
+  var versionSeparatorIndex = p.rfind(
+    specialVersionSeparator, 0, checksumSeparatorIndex - 1)
+  if versionSeparatorIndex != separatorNotFound:
+    result.version = p.substr(
+      versionSeparatorIndex + 1, checksumSeparatorIndex - 1)
+  else:
+    versionSeparatorIndex = p.rfind(
+      versionSeparator, 0, checksumSeparatorIndex - 1)
+    if versionSeparatorIndex != separatorNotFound:
+      result.version = p.substr(
+        versionSeparatorIndex + 1, checksumSeparatorIndex - 1)
+    else:
+      versionSeparatorIndex = checksumSeparatorIndex
 
-  result.name = p[0..sepIdx - 1]
-  result.version = p.substr(sepIdx + 1)
+  result.name = p[0..<versionSeparatorIndex]
 
-proc addPackage(conf: ConfigRef; packages: StringTableRef, p: string; info: TLineInfo) =
-  let (name, ver) = getPathVersion(p)
+proc addPackage(conf: ConfigRef; packages: var PackageInfo, p: string;
+                info: TLineInfo) =
+  let (name, ver, checksum) = getPathVersionChecksum(p)
   if isValidVersion(ver):
     let version = newVersion(ver)
-    if packages.getOrDefault(name).newVersion < version or
+    if packages.getOrDefault(name).version.newVersion < version or
       (not packages.hasKey(name)):
-      packages[name] = $version
+      if checksum.isValidSha1Hash():
+        packages[name] = ($version, checksum)
+      else:
+        packages[name] = ($version, "")
   else:
     localError(conf, info, "invalid package name: " & p)
 
-iterator chosen(packages: StringTableRef): string =
+iterator chosen(packages: PackageInfo): string =
   for key, val in pairs(packages):
-    let res = if val.len == 0: key else: key & '-' & val
+    var res = key
+    if val.version.len != 0:
+      res &= '-'
+      res &= val.version
+    if val.checksum.len != 0:
+      res &= '-'
+      res &= val.checksum
     yield res
 
 proc addNimblePath(conf: ConfigRef; p: string, info: TLineInfo) =
@@ -118,7 +140,7 @@ proc addNimblePath(conf: ConfigRef; p: string, info: TLineInfo) =
     conf.lazyPaths.insert(AbsoluteDir path, 0)
 
 proc addPathRec(conf: ConfigRef; dir: string, info: TLineInfo) =
-  var packages = newStringTable(modeStyleInsensitive)
+  var packages: PackageInfo
   var pos = dir.len-1
   if dir[pos] in {DirSep, AltSep}: inc(pos)
   for k,p in os.walkDir(dir):
diff --git a/lib/std/sha1.nim b/lib/std/sha1.nim
index bda0c6de7c9fa..fce0e14cae8de 100644
--- a/lib/std/sha1.nim
+++ b/lib/std/sha1.nim
@@ -275,3 +275,14 @@ proc `==`*(a, b: SecureHash): bool =
 
   # Not a constant-time comparison, but that's acceptable in this context
   Sha1Digest(a) == Sha1Digest(b)
+
+proc isValidSha1Hash*(s: string): bool =
+  ## Checks whether given string is a valid sha1 hash value.
+  if s.len != 40:
+    # A valid sha1 hash should be 40 characters long string.
+    return false
+  for c in s:
+    if c notin HexDigits:
+      # It should also contain only hexadecimal digits.
+      return false
+  return true
diff --git a/tests/compiler/tnimblecmd.nim b/tests/compiler/tnimblecmd.nim
index d39179be189f8..576200e6aecab 100644
--- a/tests/compiler/tnimblecmd.nim
+++ b/tests/compiler/tnimblecmd.nim
@@ -1,26 +1,77 @@
 include compiler/[nimblecmd]
+import compiler/[options, lineinfos]
+import std/[sets]
 
 proc v(s: string): Version = s.newVersion
-# #head is special in the sense that it's assumed to always be newest.
-doAssert v"1.0" < v"#head"
-doAssert v"1.0" < v"1.1"
-doAssert v"1.0.1" < v"1.1"
-doAssert v"1" < v"1.1"
-doAssert v"#aaaqwe" < v"1.1" # We cannot assume that a branch is newer.
-doAssert v"#a111" < v"#head"
-
-let conf = newConfigRef()
-var rr = newStringTable()
-addPackage conf, rr, "irc-#a111", unknownLineInfo
-addPackage conf, rr, "irc-#head", unknownLineInfo
-addPackage conf, rr, "irc-0.1.0", unknownLineInfo
-#addPackage conf, rr, "irc", unknownLineInfo
-#addPackage conf, rr, "another", unknownLineInfo
-addPackage conf, rr, "another-0.1", unknownLineInfo
-
-addPackage conf, rr, "ab-0.1.3", unknownLineInfo
-addPackage conf, rr, "ab-0.1", unknownLineInfo
-addPackage conf, rr, "justone-1.0", unknownLineInfo
-
-doAssert toSeq(rr.chosen) ==
-  @["irc-#head", "ab-0.1.3", "justone-1.0", "another-0.1"]
+
+proc testVersionComparison() =
+  #head is special in the sense that it's assumed to always be newest.
+  doAssert v"1.0" < v"#head"
+  doAssert v"1.0" < v"1.1"
+  doAssert v"1.0.1" < v"1.1"
+  doAssert v"1" < v"1.1"
+  doAssert v"#aaaqwe" < v"1.1" # We cannot assume that a branch is newer.
+  doAssert v"#a111" < v"#head"
+
+proc testAddPackageWithoutChecksum() =
+  ## For backward compatibility it is not required all packages to have a
+  ## sha1 checksum at the end of the name of the Nimble cache directory.
+  ## This way a new compiler will be able to work with an older Nimble.
+
+  let conf = newConfigRef()
+  var rr: PackageInfo
+
+  addPackage conf, rr, "irc-#a111", unknownLineInfo
+  addPackage conf, rr, "irc-#head", unknownLineInfo
+  addPackage conf, rr, "irc-0.1.0", unknownLineInfo
+
+  addPackage conf, rr, "another-0.1", unknownLineInfo
+
+  addPackage conf, rr, "ab-0.1.3", unknownLineInfo
+  addPackage conf, rr, "ab-0.1", unknownLineInfo
+  addPackage conf, rr, "justone-1.0", unknownLineInfo
+
+  doAssert toSeq(rr.chosen).toHashSet ==
+    ["irc-#head", "another-0.1", "ab-0.1.3", "justone-1.0"].toHashSet
+
+proc testAddPackageWithChecksum() =
+  let conf = newConfigRef()
+  var rr: PackageInfo
+
+  # In the case of packages with the same version, but different checksums for
+  # now the first one will be chosen
+
+  addPackage conf, rr, "irc-#a111-DBC1F902CB79946E990E38AF51F0BAD36ACFABD9",
+              unknownLineInfo
+  addPackage conf, rr, "irc-#head-042D4BE2B90ED0672E717D71850ABDB0A2D19CD1",
+              unknownLineInfo
+  addPackage conf, rr, "irc-#head-042D4BE2B90ED0672E717D71850ABDB0A2D19CD2",
+              unknownLineInfo
+  addPackage conf, rr, "irc-0.1.0-6EE6DE936B32E82C7DBE526DA3463574F6568FAF",
+              unknownLineInfo
+
+  addPackage conf, rr, "another-0.1", unknownLineInfo
+  addPackage conf, rr, "another-0.1-F07EE6040579F0590608A8FD34F5F2D91D859340",
+              unknownLineInfo
+
+  addPackage conf, rr, "ab-0.1.3-34BC3B72CE46CF5A496D1121CFEA7369385E9EA2",
+              unknownLineInfo
+  addPackage conf, rr, "ab-0.1.3-24BC3B72CE46CF5A496D1121CFEA7369385E9EA2",
+              unknownLineInfo
+  addPackage conf, rr, "ab-0.1-A3CFFABDC4759F7779D541F5E031AED17169390A",
+              unknownLineInfo
+
+  # lower case hex digits is also a valid sha1 checksum
+  addPackage conf, rr, "justone-1.0-f07ee6040579f0590608a8fd34f5f2d91d859340",
+              unknownLineInfo
+
+  doAssert toSeq(rr.chosen).toHashSet == [
+    "irc-#head-042D4BE2B90ED0672E717D71850ABDB0A2D19CD1",
+    "another-0.1",
+    "ab-0.1.3-34BC3B72CE46CF5A496D1121CFEA7369385E9EA2",
+    "justone-1.0-f07ee6040579f0590608a8fd34f5f2d91d859340"
+    ].toHashSet
+
+testVersionComparison()
+testAddPackageWithoutChecksum()
+testAddPackageWithChecksum()
diff --git a/tests/stdlib/tsha1.nim b/tests/stdlib/tsha1.nim
index 7e67ccaf61d2a..5dfb3559040de 100644
--- a/tests/stdlib/tsha1.nim
+++ b/tests/stdlib/tsha1.nim
@@ -11,3 +11,13 @@ checkVector("", "da39a3ee5e6b4b0d3255bfef95601890afd80709")
 checkVector("abc", "a9993e364706816aba3e25717850c26c9cd0d89d")
 checkVector("abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq",
             "84983e441c3bd26ebaae4aa1f95129e5e54670f1")
+
+proc testIsValidSha1Hash =
+  doAssert not isValidSha1Hash("")
+  doAssert not isValidSha1Hash("9")
+  doAssert not isValidSha1Hash("99345ce680cd3e48acdb9ab4212e4bd9bf9358g7")
+  doAssert not isValidSha1Hash("99345ce680cd3e48acdb9ab4212e4bd9bf9358b")
+  doAssert isValidSha1Hash("99345CE680CD3E48ACDB9AB4212E4BD9BF9358B7")
+  doAssert isValidSha1Hash("99345ce680cd3e48acdb9ab4212e4bd9bf9358b7")
+
+testIsValidSha1Hash()