Add error checking for publishTags command and rename to versions

src/nimble.nim

@@ -2382,9 +2382,9 @@ proc doAction(options: var Options) =
   of actionPublish:
     var pkgInfo = getPkgInfo(getCurrentDir(), options)
     publish(pkgInfo, options)
-  of actionPublishTags:
+  of actionPublishVersions:
     var pkgInfo = getPkgInfo(getCurrentDir(), options)
-    publishTags(pkgInfo, options)
+    publishVersions(pkgInfo, options)
   of actionDump:
     dump(options)
   of actionTasks:

src/nimblepkg/download.nim

@@ -53,22 +53,35 @@ proc gitFetchTags*(repoDir: string, downloadMethod: DownloadMethod) =
     of DownloadMethod.hg:
       assert false, "hg not supported"
 
+proc gitTagsFromRefs(output: string, derefTags = true): OrderedTable[string, Sha1Hash] =
+  for line in output.splitLines():
+    let refStart = line.find("refs/tags/")
+    # git outputs warnings, empty lines, etc
+    if refStart == -1: continue
+    if line.len() < 50: continue
+    let hashStr = line[0..<40]
+    let start = refStart+"refs/tags/".len
+    let tag = line[start .. line.len-1]
+    let hash = initSha1Hash(hashStr)
+    if tag.endswith("^{}") and derefTags:
+      result[tag[0..^4]] = hash
+    elif not tag.endswith("^{}"):
+      result[tag] = hash
 
 proc getTagsList*(dir: string, meth: DownloadMethod): seq[string] =
   var output: string
   cd dir:
     case meth
     of DownloadMethod.git:
-      output = tryDoCmdEx("git tag")
+      output = tryDoCmdEx(&"git show-ref --dereference")
     of DownloadMethod.hg:
       output = tryDoCmdEx("hg tags")
   if output.len > 0:
     case meth
     of DownloadMethod.git:
       result = @[]
-      for i in output.splitLines():
-        if i == "": continue
-        result.add(i)
+      for item in output.gitTagsFromRefs().pairs:
+        result.add(item[0])
     of DownloadMethod.hg:
       result = @[]
       for i in output.splitLines():
@@ -81,20 +94,11 @@ proc getTagsList*(dir: string, meth: DownloadMethod): seq[string] =
     result = @[]
 
 proc getTagsListRemote*(url: string, meth: DownloadMethod): seq[string] =
-  result = @[]
   case meth
   of DownloadMethod.git:
-    var (output, exitCode) = doCmdEx(&"git ls-remote --tags {url}")
-    if exitCode != QuitSuccess:
-      raise nimbleError("Unable to query remote tags for " & url &
-                        ". Git returned: " & output)
-    for i in output.splitLines():
-      let refStart = i.find("refs/tags/")
-      # git outputs warnings, empty lines, etc
-      if refStart == -1: continue
-      let start = refStart+"refs/tags/".len
-      let tag = i[start .. i.len-1]
-      if not tag.endswith("^{}"): result.add(tag)
+    var output = tryDoCmdEx(&"git ls-remote {url}")
+    for item in output.gitTagsFromRefs().pairs:
+      result.add item[0]
 
   of DownloadMethod.hg:
     # http://stackoverflow.com/questions/2039150/show-tags-for-remote-hg-repository
@@ -116,6 +120,40 @@ proc getVersionList*(tags: seq[string]): OrderedTable[Version, string] =
               SortOrder.Descending)
   result = toOrderedTable[Version, string](taggedVers)
 
+proc gitTagCommits*(repoDir: string, downloadMethod: DownloadMethod): Table[string, Sha1Hash] =
+  ## Return a table of tag -> commit
+  case downloadMethod:
+    of DownloadMethod.git:
+      let output = tryDoCmdEx(&"git -C {repoDir} show-ref --dereference")
+      for item in output.gitTagsFromRefs().pairs:
+        result[item[0]] = item[1]
+    of DownloadMethod.hg:
+      assert false, "hg not supported"
+
+proc vcsFindCommits*(repoDir, nimbleFile: string, downloadMethod: DownloadMethod): seq[(Sha1Hash, string)] =
+  var output: string
+  case downloadMethod:
+    of DownloadMethod.git:
+      output = tryDoCmdEx(&"git -C {repoDir} log --format=\"%H %s\" -- $2")
+    of DownloadMethod.hg:
+      assert false, "hg not supported"
+
+  for line in output.splitLines():
+    let line = line.strip()
+    if line != "":
+      result.add((line[0..39].initSha1Hash(), line[40..^1]))
+
+proc vcsDiff*(commit: Sha1Hash, repoDir, nimbleFile: string, downloadMethod: DownloadMethod): seq[string] =
+  case downloadMethod:
+    of DownloadMethod.git:
+      let (output, exitCode) = doCmdEx(&"git -C {repoDir} diff {commit}~ {commit} {nimbleFile}")
+      if exitCode != QuitSuccess:
+        return @[]
+      else:
+        return output.splitLines()
+    of DownloadMethod.hg:
+      assert false, "hg not supported"
+
 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``.

