Fix VCS revision upgrades to use correct commit hash

Author: jmgomez

src/nimblepkg/download.nim

@@ -801,7 +801,8 @@ proc downloadPkg*(url: string, verRange: VersionRange,
   result.dir = pkgDir
 
   #when using a persistent download dir we can skip the download if it's already done
-  if pkgDirHasNimble(result.dir, options):
+  # BUT: for specific VCS revisions, we need to ensure the correct revision is checked out
+  if pkgDirHasNimble(result.dir, options) and vcsRevision == notSetSha1Hash:
     return # already downloaded, skipping
 
   if options.offline:
@@ -882,7 +883,8 @@ proc downloadPkgAsync*(url: string, verRange: VersionRange,
   result.dir = pkgDir
 
   #when using a persistent download dir we can skip the download if it's already done
-  if pkgDirHasNimble(result.dir, options):
+  # BUT: for specific VCS revisions, we need to ensure the correct revision is checked out
+  if pkgDirHasNimble(result.dir, options) and vcsRevision == notSetSha1Hash:
     return # already downloaded, skipping
 
   if options.offline:

src/nimblepkg/nimblesat.nim

@@ -597,8 +597,18 @@ proc downloadFromDownloadInfo*(dlInfo: PackageDownloadInfo, options: Options, ni
     let downloadRes = (dir: pkgInfo.getNimbleFileDir(), version: pkgInfo.basicInfo.version, vcsRevision: notSetSha1Hash)
     (downloadRes, none(DownloadMethod))
   else:
+    # Extract VCS revision from version range if it's a special version (e.g., @#commit-hash)
+    var vcsRev = notSetSha1Hash
+    var downloadDir = dlInfo.downloadDir
+    if dlInfo.pv.ver.kind == verSpecial:
+      let speStr = $dlInfo.pv.ver.spe
+      # Remove '#' prefix if present (e.g., "#head" or "#abc123...")
+      let revStr = if speStr.startsWith("#"): speStr[1..^1] else: speStr
+      vcsRev = initSha1Hash(revStr)
+      # Don't use cached download dir for specific revisions - let downloadPkg create unique dir
+      downloadDir = ""
     let downloadRes = downloadPkg(dlInfo.url, dlInfo.pv.ver, dlInfo.meth.get, dlInfo.subdir, options,
-                  dlInfo.downloadDir, vcsRevision = notSetSha1Hash, nimBin = nimBin)
+                  downloadDir, vcsRevision = vcsRev, nimBin = nimBin)
     (downloadRes, dlInfo.meth)
 
 proc downloadPkgFromUrl*(pv: PkgTuple, options: Options, doPrompt = false, nimBin: string): (DownloadPkgResult, Option[DownloadMethod]) = 
@@ -967,8 +977,18 @@ proc downloadFromDownloadInfoAsync*(dlInfo: PackageDownloadInfo, options: Option
     except Exception as e:
       raise newException(CatchableError, e.msg)
   else:
+    # Extract VCS revision from version range if it's a special version (e.g., @#commit-hash)
+    var vcsRev = notSetSha1Hash
+    var downloadDir = dlInfo.downloadDir
+    if dlInfo.pv.ver.kind == verSpecial:
+      let speStr = $dlInfo.pv.ver.spe
+      # Remove '#' prefix if present (e.g., "#head" or "#abc123...")
+      let revStr = if speStr.startsWith("#"): speStr[1..^1] else: speStr
+      vcsRev = initSha1Hash(revStr)
+      # Don't use cached download dir for specific revisions - let downloadPkgAsync create unique dir
+      downloadDir = ""
     let downloadRes = await downloadPkgAsync(dlInfo.url, dlInfo.pv.ver, dlInfo.meth.get, dlInfo.subdir, options,
-                  dlInfo.downloadDir, vcsRevision = notSetSha1Hash, nimBin = nimBin)
+                  downloadDir, vcsRevision = vcsRev, nimBin = nimBin)
     return (downloadRes, dlInfo.meth)
 
 proc downloadPkgFromUrlAsync*(pv: PkgTuple, options: Options, doPrompt = false, nimBin: string): Future[(DownloadPkgResult, Option[DownloadMethod])] {.async.} =
@@ -1034,6 +1054,11 @@ proc downloadMinimalPackageAsync*(pv: PkgTuple, options: Options, nimBin: string
   ## Async version of downloadMinimalPackage with deduplication.
   ## If multiple calls request the same package concurrently, they share the same download.
   ## Cache key uses canonical package URL (not version) since we download all versions anyway.
+  ## For specific versions (verSpecial, verEq), skip cache to ensure correct revision is downloaded.
+
+  # Skip cache for specific versions/revisions - they need exact download
+  if pv.ver.kind in [verSpecial, verEq]:
+    return await downloadMinimalPackageAsyncImpl(pv, options, nimBin)
 
   # Get canonical URL to use as cache key (handles both short names and full URLs)
   var cacheKey: string

src/nimblepkg/packagemetadatafile.nim

@@ -76,3 +76,6 @@ proc loadMetaData*(dirName: string, raiseIfNotFound: bool, options: Options): Pa
 proc fillMetaData*(packageInfo: var PackageInfo, dirName: string,
                    raiseIfNotFound: bool, options: Options) =
   packageInfo.metaData = loadMetaData(dirName, raiseIfNotFound, options)
+  # Update checksum from metadata if it's set (important for packages downloaded with specific VCS revision)
+  if packageInfo.metaData.vcsRevision != notSetSha1Hash:
+    packageInfo.basicInfo.checksum = packageInfo.metaData.vcsRevision

src/nimblepkg/packageparser.nim

@@ -336,14 +336,31 @@ proc readPackageInfo(pkgInfo: var PackageInfo, nf: NimbleFile, options: Options,
     pkgInfo.metaData.specialVersions.incl pkgInfo.basicInfo.version
     # If the `fileDir` is a VCS repository we can get some of the package meta
     # data from it.
+    var currentVcsRevision = notSetSha1Hash
     try:
-      pkgInfo.metaData.vcsRevision = getVcsRevision(fileDir)
+      currentVcsRevision = getVcsRevision(fileDir)
+      pkgInfo.metaData.vcsRevision = currentVcsRevision
     except CatchableError:
       raise nimbleError(
         msg = "Failed to get VCS revision of your project!",
         hint = "Try making a commit to your project if you haven't made one yet."
       )
 
+    # Try to load metadata from nimblemeta.json (for packages downloaded with specific VCS revision)
+    try:
+      fillMetaData(pkgInfo, fileDir, raiseIfNotFound = false, options)
+    except CatchableError:
+      discard  # Metadata file doesn't exist or failed to load, use values set above
+
+    # Always use the current VCS revision from the git repo, not from cached metadata
+    # This ensures we get the correct revision even if metadata file is stale
+    if currentVcsRevision != notSetSha1Hash:
+      pkgInfo.metaData.vcsRevision = currentVcsRevision
+
+    # For VCS packages, use the VCS revision as the checksum (more accurate than directory SHA1)
+    if pkgInfo.metaData.vcsRevision != notSetSha1Hash:
+      pkgInfo.basicInfo.checksum = pkgInfo.metaData.vcsRevision
+
     case getVcsType(fileDir)
       of vcsTypeGit: pkgInfo.metaData.downloadMethod = DownloadMethod.git
       of vcsTypeHg: pkgInfo.metaData.downloadMethod = DownloadMethod.hg

src/nimblepkg/tools.nim

@@ -169,8 +169,9 @@ proc getDownloadDirName*(uri: string, verRange: VersionRange,
   if verSimple != "":
     result.add "_"
     result.add verSimple
-  
-  if vcsRevision != notSetSha1Hash:
+
+  # Only add vcsRevision if it's not already in the version string (verSpecial already includes it)
+  if vcsRevision != notSetSha1Hash and verRange.kind != verSpecial:
     result.add "_"
     result.add $vcsRevision
 

src/nimblepkg/version.nim

@@ -353,6 +353,9 @@ proc getSimpleString*(verRange: VersionRange): string =
   case verRange.kind
   of verSpecial:
     result = $verRange.spe
+    # Remove '#' prefix for directory names (can cause issues with shell commands)
+    if result.startsWith("#"):
+      result = result[1..^1]
   of verLater, verEarlier, verEqLater, verEqEarlier, verEq:
     result = $verRange.ver
   of verIntersect, verTilde, verCaret:

src/nimblepkg/vnext.nim

@@ -15,7 +15,7 @@ After we resolve nim, we try to resolve the dependencies for a root package. Roo
 import std/[sequtils, sets, options, os, strutils, tables, strformat, algorithm]
 import nimblesat, packageinfotypes, options, version, declarativeparser, packageinfo, common,
   nimenv, lockfile, cli, downloadnim, packageparser, tools, nimscriptexecutor, packagemetadatafile,
-  displaymessages, packageinstaller, reversedeps, developfile, urls
+  displaymessages, packageinstaller, reversedeps, developfile, urls, sha1hashes
 
 when defined(windows):
   import std/strscans
@@ -658,10 +658,21 @@ proc packageExists(nimBin: string, pkgInfo: PackageInfo, options: Options):
     return some(oldPkgInfo)
 
 
-proc installFromDirDownloadInfo(nimBin: string,downloadDir: string, url: string, pv: PkgTuple, options: Options): PackageInfo {.instrument.} = 
+proc installFromDirDownloadInfo(nimBin: string,downloadDir: string, url: string, pv: PkgTuple, options: Options): PackageInfo {.instrument.} =
 
   let dir = downloadDir
   var pkgInfo = getPkgInfo(dir, options, nimBin = nimBin)
+
+  # If we requested a specific VCS revision (verSpecial), ensure we use it as the checksum
+  # This handles cases where getVcsRevision might have returned a stale value
+  if pv.ver.kind == verSpecial:
+    let speStr = $pv.ver.spe
+    let revStr = if speStr.startsWith("#"): speStr[1..^1] else: speStr
+    let requestedRevision = initSha1Hash(revStr)
+    if requestedRevision != notSetSha1Hash:
+      pkgInfo.metaData.vcsRevision = requestedRevision
+      pkgInfo.basicInfo.checksum = requestedRevision
+
   var depsOptions = options
   depsOptions.depsOnly = false
 
@@ -671,13 +682,26 @@ proc installFromDirDownloadInfo(nimBin: string,downloadDir: string, url: string,
 
   let oldPkg = packageExists(nimBin, pkgInfo, options)
   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), MediumPriority)
-    var oldPkg = oldPkg.get
-    oldPkg.metaData.specialVersions.incl pkgInfo.metaData.specialVersions
-    saveMetaData(oldPkg.metaData, oldPkg.getNimbleFileDir, changeRoots = false)
-    return oldPkg
+    # Check if checksums are different - if so, force reinstall (handles VCS revision upgrades)
+    let isDifferentPackage = pkgInfo.basicInfo.checksum != oldPkg.get.basicInfo.checksum
+
+    if not isDifferentPackage:
+      # 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), MediumPriority)
+      var oldPkg = oldPkg.get
+      oldPkg.metaData.specialVersions.incl pkgInfo.metaData.specialVersions
+      saveMetaData(oldPkg.metaData, oldPkg.getNimbleFileDir, changeRoots = false)
+      return oldPkg
+    else:
+      # Different checksum - reinstall (e.g., different VCS revision)
+      display("Upgrading", "$1 from checksum $2 to $3" %
+        [pkgInfo.basicInfo.name, $oldPkg.get.basicInfo.checksum, $pkgInfo.basicInfo.checksum],
+        priority = MediumPriority)
+      # Remove the old package directory to force reinstallation
+      let oldPkgDir = oldPkg.get.getPkgDest(options)
+      if dirExists(oldPkgDir):
+        removeDir(oldPkgDir)
 
   let pkgDestDir = pkgInfo.getPkgDest(options)