Fixes #504 (#683)

* Improve messaging for uninstall

* Uninstall if in right order

* Fix CI - cannot looks better

* Raise exception when nothing to remove

* Test case, minor test fix

* 0.19.6 fixes

* Fix Nim devel hash

* Feedback

.travis.yml

@@ -6,13 +6,13 @@ language: c
 
 env:
   - BRANCH=0.19.6
-  - BRANCH=0.20.0
-  - BRANCH=#ced0527ae334439a10e1719d1eccb727c19dc781
+  - BRANCH=0.20.2
+  - BRANCH=#44aadd50cfa647a759610a15967960632bf597ce
 
 cache:
   directories:
     - "$HOME/.choosenim/toolchains/nim-0.19.6"
-    - "$HOME/.choosenim/toolchains/nim-0.20.0"
+    - "$HOME/.choosenim/toolchains/nim-0.20.2"
 
 install:
   - export CHOOSENIM_CHOOSE_VERSION=$BRANCH

src/nimble.nim

@@ -840,7 +840,8 @@ proc uninstall(options: Options) =
     raise newException(NimbleError,
         "Please specify the package(s) to uninstall.")
 
-  var pkgsToDelete: seq[PackageInfo] = @[]
+  var pkgsToDelete: HashSet[PackageInfo]
+  pkgsToDelete.init()
   # Do some verification.
   for pkgTup in options.action.packages:
     display("Looking", "for $1 ($2)" % [pkgTup.name, $pkgTup.ver],
@@ -851,37 +852,33 @@ proc uninstall(options: Options) =
       raise newException(NimbleError, "Package not found")
 
     display("Checking", "reverse dependencies", priority = HighPriority)
-    var errors: seq[string] = @[]
     for pkg in pkgList:
       # Check whether any packages depend on the ones the user is trying to
       # uninstall.
       if options.uninstallRevDeps:
         getAllRevDeps(options, pkg, pkgsToDelete)
       else:
-        let revDeps = getRevDeps(options, pkg)
+        let
+          revDeps = getRevDeps(options, pkg)
         var reason = ""
-        if revDeps.len == 1:
-          reason = "$1 ($2) depends on it" % [revDeps[0].name, $revDeps[0].ver]
-        else:
-          for i in 0 ..< revDeps.len:
-            reason.add("$1 ($2)" % [revDeps[i].name, $revDeps[i].ver])
-            if i != revDeps.len-1:
-              reason.add ", "
-          reason.add " depend on it"
+        for revDep in revDeps:
+          if reason.len != 0: reason.add ", "
+          reason.add("$1 ($2)" % [revDep.name, revDep.version])
+        if reason.len != 0:
+          reason &= " depend" & (if revDeps.len == 1: "s" else: "") & " on it"
 
-        if revDeps.len > 0:
-          errors.add("Cannot uninstall $1 ($2) because $3" %
-                     [pkgTup.name, pkg.specialVersion, reason])
+        if len(revDeps - pkgsToDelete) > 0:
+          display("Cannot", "uninstall $1 ($2) because $3" %
+                  [pkgTup.name, pkg.specialVersion, reason], Warning, HighPriority)
         else:
-          pkgsToDelete.add pkg
+          pkgsToDelete.incl pkg
 
-    if pkgsToDelete.len == 0:
-      raise newException(NimbleError, "\n  " & errors.join("\n  "))
+  if pkgsToDelete.len == 0:
+    raise newException(NimbleError, "Failed uninstall - no packages selected")
 
   var pkgNames = ""
-  for i in 0 ..< pkgsToDelete.len:
-    if i != 0: pkgNames.add ", "
-    let pkg = pkgsToDelete[i]
+  for pkg in pkgsToDelete.items:
+    if pkgNames.len != 0: pkgNames.add ", "
     pkgNames.add("$1 ($2)" % [pkg.name, pkg.specialVersion])
 
   # Let's confirm that the user wants these packages removed.

src/nimblepkg/packageinfo.nim

@@ -3,7 +3,7 @@
 
 # Stdlib imports
 import system except TResult
-import parsecfg, json, streams, strutils, parseutils, os, sets, tables
+import hashes, parsecfg, json, streams, strutils, parseutils, os, sets, tables
 import httpclient
 
 # Local imports
@@ -542,6 +542,11 @@ proc `==`*(pkg1: PackageInfo, pkg2: PackageInfo): bool =
   if pkg1.name == pkg2.name and pkg1.myPath == pkg2.myPath:
     return true
 
+proc hash*(x: PackageInfo): Hash =
+  var h: Hash = 0
+  h = h !& hash(x.myPath)
+  result = !$h
+
 when isMainModule:
   doAssert getNameVersion("/home/user/.nimble/libs/packagea-0.1") ==
       ("packagea", "0.1")

src/nimblepkg/reversedeps.nim

@@ -1,7 +1,7 @@
 # Copyright (C) Dominik Picheta. All rights reserved.
 # BSD License. Look at license.txt for more info.
 
-import os, json
+import os, json, sets
 
 import options, common, version, download, packageinfo
 
@@ -58,7 +58,7 @@ proc removeRevDep*(nimbleData: JsonNode, pkg: PackageInfo) =
         newData[key] = newVal
   nimbleData["reverseDeps"] = newData
 
-proc getRevDeps*(options: Options, pkg: PackageInfo): seq[PkgTuple] =
+proc getRevDepTups*(options: Options, pkg: PackageInfo): seq[PkgTuple] =
   ## Returns a list of *currently installed* reverse dependencies for `pkg`.
   result = @[]
   let thisPkgsDep =
@@ -76,18 +76,25 @@ proc getRevDeps*(options: Options, pkg: PackageInfo): seq[PkgTuple] =
 
       result.add(pkgTup)
 
-proc getAllRevDeps*(options: Options, pkg: PackageInfo, result: var seq[PackageInfo]) =
+proc getRevDeps*(options: Options, pkg: PackageInfo): HashSet[PackageInfo] =
+  result.init()
+  let installedPkgs = getInstalledPkgsMin(options.getPkgsDir(), options)
+  for rdepTup in getRevDepTups(options, pkg):
+    for rdepInfo in findAllPkgs(installedPkgs, rdepTup):
+      result.incl rdepInfo
+
+proc getAllRevDeps*(options: Options, pkg: PackageInfo, result: var HashSet[PackageInfo]) =
   if pkg in result:
     return
 
   let installedPkgs = getInstalledPkgsMin(options.getPkgsDir(), options)
-  for rdepTup in getRevDeps(options, pkg):
+  for rdepTup in getRevDepTups(options, pkg):
     for rdepInfo in findAllPkgs(installedPkgs, rdepTup):
       if rdepInfo in result:
         continue
 
       getAllRevDeps(options, rdepInfo, result)
-  result.add pkg
+  result.incl pkg
 
 when isMainModule:
   var nimbleData = %{"reverseDeps": newJObject()}

tests/tester.nim

@@ -465,8 +465,7 @@ test "can uninstall":
 
     let ls = outp.strip.processOutput()
     check exitCode != QuitSuccess
-    check "Cannot uninstall issue27b (0.1.0) because issue27a (0.1.0) depends" &
-          " on it" in ls[ls.len-1]
+    check inLines(ls, "Cannot uninstall issue27b (0.1.0) because issue27a (0.1.0) depends")
 
     check execNimble("uninstall", "-y", "issue27").exitCode == QuitSuccess
     check execNimble("uninstall", "-y", "issue27a").exitCode == QuitSuccess
@@ -827,3 +826,17 @@ test "init does not overwrite existing files (#581)":
     check execNimbleYes("init").exitCode == QuitSuccess
     check readFile("src/issue581.nim") == Src
   removeDir("issue581")
+
+test "remove skips packages with revDeps (#504)":
+  check execNimble("install", "nimboost@0.5.5", "nimfp@0.4.4", "-y").exitCode == QuitSuccess
+
+  var (output, exitCode) = execNimble("uninstall", "nimboost", "nimfp", "-n")
+  var lines = output.strip.processOutput()
+  check inLines(lines, "Cannot uninstall nimboost")
+
+  (output, exitCode) = execNimble("uninstall", "nimfp", "nimboost", "-y")
+  lines = output.strip.processOutput()
+  check (not inLines(lines, "Cannot uninstall nimboost"))
+
+  check execNimble("path", "nimboost").exitCode != QuitSuccess
+  check execNimble("path", "nimfp").exitCode != QuitSuccess