src/nimblepkg/options.nim

@@ -71,7 +71,7 @@ type
     actionUninstall, actionCompile, actionDoc, actionCustom, actionTasks,
     actionDevelop, actionCheck, actionLock, actionRun, actionSync, actionSetup,
     actionClean, actionDeps, actionShellEnv, actionShell, actionAdd, actionManual,
-    actionPublishTags
+    actionPublishVersions
 
   DevelopActionType* = enum
     datAdd, datRemoveByPath, datRemoveByName, datInclude, datExclude
@@ -125,8 +125,10 @@ type
       depsAction*: string
     of actionPublish:
       publishAction*: string
-    of actionPublishTags:
-      onlyListTags*: bool
+    of actionPublishVersions:
+      createTags*: bool
+      pushTags*: bool
+      allTags*: bool
     of actionShellEnv, actionShell:
       discard
 
@@ -180,10 +182,11 @@ Commands:
   publish                         Publishes a package on nim-lang/packages.
                                   The current working directory needs to be the
                                   top level directory of the Nimble package.
-  publishTags                     Finds and publishes new tags based on the
-                                  commits where a package's Nimble file changed.
-               [-l, --listOnly]   Only list the tags and versions which are found without
-                                  actually performing tag or publishing them.
+  versions                        Lists package versions based on commits in the
+                                  current branch where the package's Nimble version
+                                  was changed.
+               [-c, --create]     Creates tags for missing versions.
+               [-p, --push]       Push only tagged versions (tags) to VCS.
   uninstall    [pkgname, ...]     Uninstalls a list of packages.
                [-i, --inclDeps]   Uninstalls package and dependent package(s).
   build        [opts, ...] [bin]  Builds a package. Passes options to the Nim
@@ -338,8 +341,8 @@ proc parseActionType*(action: string): ActionType =
     result = actionUninstall
   of "publish":
     result = actionPublish
-  of "publishtags":
-    result = actionPublishTags
+  of "versions", "publishversions":
+    result = actionPublishVersions
   of "upgrade":
     result = actionUpgrade
   of "tasks":
@@ -773,10 +776,14 @@ proc parseFlag*(flag, val: string, result: var Options, kind = cmdLongOption) =
       result.action.publishAction = "tags"
     else:
       wasFlagHandled = false
-  of actionPublishTags:
+  of actionPublishVersions:
     case f
-    of "l", "listonly":
-      result.action.onlyListTags = true
+    of "c", "create":
+      result.action.createTags = true
+    of "p", "push":
+      result.action.pushTags = true
+    of "a", "all":
+      result.action.allTags = true
     else:
       wasFlagHandled = false
   of actionDeps:

src/nimblepkg/publish.nim

@@ -6,8 +6,8 @@
 
 import system except TResult
 import httpclient, strutils, json, os, browsers, times, uri
-import common, tools, cli, config, options, packageinfotypes, sha1hashes, version, download
-import strformat, sequtils, pegs, sets
+import common, tools, cli, config, options, packageinfotypes, vcstools, sha1hashes, version, download
+import strformat, sequtils, pegs, sets, tables, algorithm
 {.warning[UnusedImport]: off.}
 from net import SslCVerifyMode, newContext
 
@@ -248,73 +248,107 @@ proc publish*(p: PackageInfo, o: Options) =
     let prUrl = createPullRequest(auth, p, url, branchName)
     display("Success:", "Pull request successful, check at " & prUrl , Success, HighPriority)
 
-proc vcsFindCommits*(repoDir, nimbleFile: string, downloadMethod: DownloadMethod): seq[(Sha1Hash, string)] =
-  var output: string
-  case downloadMethod:
-    of DownloadMethod.git:
-      output = tryDoCmdEx(&"git -C {repoDir} log --format=\"%H %s\" -- $2")
-    of DownloadMethod.hg:
-      assert false, "hg not supported"
-
-  for line in output.splitLines():
-    let line = line.strip()
-    if line != "":
-      result.add((line[0..39].initSha1Hash(), line[40..^1]))
-
-proc vcsDiff*(commit: Sha1Hash, repoDir, nimbleFile: string, downloadMethod: DownloadMethod): seq[string] =
+proc createTag*(tag: string, commit: Sha1Hash, message, repoDir, nimbleFile: string, downloadMethod: DownloadMethod): bool =
   case downloadMethod:
     of DownloadMethod.git:
