fix #1994 gorge, staticExec now raise Assert on exitCode !=0; simplified implementation

changelog.md

@@ -67,7 +67,9 @@
 - two poorly documented and not used modules (`subexes`, `scgi`) were moved to
   graveyard (they are available as Nimble packages)
 
-
+- `gorge`, `staticExec` now raise AssertError if exitCode is not 0 instead of
+  silently ignoring errors. See https://github.com/nim-lang/Nim/issues/1994#issuecomment-327904129
+  Use `gorgeEx` to get `tuple[output: string, exitCode: int]`
 
 #### Breaking changes in the compiler
 

compiler/ast.nim

@@ -580,7 +580,7 @@ type
     mDefined, mDefinedInScope, mCompiles, mArrGet, mArrPut, mAsgn,
     mLow, mHigh, mSizeOf, mAlignOf, mOffsetOf, mTypeTrait,
     mIs, mOf, mAddr, mType, mTypeOf,
-    mRoof, mPlugin, mEcho, mShallowCopy, mSlurp, mStaticExec, mStatic,
+    mRoof, mPlugin, mEcho, mShallowCopy, mSlurp, mStatic,
     mParseExprToAst, mParseStmtToAst, mExpandToAst, mQuoteAst,
     mUnaryLt, mInc, mDec, mOrd,
     mNew, mNewFinalize, mNewSeq, mNewSeqOfCap,

compiler/jsgen.nim

@@ -1953,7 +1953,7 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) =
   of mOf: genOf(p, n, r)
   of mReset: genReset(p, n)
   of mEcho: genEcho(p, n, r)
-  of mNLen..mNError, mSlurp, mStaticExec:
+  of mNLen..mNError, mSlurp:
     localError(p.config, n.info, errXMustBeCompileTime % n.sons[0].sym.name.s)
   of mCopyStr:
     binaryExpr(p, n, r, "", "($1.slice($2))")

compiler/vm.nim

@@ -1422,18 +1422,6 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       createStr regs[ra]
       regs[ra].node.strVal = opSlurp(regs[rb].node.strVal, c.debug[pc],
                                      c.module, c.config)
-    of opcGorge:
-      when defined(nimcore):
-        decodeBC(rkNode)
-        inc pc
-        let rd = c.code[pc].regA
-
-        createStr regs[ra]
-        regs[ra].node.strVal = opGorge(regs[rb].node.strVal,
-                                      regs[rc].node.strVal, regs[rd].node.strVal,
-                                      c.debug[pc], c.config)[0]
-      else:
-        globalError(c.config, c.debug[pc], "VM is not built with 'gorge' support")
     of opcNError, opcNWarning, opcNHint:
       decodeB(rkNode)
       let a = regs[ra].node

compiler/vmdef.nim

@@ -100,7 +100,6 @@ type
     opcNctPut, opcNctLen, opcNctGet, opcNctHasNext, opcNctNext,
 
     opcSlurp,
-    opcGorge,
     opcParseExprToAst,
     opcParseStmtToAst,
     opcQueryErrorFlag,

compiler/vmgen.nim

@@ -665,6 +665,8 @@ proc genBinaryABC(c: PCtx; n: PNode; dest: var TDest; opc: TOpcode) =
   c.freeTemp(tmp2)
 
 proc genBinaryABCD(c: PCtx; n: PNode; dest: var TDest; opc: TOpcode) =
+  # was used by `of mStaticExec: genBinaryABCD(c, n, dest, opcGorge)`
+  # which was removed; keeping this in case of future use
   let
     tmp = c.genx(n.sons[1])
     tmp2 = c.genx(n.sons[2])
@@ -1156,7 +1158,6 @@ proc genMagic(c: PCtx; n: PNode; dest: var TDest; m: TMagic) =
     c.gABC(n, opcTypeTrait, dest, tmp)
     c.freeTemp(tmp)
   of mSlurp: genUnaryABC(c, n, dest, opcSlurp)
-  of mStaticExec: genBinaryABCD(c, n, dest, opcGorge)
   of mNLen: genUnaryABI(c, n, dest, opcLenSeq, nimNodeFlag)
   of mGetImpl: genUnaryABC(c, n, dest, opcGetImpl)
   of mGetImplTransf: genUnaryABC(c, n, dest, opcGetImplTransf)

lib/system.nim

@@ -3849,38 +3849,6 @@ proc staticRead*(filename: string): string {.magic: "Slurp".}
   ##
   ## `slurp <#slurp>`_ is an alias for ``staticRead``.
 
