Refactoring and many fixes to PR #378. (Broken)

This commit is broken due to a cleanup issue with NimScript eval.

src/nimble.nim

@@ -13,7 +13,8 @@ from sequtils import toSeq
 import nimblepkg/packageinfo, nimblepkg/version, nimblepkg/tools,
        nimblepkg/download, nimblepkg/config, nimblepkg/common,
        nimblepkg/publish, nimblepkg/options, nimblepkg/packageparser,
-       nimblepkg/cli, nimblepkg/packageinstaller, nimblepkg/reversedeps
+       nimblepkg/cli, nimblepkg/packageinstaller, nimblepkg/reversedeps,
+       nimblepkg/nimscriptexecutor
 
 import nimblepkg/nimscriptsupport
 
@@ -529,28 +530,6 @@ proc execBackend(options: Options) =
         [backend, args, bin], showOutput = true)
   display("Success:", "Execution finished", Success, HighPriority)
 
-proc tempOutArg(file: string): string =
-  ## Returns the `--out` argument to pass to the compiler, using a temp file
-  let (_, name, _) = splitFile(file)
-  let dir = getNimbleTempDir() / "tests"
-  createDir(dir)
-  result = "--out:" & (dir / name)
-
-proc test(options: Options) =
-  ## Executes all tests
-  var files = toSeq(walkDir(getCurrentDir() / "tests"))
-  files.sort do (a, b: auto) -> int:
-    result = cmp(a.path, b.path)
-
-  for file in files:
-    if file.path.endsWith(".nim") and file.kind in { pcFile, pcLinkToFile }:
-      var optsCopy = options
-      optsCopy.action.file = file.path
-      optsCopy.action.backend = "c -r"
-      optsCopy.action.compileOptions.add("--path:.")
-      optsCopy.action.compileOptions.add(file.path.tempOutArg)
-      execBackend(optsCopy)
-
 proc search(options: Options) =
   ## Searches for matches in ``options.action.search``.
   ##
@@ -922,25 +901,23 @@ proc develop(options: Options) =
       discard downloadPkg(url, pv.ver, meth, options, downloadDir)
       developFromDir(downloadDir, options)
 
-proc execHook(options: Options, before: bool): bool =
-  ## Returns whether to continue.
-  result = true
-  var nimbleFile = ""
-  try:
-    nimbleFile = findNimbleFile(getCurrentDir(), true)
-  except NimbleError: return true
-  # PackageInfos are cached so we can read them as many times as we want.
-  let pkgInfo = getPkgInfoFromFile(nimbleFile, options)
-  let actionName =
-    if options.action.typ == actionCustom: options.action.command
-    else: ($options.action.typ)[6 .. ^1]
-  let hookExists =
-    if before: actionName.normalize in pkgInfo.preHooks
-    else: actionName.normalize in pkgInfo.postHooks
-  if pkgInfo.isNimScript and hookExists:
-    let res = execHook(nimbleFile, actionName, before, options)
-    if res.success:
-      result = res.retVal
+proc test(options: Options) =
+  ## Executes all tests.
+  var files = toSeq(walkDir(getCurrentDir() / "tests"))
+  files.sort((a, b) => cmp(a.path, b.path))
+
+  for file in files:
+    if file.path.endsWith(".nim") and file.kind in {pcFile, pcLinkToFile}:
+      var optsCopy = options.briefClone()
+      optsCopy.action.typ = actionCompile
+      optsCopy.action.file = file.path
+      optsCopy.action.backend = "c"
+      optsCopy.action.compileOptions = @[]
+      optsCopy.action.compileOptions.add("-r")
+      optsCopy.action.compileOptions.add("--path:.")
+      execBackend(optsCopy)
+
+  display("Success:", "All tests passed", Success, HighPriority)
 
 proc doAction(options: Options) =
   if options.showHelp:
@@ -982,8 +959,6 @@ proc doAction(options: Options) =
     listPaths(options)
   of actionBuild:
     build(options)
-  of actionTest:
-    test(options)
   of actionCompile, actionDoc:
     execBackend(options)
   of actionInit:
@@ -1000,37 +975,19 @@ proc doAction(options: Options) =
   of actionNil:
     assert false
   of actionCustom:
-    # Custom command. Attempt to call a NimScript task.
-    let nimbleFile = findNimbleFile(getCurrentDir(), true)
-    if not nimbleFile.isNimScript(options):
-      writeHelp()
+    let isPreDefined = options.action.command.normalize == "test"
 