-      let (output, exitCode) = doCmdEx(&"git -C {repoDir} diff {commit}~ {commit} {nimbleFile}")
-      if exitCode != QuitSuccess:
-        return @[]
-      else:
-        return output.splitLines()
+      let (output, code) = doCmdEx(&"git -C {repoDir} tag -a {tag.quoteShell()} {commit} -m {message.quoteShell()}")
+      result = code == QuitSuccess
+      if not result:
+        displayError(&"Failed to create tag {tag.quoteShell()} with error {output}")
     of DownloadMethod.hg:
       assert false, "hg not supported"
 
-proc createTag*(tag: string, commit: Sha1Hash, message, repoDir, nimbleFile: string, downloadMethod: DownloadMethod): bool =
+proc pushTags*(tags: seq[string], repoDir: string, downloadMethod: DownloadMethod): bool =
   case downloadMethod:
     of DownloadMethod.git:
-      let (output, code) = doCmdEx(&"git -C {repoDir} tag -a {tag.quoteShell()} {commit} -m {message.quoteShell()}")
+      # git push origin tag experiment-0.8.1
+      let tags = tags.mapIt(it.quoteShell()).join(" ")
+      let (output, code) = doCmdEx(&"git -C {repoDir} push origin tag {tags} ")
       result = code == QuitSuccess
       if not result:
-        displayError(&"Failed to create tag {tag.quoteShell()} with error {output}")
+        displayError(&"Failed to push tag {tags} with error {output}")
     of DownloadMethod.hg:
       assert false, "hg not supported"
 
+const TagVersionFmt = "v$1"
+
 proc findVersions(commits: seq[(Sha1Hash, string)], projdir, nimbleFile: string, downloadMethod: DownloadMethod, options: Options) =
   ## parse the versions
   var
-    versions: HashSet[Version]
-    existingTags: HashSet[Version]
-  for tag in getTagsList(projdir, downloadMethod):
-    let tag = tag.strip(leading=true, chars={'v'})
-    try:
-      existingTags.incl(newVersion(tag))
-    except ParseVersionError:
-      discard
+    versions: OrderedTable[Version, tuple[commit: Sha1Hash, message: string]]
+    existingTags = gitTagCommits(projdir, downloadMethod)
+    existingVers = existingTags.keys().toSeq().getVersionList()
+
+  let currBranch = getCurrentBranch(projdir)
+  if currBranch notin ["main", "master"]:
+    displayWarning(&"Note runnig this command on a non-standard primary branch `{currBranch}` may have unintened consequences", HighPriority)
+
+  for ver, tag in existingVers.pairs():
+    let commit = existingTags[tag]
+    displayInfo(&"Existing version {ver} with tag {tag} at commit {$commit} ", HighPriority)
 
   # adapted from @beef331's algorithm https://github.com/beef331/graffiti/blob/master/src/graffiti.nim