-proc gorge*(command: string, input = "", cache = ""): string {.
-  magic: "StaticExec".} = discard
-  ## This is an alias for `staticExec <#staticExec>`_.
-
-proc staticExec*(command: string, input = "", cache = ""): string {.
-  magic: "StaticExec".} = discard
-  ## Executes an external process at compile-time.
-  ## if `input` is not an empty string, it will be passed as a standard input
-  ## to the executed program.
-  ##
-  ## .. code-block:: nim
-  ##     const buildInfo = "Revision " & staticExec("git rev-parse HEAD") &
-  ##                       "\nCompiled on " & staticExec("uname -v")
-  ##
-  ## `gorge <#gorge>`_ is an alias for ``staticExec``. Note that you can use
-  ## this proc inside a pragma like `passC <nimc.html#passc-pragma>`_ or `passL
-  ## <nimc.html#passl-pragma>`_.
-  ##
-  ## If ``cache`` is not empty, the results of ``staticExec`` are cached within
-  ## the ``nimcache`` directory. Use ``--forceBuild`` to get rid of this caching
-  ## behaviour then. ``command & input & cache`` (the concatenated string) is
-  ## used to determine whether the entry in the cache is still valid. You can
-  ## use versioning information for ``cache``:
-  ##
-  ## .. code-block:: nim
-  ##     const stateMachine = staticExec("dfaoptimizer", "input", "0.8.0")
-
-proc gorgeEx*(command: string, input = "", cache = ""): tuple[output: string,
-                                                              exitCode: int] =
-  ## Same as `gorge` but also returns the precious exit code.
-  discard
-
 proc `+=`*[T: SomeOrdinal|uint|uint64](x: var T, y: T) {.
   magic: "Inc", noSideEffect.}
   ## Increments an ordinal
@@ -4005,6 +3973,48 @@ template doAssert*(cond: untyped, msg = "") =
   const expr = astToStr(cond)
   assertImpl(cond, msg, expr, true)
 
+
+proc gorgeEx*(command: string, input = "", cache = ""):
+  tuple[output: string, exitCode: int] {.compileTime.} =
+  ## Executes an external process at compile-time.
+  ## if `input` is not an empty string, it will be passed as a standard input
+  ## to the executed program.
+  ##
+  ## .. code-block:: nim
+  ##     const buildInfo = "Revision " & gorgeEx("git rev-parse HEAD") &
+  ##                       "\nCompiled on " & gorgeEx("uname -v")
+  ##
+  ## If ``cache`` is not empty, the results of ``gorgeEx`` are cached within
+  ## the ``nimcache`` directory. Use ``--forceBuild`` to get rid of this caching
+  ## behaviour then. ``command & input & cache`` (the concatenated string) is
+  ## used to determine whether the entry in the cache is still valid. You can
+  ## use versioning information for ``cache``:
+  ##
+  ## .. code-block:: nim
+  ##     const (stateMachine, exitCode) = gorgeEx("dfaoptimizer", "input", "0.8.0")
+  ##
+  runnableExamples:
+    import os, strutils
+    const ret = gorgeEx(getCurrentCompilerExe() & " --version")
+    doAssert ret.exitCode == 0
+    doAssert ret.output.startsWith("Nim Compiler Version")
+  discard
+
+proc gorge*(command: string, input = "", cache = ""): string {.compileTime.} =
+  ## Convenience wrapper around `gorgeEx` that returns the output after
+  ## checking `exitCode`.
+  ## Note that you can use this proc inside a pragma like
+  ## `passC <nimc.html#passc-pragma>`_ or
+  ## `passL <nimc.html#passl-pragma>`_,
+  ## eg: `{.passL: gorge("pkg-config --libs sdl").}`
+  let ret = gorgeEx(command, input, cache)
+  doAssert ret.exitCode == 0, "gorgeEx failed: cmd: `" & command & "` input: `" & input & "` cache: `" & cache & "`"
+  result = ret.output
+
+proc staticExec*(command: string, input = "", cache = ""): string {.compileTime.} =
+  ## alias for `gorge`
+  result = gorge(command, input, cache)
+
 iterator items*[T](a: seq[T]): T {.inline.} =
   ## iterates over each item of `a`.
   var i = 0

tests/vm/tvmops.nim

@@ -5,6 +5,8 @@ import os
 import math
 import strutils
 
+const unexistant = "D20190116T211842"
+
 template forceConst(a: untyped): untyped =
   ## Force evaluation at CT, useful for example here:
   ## `callFoo(forceConst(getBar1()), getBar2())`
@@ -44,4 +46,10 @@ block:
   # Check against bugs like #9176
   doAssert getCurrentCompilerExe() == forceConst(getCurrentCompilerExe())
   if false: #pending #9176
-    doAssert gorgeEx("unexistant") == forceConst(gorgeEx("unexistant"))
+    doAssert gorgeEx(unexistant) == forceConst(gorgeEx(unexistant))
+  echo gorge("pwd")
+
+block: # issue #1994
+  static:
+    doAssertRaises(AssertionError):
+      let a = gorge(unexistant)