-    let execResult = execTask(nimbleFile, options.action.command, options)
-    if not execResult.success:
-      raiseNimbleError(msg = "Could not find task $1 in $2" %
-                             [options.action.command, nimbleFile],
-                       hint = "Run `nimble --help` and/or `nimble tasks` for" &
-                              " a list of possible commands.")
-
-    if execResult.command.normalize == "nop":
-      display("Warning:", "Using `setCommand 'nop'` is not necessary.", Warning,
-              HighPriority)
-      return
-
-    if not execHook(options, false):
-      return
-
-    if execResult.hasTaskRequestedCommand():
-      var newOptions = initOptions()
-      newOptions.config = options.config
-      newOptions.nimbleData = options.nimbleData
-      parseCommand(execResult.command, newOptions)
-      for arg in execResult.arguments:
-        parseArgument(arg, newOptions)
-      for flag, vals in execResult.flags:
-        for val in vals:
-          parseFlag(flag, val, newOptions)
-      doAction(newOptions)
+    var execResult: ExecutionResult[void]
+    if execCustom(options, execResult, failFast=not isPreDefined):
+      if execResult.hasTaskRequestedCommand():
+        doAction(execResult.getOptionsForCommand(options))
+    else:
+      # If there is no task defined for the `test` task, we run the pre-defined
+      # fallback logic.
+      if isPreDefined:
+        test(options)
+        # Run the post hook for `test` in case it exists.
+        discard execHook(options, false)
 
   if options.action.typ != actionCustom:
     discard execHook(options, false)

src/nimblepkg/nimscriptexecutor.nim

@@ -0,0 +1,70 @@
+# Copyright (C) Dominik Picheta. All rights reserved.
+# BSD License. Look at license.txt for more info.
+
+import os, tables, strutils, sets
+
+import packageparser, common, packageinfo, options, nimscriptsupport, cli
+
+proc execHook*(options: Options, before: bool): bool =
+  ## Returns whether to continue.
+  result = true
+  var nimbleFile = ""
+  try:
+    nimbleFile = findNimbleFile(getCurrentDir(), true)
+  except NimbleError: return true
+  # PackageInfos are cached so we can read them as many times as we want.
+  let pkgInfo = getPkgInfoFromFile(nimbleFile, options)
+  let actionName =
+    if options.action.typ == actionCustom: options.action.command
+    else: ($options.action.typ)[6 .. ^1]
+  let hookExists =
+    if before: actionName.normalize in pkgInfo.preHooks
+    else: actionName.normalize in pkgInfo.postHooks
+  if pkgInfo.isNimScript and hookExists:
+    let res = execHook(nimbleFile, actionName, before, options)
+    if res.success:
+      result = res.retVal
+
+proc execCustom*(options: Options,
+                 execResult: var ExecutionResult[void],
+                 failFast = true): bool =
+  ## Executes the custom command using the nimscript backend.
+  ##
+  ## If failFast is true then exceptions will be raised when something is wrong.
+  ## Otherwise this function will just return false.
+
+  # Custom command. Attempt to call a NimScript task.
+  let nimbleFile = findNimbleFile(getCurrentDir(), true)
+  if not nimbleFile.isNimScript(options) and failFast:
+    writeHelp()
+
+  execResult = execTask(nimbleFile, options.action.command, options)
+  if not execResult.success:
+    if not failFast:
+      return
+    raiseNimbleError(msg = "Could not find task $1 in $2" %
+                           [options.action.command, nimbleFile],
+                     hint = "Run `nimble --help` and/or `nimble tasks` for" &
+                            " a list of possible commands.")
+
+  if execResult.command.normalize == "nop":
+    display("Warning:", "Using `setCommand 'nop'` is not necessary.", Warning,
+            HighPriority)
+    return
+
+  if not execHook(options, false):
+    return
+
+  return true
+
+proc getOptionsForCommand*(execResult: ExecutionResult,
+                           options: Options): Options =
+  ## Creates an Options object for the requested command.
+  var newOptions = options.briefClone()
+  parseCommand(execResult.command, newOptions)
+  for arg in execResult.arguments:
+    parseArgument(arg, newOptions)
+  for flag, vals in execResult.flags:
+    for val in vals:
+      parseFlag(flag, val, newOptions)
+  return newOptions

src/nimblepkg/nimscriptsupport.nim

@@ -454,6 +454,6 @@ proc listTasks*(scriptName: string, options: Options) =
   setNimScriptCommand("help")
 
   discard execScript(scriptName, nil, options)
-  # TODO: Make the 'task' template generate explicit data structure containing
-  # all the task names + descriptions.
-  cleanup()
+  # TODO (#402): Make the 'task' template generate explicit data structure
+  # containing all the task names + descriptions.
+  cleanup()

src/nimblepkg/options.nim

@@ -27,7 +27,7 @@ type
     actionNil, actionRefresh, actionInit, actionDump, actionPublish,
     actionInstall, actionSearch,
     actionList, actionBuild, actionPath, actionUninstall, actionCompile,
-    actionDoc, actionCustom, actionTasks, actionDevelop, actionTest
+    actionDoc, actionCustom, actionTasks, actionDevelop
 
   Action* = object
     case typ*: ActionType
@@ -41,7 +41,7 @@ type
       search*: seq[string] # Search string.
     of actionInit, actionDump:
       projName*: string