-  for (commit, message) in commits:
-    # echo "commit: ", commit
-    let diffs = vcsDiff(commit, projdir, nimbleFile, downloadMethod)
-    for line in diffs:
-      var matches: array[0..MaxSubpatterns, string]
-      if line.find(peg"'+version' \s* '=' \s* {[\34\39]} {@} $1", matches) > -1:
-        let version = newVersion(matches[1])
-        if version notin versions: 
-          versions.incl(version)
-          if version in existingTags:
-            displayInfo(&"Found existing tag for version {version} at commit {commit}", HighPriority)
-          else:
-            displayInfo(&"Found new version {version} at {commit}", HighPriority)
-            if not options.action.onlyListTags:
-              displayWarning(&"Creating tag for new version {version} at {commit}", HighPriority)
-              let res = createTag(&"v{version}", commit, message, projdir, nimbleFile, downloadMethod)
-              if not res:
-                displayError(&"Unable to create tag {version}", HighPriority)
-
-proc publishTags*(p: PackageInfo, options: Options) =
+  block outer:
+    for (commit, message) in commits:
+      # echo "commit: ", commit
+      let diffs = vcsDiff(commit, projdir, nimbleFile, downloadMethod)
+      for line in diffs:
+        var matches: array[0..MaxSubpatterns, string]
+        if line.find(peg"'+version' \s* '=' \s* {[\34\39]} {@} $1", matches) > -1:
+          let ver = newVersion(matches[1])
+          if ver notin versions:
+            if ver in existingVers:
+              if options.action.allTags:
+                displayWarning(&"Skipping historical version {ver} at commit {commit} that has an existing tag", HighPriority)
+              else:
+                break outer
+            else:
+              displayInfo(&"Found new version {ver} at {commit}", HighPriority)
+              versions[ver] = (commit: commit, message: message)
+
+  var nonMonotonicVers: Table[Version, Sha1Hash]
+  if versions.len() >= 2:
+    let versions = versions.pairs().toSeq()
+    var monotonics: seq[Version]
+    for idx in 1 ..< versions.len() - 1:
+      let
+        prev = versions[idx-1]
+        (ver, info) = versions[idx]
+        prevMonotonicsOk = monotonics.mapIt(ver < it).all(proc (x: bool): bool = x)
+
+      if ver < prev[0] and prevMonotonicsOk:
+        displayHint(&"Versions monotonic between tag {TagVersionFmt % $ver}@{info.commit} " &
+                      &" and previous tag of {TagVersionFmt % $prev[0]}@{prev[1].commit}", MediumPriority)
+      else:
+        if prev[0] notin nonMonotonicVers:
+          monotonics.add(prev[0]) # track last largest monotonic so we can check, e.g. 0.2, 3.0, 0.3 and not 0.2, 3.0, 0.2 
+        nonMonotonicVers[ver] = info.commit
+        displayError(&"Non-monotonic (decreasing) version found between tag {TagVersionFmt % $ver}@{info.commit}" &
+                     &" and the previous tag {TagVersionFmt % $prev[0]}@{prev[1].commit}", HighPriority)
+        displayWarning(&"Version {ver} will be skipped. Please tag it manually if the version is correct." , HighPriority)
+        displayHint(&"Note that versions are checked from larget to smallest" , HighPriority)
+        displayHint(&"Note smaller versions later in history are always peferred. Please manually review your tags before pushing." , HighPriority)
+
+  var newTags: HashSet[string]
+  if options.action.createTags:
+    for (version, info) in versions.pairs:
+      if version in nonMonotonicVers:
+        displayWarning(&"Skipping creating tag for non-monotonic {version} at {info.commit}", HighPriority)
+      else:
+        let tag = TagVersionFmt % [$version]
+        displayWarning(&"Creating tag for new version {version} at {info.commit}", HighPriority)
+        let res = createTag(tag, info.commit, info.message, projdir, nimbleFile, downloadMethod)
+        if not res:
+          displayError(&"Unable to create tag {TagVersionFmt % $version}", HighPriority)
+        else:
+          newTags.incl(tag)
+
+  if options.action.pushTags:
+    let res = pushTags(newTags.toSeq(), projdir, downloadMethod)
+    if not res:
+      displayError(&"Error pushing tags", HighPriority)
+
+proc publishVersions*(p: PackageInfo, options: Options) =
   displayInfo(&"Searcing for new tags for {$p.basicInfo.name} @{$p.basicInfo.version}", HighPriority)
   let (projdir, file, ext) = p.myPath.splitFile()
   let nimblefile = file & ext

tests/tester.nim

@@ -12,6 +12,7 @@ import tdevelopfeature
 import tissues
 import tlocaldeps
 import tlockfile
+import tpublish
 import tmisctests
 import tmoduletests
 import tmultipkgs

tests/testscommon.nim

@@ -6,6 +6,7 @@
 import sequtils, strutils, strformat, os, osproc, sugar, unittest, macros
 import pkg/checksums/sha1
 
+import nimblepkg/cli
 from nimblepkg/common import cd, nimblePackagesDirName, ProcessOutput
 from nimblepkg/developfile import developFileVersion
 
@@ -70,7 +71,8 @@ template verify*(res: (string, int)) =
   check r[1] == QuitSuccess
 
 proc processOutput*(output: string): seq[string] =
-  output.strip.splitLines().filter(
+  checkpoint(output)
+  result = output.strip.splitLines().filter(
     (x: string) => (
       x.len > 0 and
       "Using env var NIM_LIB_PREFIX" notin x
@@ -207,6 +209,9 @@ proc writeDevelopFile*(path: string, includes: seq[string],
 # Set env var to propagate nimble binary path
 putEnv("NIMBLE_TEST_BINARY_PATH", nimblePath)
 
+setVerbosity(MediumPriority)
+setShowColor(false)
+
 # Always recompile.
 block:
   # Verbose name is used for exit code so assert is clearer