fix nim-lang#10273 execShellCmd now returns nonzero when child exits …

…with signal

.gitignore

@@ -32,7 +32,7 @@ doc/*.html
 doc/*.pdf
 doc/*.idx
 /web/upload
-build/*
+/build/*
 bin/*
 
 # iOS specific wildcards.

lib/pure/os.nim

@@ -1287,6 +1287,14 @@ proc moveFile*(source, dest: string) {.rtl, extern: "nos$1",
         discard tryRemoveFile(dest)
         raise
 
+proc exitStatusLikeShell*(status: cint): cint {.noNimScript.} =
+  ## converts exit code from `c_system` into a shell exit code
+  if WIFSIGNALED(status):
+    # like the shell!
+    128 + WTERMSIG(status)
+  else:
+    WEXITSTATUS(status)
+
 proc execShellCmd*(command: string): int {.rtl, extern: "nos$1",
   tags: [ExecIOEffect], noNimScript.} =
   ## Executes a `shell command`:idx:.
@@ -1297,10 +1305,8 @@ proc execShellCmd*(command: string): int {.rtl, extern: "nos$1",
   ## the process has finished. To execute a program without having a
   ## shell involved, use the `execProcess` proc of the `osproc`
   ## module.
-  when defined(macosx):
-    result = c_system(command) shr 8
-  elif defined(posix):
-    result = WEXITSTATUS(c_system(command))
+  when defined(posix):
+    result = exitStatusLikeShell(c_system(command))
   else:
     result = c_system(command)
 

lib/pure/osproc.nim

@@ -724,13 +724,6 @@ elif not defined(useNimRtl):
   proc isExitStatus(status: cint): bool =
     WIFEXITED(status) or WIFSIGNALED(status)
 
-  proc exitStatusLikeShell(status: cint): cint =
-    if WIFSIGNALED(status):
-      # like the shell!
-      128 + WTERMSIG(status)
-    else:
-      WEXITSTATUS(status)
-
   proc envToCStringArray(t: StringTableRef): cstringArray =
     result = cast[cstringArray](alloc0((t.len + 1) * sizeof(cstring)))
     var i = 0

lib/std/special_paths.nim

@@ -0,0 +1,29 @@
+#[
+todo: move findNimStdLibCompileTime, findNimStdLib here
+]#
+
+import os
+
+# Note: all the const paths defined here are known at compile time and valid
+# so long Nim repo isn't relocated after compilation.
+
+const sourcePath = currentSourcePath()
+  # robust way to derive other paths here
+  # We don't depend on PATH so this is robust to having multiple nim
+  # binaries
+
+const nimRootDir* = sourcePath.parentDir.parentDir.parentDir
+  ## root of Nim repo
+
+const stdlibDir* = nimRootDir / "lib"
+  # todo: make nimeval.findNimStdLibCompileTime use this
+
+const systemPath* = stdlibDir / "system.nim"
+
+const buildDir* = nimRootDir / "build"
+  ## refs #10268: all testament generated files should go here to avoid
+  ## polluting .gitignore
+
+static:
+  # sanity check
+  doAssert fileExists(systemPath)

tests/stdlib/t10231.nim

@@ -6,6 +6,8 @@ discard """
 
 import os
 
+# consider moving this inside tosproc (taking care that it's for cpp mode)
+
 if paramCount() == 0:
   # main process
   doAssert execShellCmd(getAppFilename().quoteShell & " test") == 1

tests/stdlib/tos.nim

@@ -263,3 +263,5 @@ block splitFile:
   doAssert splitFile("abc/.") == ("abc", ".", "")
   doAssert splitFile("..") == ("", "..", "")
   doAssert splitFile("a/..") == ("a", "..", "")
+
+# execShellCmd is tested in tosproc

tests/stdlib/tosproc.nim

@@ -3,21 +3,93 @@ discard """
 """
 # test the osproc module
 
-import os, osproc
-
-block execProcessTest:
-  let dir = parentDir(currentSourcePath())
-  let (outp, err) = execCmdEx("nim c " & quoteShell(dir / "osproctest.nim"))
-  doAssert err == 0
-  let exePath = dir / addFileExt("osproctest", ExeExt)
-  let outStr1 = execProcess(exePath, workingDir=dir, args=["foo", "b A r"], options={})
-  doAssert outStr1 == dir & "\nfoo\nb A r\n"
-
-  const testDir = "t e st"
-  createDir(testDir)
-  doAssert dirExists(testDir)
-  let outStr2 = execProcess(exePath, workingDir=testDir, args=["x yz"], options={})
-  doAssert outStr2 == absolutePath(testDir) & "\nx yz\n"
-
-  removeDir(testDir)
-  removeFile(exePath)
+import compiler/unittest_light
+import std/special_paths
+
+when defined(case_testfile): # compiled test file for child process
+  from posix import exitnow
+  proc c_exit2(code: c_int): void {.importc: "_exit", header: "<unistd.h>".}
+  import os
+  var a = 0
+  proc fun(b = 0) =
+    a.inc
+    # if a mod 100000 == 0:
+    if a mod 10000000 == 0:
+      echo a
+      discard
+    fun(b+1)
+
+  proc main() =
+    let args = commandLineParams()
+    # echo "ok1"
+    echo (msg: "child binary", pid: getCurrentProcessId())
+    let arg = args[0]
+    echo (arg: arg)
+    case arg
+    of "exit_0":
+      if true: quit(0)
+    of "exitnow_139":
+      if true: exitnow(139)
+    of "c_exit2_139":
+      if true: c_exit2(139)
+    of "quit_139":
+      #[
+      139-128=11=SIGSEGV
+      ]#
+      if true: quit(139)
+    of "exit_recursion": # stack overflow by infinite recursion
+      fun()
+      echo a
+    of "exit_array": # bad array access
+      echo args[1]
+  main()
+
+else:
+
+  import os, osproc, strutils, posix
+
+  block execShellCmdTest:
+    ## first, compile child program
+    const nim = getCurrentCompilerExe()
+    const sourcePath = currentSourcePath()
+    let output = buildDir / "D20190111T024543".addFileExt(ExeExt)
+    let cmd = "$# c -o:$# -d:release -d:case_testfile $#" % [nim, output,
+        sourcePath]
+    # we're testing `execShellCmd` so don't rely on it to compile test file
+    # note: this should be exported in posix.nim
+    proc c_system(cmd: cstring): cint {.importc: "system",
+      header: "<stdlib.h>".}
+    assertEquals c_system(cmd), 0
+
+    ## use it
+    template runTest(arg: string, expected: int) =
+      echo (arg2: arg, expected2: expected)
+      assertEquals execShellCmd(output & " " & arg), expected
+
+    runTest("exit_0", 0)
+    runTest("exitnow_139", 139)
+    runTest("c_exit2_139", 139)
+    runTest("quit_139", 139)
+    runTest("exit_array", 1)
+    runTest("exit_recursion", SIGSEGV.int + 128)
+
+    assertEquals exitStatusLikeShell(SIGSEGV), SIGSEGV + 128.cint
+
+  block execProcessTest:
+    let dir = parentDir(currentSourcePath())
+    let (outp, err) = execCmdEx("nim c " & quoteShell(dir / "osproctest.nim"))
+    doAssert err == 0
+    let exePath = dir / addFileExt("osproctest", ExeExt)
+    let outStr1 = execProcess(exePath, workingDir = dir, args = ["foo",
+        "b A r"], options = {})
+    doAssert outStr1 == dir & "\nfoo\nb A r\n"
+
+    const testDir = "t e st"
+    createDir(testDir)
+    doAssert dirExists(testDir)
+    let outStr2 = execProcess(exePath, workingDir = testDir, args = ["x yz"],
+        options = {})
+    doAssert outStr2 == absolutePath(testDir) & "\nx yz\n"
+
+    removeDir(testDir)
+    removeFile(exePath)