-    of actionCompile, actionDoc, actionBuild, actionTest:
+    of actionCompile, actionDoc, actionBuild:
       file*: string
       backend*: string
       compileOptions*: seq[string]
@@ -126,8 +126,6 @@ proc parseActionType*(action: string): ActionType =
     result = actionPath
   of "build":
     result = actionBuild
-  of "test":
-    result = actionTest
   of "c", "compile", "js", "cpp", "cc":
     result = actionCompile
   of "doc", "doc2":
@@ -160,7 +158,7 @@ proc initAction*(options: var Options, key: string) =
   case options.action.typ
   of actionInstall, actionPath, actionDevelop, actionUninstall:
     options.action.packages = @[]
-  of actionCompile, actionDoc, actionBuild, actionTest:
+  of actionCompile, actionDoc, actionBuild:
     options.action.compileOptions = @[]
     options.action.file = ""
     if keyNorm == "c" or keyNorm == "compile": options.action.backend = ""
@@ -242,7 +240,7 @@ proc parseArgument*(key: string, result: var Options) =
     result.action.projName = key
   of actionCompile, actionDoc:
     result.action.file = key
-  of actionList, actionBuild, actionTest, actionPublish:
+  of actionList, actionBuild, actionPublish:
     result.showHelp = true
   of actionCustom:
     result.action.arguments.add(key)
@@ -281,7 +279,7 @@ proc parseFlag*(flag, val: string, result: var Options, kind = cmdLongOption) =
         result.depsOnly = true
       else:
         wasFlagHandled = false
-    of actionCompile, actionDoc, actionBuild, actionTest:
+    of actionCompile, actionDoc, actionBuild:
       let prefix = if kind == cmdShortOption: "-" else: "--"
       if val == "":
         result.action.compileOptions.add(prefix & flag)
@@ -370,3 +368,10 @@ proc getProxy*(options: Options): Proxy =
     return newProxy($parsed, auth)
   else:
     return nil
+
+proc briefClone*(options: Options): Options =
+  ## Clones the few important fields and creates a new Options object.
+  var newOptions = initOptions()
+  newOptions.config = options.config
+  newOptions.nimbleData = options.nimbleData
+  return newOptions

src/nimblepkg/packageparser.nim

@@ -268,6 +268,7 @@ proc readPackageInfo(nf: NimbleFile, options: Options,
   assert fileExists(nf)
 
   # Check the cache.
+  echo(nf, options.pkgInfoCache.hasKey(nf))
   if options.pkgInfoCache.hasKey(nf):
     return options.pkgInfoCache[nf]
 

tests/testCommand/testOverride/myTester.nim

@@ -0,0 +1 @@
+echo("overriden")

tests/testCommand/testOverride/pkga.nimble

@@ -0,0 +1,9 @@
+version       = "0.1.0"
+author        = "John Doe"
+description   = "Nimble Test"
+license       = "BSD"
+
+skipFiles = @["myTester.nim"]
+
+task test, "Custom tester":
+  exec "nim c -r myTester.nim"

tests/tester.nim

@@ -557,22 +557,29 @@ suite "develop feature":
     createDir(cloneDir)
     cd cloneDir:
       let url = "https://github.com/nimble-test/packagea.git"
-      let (output, exitCode) = execNimble("develop", "-y", url)
+      let (_, exitCode) = execNimble("develop", "-y", url)
       check exitCode == QuitSuccess
 
-test "Runs passing unit tests":
-  cd "testsPass":
-    let (outp, exitCode) = execNimble("test")
-    check: exitCode == QuitSuccess
-    check: outp.processOutput.inLines("First test")
-    check: outp.processOutput.inLines("Second test")
-    check: outp.processOutput.inLines("Third test")
-    check: outp.processOutput.inLines("Executing my func")
+suite "test command":
+  test "Runs passing unit tests":
+    cd "testCommand/testsPass":
+      let (outp, exitCode) = execNimble("test")
+      check exitCode == QuitSuccess
+      check outp.processOutput.inLines("First test")
+      check outp.processOutput.inLines("Second test")
+      check outp.processOutput.inLines("Third test")
+      check outp.processOutput.inLines("Executing my func")
 
-test "Runs failing unit tests":
-  cd "testsFail":
-    let (outp, exitCode) = execNimble("test")
-    check: exitCode == QuitFailure
-    check: outp.processOutput.inLines("First test")
-    check: outp.processOutput.inLines("Failing Second test")
-    check: not outp.processOutput.inLines("Third test")
+  test "Runs failing unit tests":
+    cd "testCommand/testsFail":
+      let (outp, exitCode) = execNimble("test")
+      check exitCode == QuitFailure
+      check outp.processOutput.inLines("First test")
+      check outp.processOutput.inLines("Failing Second test")
+      check(not outp.processOutput.inLines("Third test"))
+
+  test "test command can be overriden":
+    cd "testCommand/testOverride":
+      let (outp, exitCode) = execNimble("test")
+      check exitCode == QuitSuccess
+      check outp.processOutput.inLines("overriden")