fix #62 (toast now addressed relatively); macros=>procs + other fixes (#70)

* fixes https://github.com/genotrance/nimterop/issues/62
*  toast is now automatically built on demand, and doesn't depend on PATH
- [ ] todo: need to track its deps properly to trigger rebuilding it; using same algo as shown here https://github.com/genotrance/nimterop/issues/69 but using nim deps instead of c deps
* many other fixes
  * trying to follow https://github.com/timotheecour/vitanim/wiki/Nim-best-practices

15 files changed, 173 insertions, 108 deletions

diff --git a/.gitignore b/.gitignore
index df228b3..61390ef 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,7 +3,12 @@
 !/**/
 !*.*
 
-# TODO: all generated stuff should go inside `build/`
+
+# Notes:
+# all generated files should go inside `build/`
+# use absolute paths to refer to a path assumed to be at a fixed level in the hierarchy
+/build
+
 
 nimcache
 
@@ -12,6 +17,3 @@ nimcache
 *.exe
 *.swp
 
-# Note: use absolute paths to refer to a path assumed to be at a fixed dir level
-
-inc
diff --git a/build/.gitignore b/build/.gitignore
deleted file mode 100644
index f72369d..0000000
--- a/build/.gitignore
+++ /dev/null
@@ -1,4 +0,0 @@
-# everything generated should go in this dir
-*
-!/.gitignore
-
diff --git a/config.nims b/config.nims
index 32bbe5e..5aac628 100644
--- a/config.nims
+++ b/config.nims
@@ -1,3 +1,8 @@
+#[
+see D20190127T231316 workaround for fact that toast needs to build scanner.cc, which would otherwise result in link erros such as:
+"std::terminate()", referenced from:
+      ___clang_call_terminate in scanner.cc.o
+]#
 when defined(MacOSX):
   switch("clang.linkerexe", "g++")
 else:
diff --git a/nimterop.nimble b/nimterop.nimble
index 77853a5..ec034f0 100644
--- a/nimterop.nimble
+++ b/nimterop.nimble
@@ -5,13 +5,17 @@ author      = "genotrance"
 description = "C/C++ interop for Nim"
 license     = "MIT"
 
-bin = @["toast"]
+# this gives Warning: Binary 'nimterop/toast' was already installed from source directory
+# when running `nimble install --verbose -y`
+bin = @["nimterop/toast"]
 installDirs = @["nimterop"]
+installFiles = @["config.nims"]
 
 # Dependencies
-
 requires "nim >= 0.19.2", "regex >= 0.10.0", "cligen >= 0.9.17"
 
+import strformat
+
 proc execCmd(cmd: string) =
   echo "execCmd:" & cmd
   exec cmd
@@ -20,26 +24,34 @@ proc tsoloud() =
   execCmd "nim c -r tests/tsoloud.nim"
   execCmd "nim cpp -r tests/tsoloud.nim"
 
-proc testall() =
+proc buildToast(options: string) =
+  execCmd(&"nim c {options} nimterop/toast.nim")
+
+task rebuildToast, "rebuild toast":
+  # If need to manually rebuild (automatically built on 1st need)
+  buildToast("-d:release")
+
+proc testAll() =
   execCmd "nim c -r tests/tnimterop_c.nim"
   execCmd "nim cpp -r tests/tnimterop_c.nim"
   execCmd "nim cpp -r tests/tnimterop_cpp.nim"
-  when defined(windows):
+
+  ## platform specific tests
+  when defined(Windows):
     execCmd "nim c -r tests/tmath.nim"
     execCmd "nim cpp -r tests/tmath.nim"
-  when not defined(OSX):
-    when defined(Windows):
-      tsoloud()
-    else:
-      if not existsEnv("TRAVIS"):
-        tsoloud()
+    tsoloud()
+  elif defined(osx):
+    discard
+  elif existsEnv("TRAVIS"):
+    discard
+  else:
+    tsoloud()
 
 task test, "Test":
-  execCmd "nim c toast"
-  testAll()
-
-  execCmd "nim c -d:release toast"
-  testAll()
+  for options in ["", "-d:release"]:
+    buildToast(options)
+    testAll()
 
 task docs, "Generate docs":
   # Uses: pip install ghp-import
diff --git a/nimterop/cimport.nim b/nimterop/cimport.nim
index 7e9cd65..7fcd14d 100644
--- a/nimterop/cimport.nim
+++ b/nimterop/cimport.nim
@@ -2,9 +2,7 @@ import hashes, macros, os, strformat, strutils
 
 const CIMPORT {.used.} = 1
 
-include "."/globals
-
-import "."/types
+import "." / [globals,types,paths]
 export types
 
 proc interpPath(dir: string): string=
@@ -88,7 +86,10 @@ proc getToast(fullpath: string, recurse: bool = false): string =
     ret = 0
     cmd = when defined(Windows): "cmd /c " else: ""
 
-  cmd &= "toast --pnim --preprocess"
+  let toastExe = toastExePath()
+  doAssert fileExists(toastExe), "toast not compiled: " & toastExe.quoteShell &
+    " make sure 'nimble build' or 'nimble install' built it"
+  cmd &= &"{toastExe} --pnim --preprocess"
 
   if recurse:
     cmd.add " --recurse"
@@ -107,6 +108,7 @@ proc getToast(fullpath: string, recurse: bool = false): string =
 
   cmd.add &" {fullpath.quoteShell}"
   echo cmd
+  # see https://github.com/genotrance/nimterop/issues/69
   (result, ret) = gorgeEx(cmd, cache=getCacheValue(fullpath))
   doAssert ret == 0, getToastError(result)
 
@@ -264,7 +266,7 @@ macro cDefine*(name: static string, val: static string = ""): untyped =
 
   var str = name
   if val.nBl:
-    str &= &"=\"{val}\""
+    str &= &"={val.quoteShell}"
 
   if str notin gStateCT.defines:
     gStateCT.defines.add(str)
@@ -302,7 +304,7 @@ macro cIncludeDir*(dir: static string): untyped =
 
   let
     fullpath = findPath(dir)
-    str = &"-I\"{fullpath}\""
+    str = &"-I{fullpath.quoteShell}"
 
   if fullpath notin gStateCT.includeDirs:
     gStateCT.includeDirs.add(fullpath)
diff --git a/nimterop/git.nim b/nimterop/git.nim
index 7961385..edd2156 100644
--- a/nimterop/git.nim
+++ b/nimterop/git.nim
@@ -1,5 +1,7 @@
 import macros, os, osproc, regex, strformat, strutils
 
+import "."/paths
+
 proc execAction*(cmd: string, nostderr=false): string =
   var
     ccmd = ""
@@ -22,7 +24,7 @@ proc execAction*(cmd: string, nostderr=false): string =
     echo result
     quit(1)
 
-macro extractZip*(zipfile, outdir: static string): untyped =
+proc extractZip*(zipfile, outdir: string) =
   var cmd = "unzip -o $#"
   if defined(Windows):
     cmd = "powershell -nologo -noprofile -command \"& { Add-Type -A " &
@@ -30,68 +32,73 @@ macro extractZip*(zipfile, outdir: static string): untyped =
           "[IO.Compression.ZipFile]::ExtractToDirectory('$#', '.'); }\""
 
   echo "Extracting " & zipfile
-  discard execAction(&"cd \"{getProjectPath()/outdir}\" && " & cmd % zipfile)
+  discard execAction(&"cd {outdir.quoteShell} && {cmd % zipfile}")
 
-macro downloadUrl*(url, outdir: static string): untyped =
+proc downloadUrl*(url, outdir: string) =
   let
     file = url.extractFilename()
     ext = file.splitFile().ext.toLowerAscii()
 
-  var cmd = "curl $# -o $#"
-  if defined(Windows):
-    cmd = "powershell wget $# -OutFile $#"
-
-  if not (ext == ".zip" and fileExists(getProjectPath()/outdir/file)):
+  if not (ext == ".zip" and fileExists(outdir/file)):
     echo "Downloading " & file
-    discard execAction(cmd % [url, getProjectPath()/outdir/file])
+    var cmd = if defined(Windows):
+      "powershell wget $# -OutFile $#"
+    else:
+      "curl $# -o $#"
+    discard execAction(cmd % [url, outdir/file])
 
   if ext == ".zip":
-    discard quote do:
-      extractZip(`file`, `outdir`)
+    extractZip(file, outdir)
 
-macro gitReset*(outdir: static string): untyped =
+proc gitReset*(outdir: string) =
   echo "Resetting " & outdir
 
-  let cmd = &"cd \"{getProjectPath()/outdir}\" && git reset --hard"
+  let cmd = &"cd {outdir.quoteShell} && git reset --hard"
   while execAction(cmd).contains("Permission denied"):
     sleep(1000)
     echo "  Retrying ..."
 
-macro gitCheckout*(file, outdir: static string): untyped =
-  echo "Resetting " & file
+proc relativePathNaive*(file, base: string): string =
+  ## naive version of `os.relativePath` ; remove after nim >= 0.19.9
+  runnableExamples:
+    doAssert "/foo/bar/baz/log.txt".relativePathNaive("/foo/bar") == "baz/log.txt"
+  var base = base
+  if not base.endsWith "/": base.add "/"
+  doAssert file.startsWith base
+  result = file[base.len .. ^1]
 
-  let cmd = &"cd \"{getProjectPath()/outdir}\" && git checkout $#" % file.replace(outdir & "/", "")
+proc gitCheckout*(file, outdir: string) =
+  echo "Resetting " & file
+  let file2 = file.relativePathNaive outdir
+  let cmd = &"cd {outdir.quoteShell} && git checkout {file2.quoteShell}"
   while execAction(cmd).contains("Permission denied"):
     sleep(500)
     echo "  Retrying ..."
 
-macro gitPull*(url: static string, outdirN: static string = "", plist: static string = "", checkout: static string = ""): untyped =
-  let
-    outdir = if outdirN.isAbsolute(): outdirN else: getProjectPath()/outdirN
-
+proc gitPull*(url: string, outdir = "", plist = "", checkout = "") =
   if dirExists(outdir/".git"):
-    discard quote do:
-      gitReset(`outdir`)
+    gitReset(outdir)
     return
-  else:
-    let
-      flag = when not defined(Windows): "-p" else: ""
-    echo execAction(&"mkdir {flag} \"{outdir}\"")
+  let
+    outdir2 = outdir.quoteShell
+    flag = when not defined(Windows): "-p" else: ""
+  echo execAction(&"mkdir {flag} {outdir2}")
 
   echo "Setting up Git repo: " & url
-  discard execAction(&"cd \"{outdir}\" && git init .")
-  discard execAction(&"cd \"{outdir}\" && git remote add origin " & url)
+  discard execAction(&"cd {outdir2} && git init .")
+  discard execAction(&"cd {outdir2} && git remote add origin {url}")
 
   if plist.len != 0:
-    let sparsefile = &"{outdir}/.git/info/sparse-checkout"
+    # TODO: document this, it's not clear
+    let sparsefile = outdir / ".git/info/sparse-checkout"
 
-    discard execAction(&"cd \"{outdir}\" && git config core.sparsecheckout true")
+    discard execAction(&"cd {outdir2} && git config core.sparsecheckout true")
     writeFile(sparsefile, plist)
 
   if checkout.len != 0:
     echo "Checking out " & checkout
-    discard execAction(&"cd \"{outdir}\" && git pull --tags origin master")
-    discard execAction(&"cd \"{outdir}\" && git checkout {checkout}")
+    discard execAction(&"cd {outdir2} && git pull --tags origin master")
+    discard execAction(&"cd {outdir2} && git checkout {checkout}")
   else:
     echo "Pulling repository"
-    discard execAction(&"cd \"{outdir}\" && git pull --depth=1 origin master")
+    discard execAction(&"cd {outdir2} && git pull --depth=1 origin master")
diff --git a/nimterop/globals.nim b/nimterop/globals.nim
index b959958..bf9a0ef 100644
--- a/nimterop/globals.nim
+++ b/nimterop/globals.nim
@@ -68,7 +68,7 @@ type
     data*: seq[tuple[name, val: string]]
 
 var
-  gStateCT {.compiletime, used.}: State
+  gStateCT* {.compiletime, used.}: State
   gStateRT {.used.}: State
 
 template nBl(s: typed): untyped {.used.} =
diff --git a/nimterop/paths.nim b/nimterop/paths.nim
new file mode 100644
index 0000000..64879a4
--- /dev/null
+++ b/nimterop/paths.nim
@@ -0,0 +1,17 @@
+import os
+
+proc nimteropRoot*(): string =
+  currentSourcePath.parentDir.parentDir
+
+proc nimteropBuildDir*(): string =
+  ## all nimterop generated files go under here (gitignored)
+  nimteropRoot() / "build"
+
+proc nimteropSrcDir*(): string =
+  nimteropRoot() / "nimterop"
+
+proc toastExePath*(): string =
+  nimteropSrcDir() / ("toast".addFileExt ExeExt)
+
+proc incDir*(): string =
+  nimteropBuildDir() / "inc"
diff --git a/nimterop/setup.nim b/nimterop/setup.nim
index 06f15d4..2c1c7c0 100644
--- a/nimterop/setup.nim
+++ b/nimterop/setup.nim
@@ -1,51 +1,50 @@
 import os, strutils
 
-import "."/git
-
-const sourcePath = currentSourcePath().split({'\\', '/'})[0..^3].join("/") & "/inc"
+import "."/[git,paths]
 
 proc treesitterSetup*() =
-  gitPull("https://github.com/tree-sitter/tree-sitter/", "inc/treesitter", """
+  gitPull("https://github.com/tree-sitter/tree-sitter/", incDir() / "treesitter", """
 include/*
 src/runtime/*
 """)
 
-  gitPull("https://github.com/JuliaStrings/utf8proc", "inc/utf8proc", """
+  gitPull("https://github.com/JuliaStrings/utf8proc", incDir() / "utf8proc", """
 *.c
 *.h
 """)
-
+  
+  # TODO: does this work on windows? if not use `os.unixToNativePath`
   let
-    stack = sourcePath & "/treesitter/src/runtime/stack.c"
+    stack = incDir() / "treesitter/src/runtime/stack.c"
 
   stack.writeFile(stack.readFile().replace("inline Stack", "Stack"))
 
 proc treesitterCSetup*() =
-  gitPull("https://github.com/tree-sitter/tree-sitter-c", "inc/treesitter_c", """
+  gitPull("https://github.com/tree-sitter/tree-sitter-c", incDir() / "treesitter_c", """
 src/*.h
 src/*.c
 src/*.cc
 """)
 
   let
-    headerc = sourcePath & "/treesitter_c/src/parser.h"
+    headerc = incDir() / "treesitter_c/src/parser.h"
 
   headerc.writeFile("""
-    typedef struct TSLanguage TSLanguage;
-    const TSLanguage *tree_sitter_c();
-  """)
+typedef struct TSLanguage TSLanguage;
+const TSLanguage *tree_sitter_c();
+""")
 
 proc treesitterCppSetup*() =
-  gitPull("https://github.com/tree-sitter/tree-sitter-cpp", "inc/treesitter_cpp", """
+  gitPull("https://github.com/tree-sitter/tree-sitter-cpp", incDir() / "treesitter_cpp", """
 src/*.h
 src/*.c
 src/*.cc
 """)
 
   let
-    headercpp = sourcePath & "/treesitter_cpp/src/parser.h"
+    headercpp = incDir() / "treesitter_cpp/src/parser.h"
 
   headercpp.writeFile("""
-    typedef struct TSLanguage TSLanguage;
-    const TSLanguage *tree_sitter_cpp();
-  """)
+typedef struct TSLanguage TSLanguage;
+const TSLanguage *tree_sitter_cpp();
+""")
diff --git a/toast.nim b/nimterop/toast.nim
index 13295af..9221802 100644
--- a/toast.nim
+++ b/nimterop/toast.nim
@@ -1,8 +1,8 @@
 import os, strformat, strutils
 
-import nimterop/treesitter/[runtime, c, cpp]
+import "."/treesitter/[runtime, c, cpp]
 
-import nimterop/[ast, globals, getters, grammar]
+import "."/[ast, globals, getters, grammar]
 
 proc printLisp(root: TSNode) =
   var
diff --git a/nimterop/treesitter/c.nim b/nimterop/treesitter/c.nim
index da13f2e..b4a8228 100644
--- a/nimterop/treesitter/c.nim
+++ b/nimterop/treesitter/c.nim
@@ -1,13 +1,12 @@
-import strutils
+import strutils, os
 
-import ".."/setup
+import ".."/[setup,paths]
 
 static:
   treesitterCSetup()
 
 import "."/runtime
 
-{.compile: ("../../inc/treesitter_c/src/parser.c", "parserc.o").}
+{.compile: incDir() / "treesitter_c/src/parser.c".}
 
-const sourcePath = currentSourcePath().split({'\\', '/'})[0..^4].join("/")
-proc treeSitterC*(): ptr TSLanguage {.importc: "tree_sitter_c", header: sourcePath & "/inc/treesitter_c/src/parser.h".}
+proc treeSitterC*(): ptr TSLanguage {.importc: "tree_sitter_c", header: incDir() / "treesitter_c/src/parser.h".}
diff --git a/nimterop/treesitter/cpp.nim b/nimterop/treesitter/cpp.nim
index ec6cf2c..c0eca0c 100644
--- a/nimterop/treesitter/cpp.nim
+++ b/nimterop/treesitter/cpp.nim
@@ -1,14 +1,34 @@
-import strutils
+import strutils, os
 
-import ".."/setup
+import ".."/[setup,paths]
 
 static:
   treesitterCppSetup()
 
 import "."/runtime
 
-{.compile: ("../../inc/treesitter_cpp/src/parser.c", "parsercpp.o").}
-{.compile: ("../../inc/treesitter_cpp/src/scanner.cc", "scannercpp.o").}
+const srcDir = incDir() / "treesitter_cpp/src"
 
-const sourcePath = currentSourcePath().split({'\\', '/'})[0..^4].join("/") & "/inc/treesitter_cpp/src/"
-proc treeSitterCpp*(): ptr TSLanguage {.importc: "tree_sitter_cpp", header: sourcePath & "parser.h".}
+when (NimMajor, NimMinor, NimPatch) < (0, 19, 9):
+  const srcDirRel = "../../build/inc/treesitter_cpp/src"
+else:
+  const srcDirRel = srcDir.relativePath(currentSourcePath.parentDir)
+
+# pending https://github.com/nim-lang/Nim/issues/9370 we need srcDirRel instead
+# of srcDir
+{.compile: (srcDirRel / "parser.c", "nimtero_cpp_parser.c.o").}
+
+#[
+D20190127T231316:here note: this will be compiled as a C++ file even with
+`nim c`, thanks to the extension (which clang/gcc understands);
+however, in `nim c` mode this will fail in link phase
+(which by default would use `clang/gcc`)
+unless linker is overridden, see D20190127T231316.
+
+cleaner alternative: compile `scanner.cc` into a shared library
+that we link against, which avoids the linker hack.
+]#
+
+{.compile: srcDir / "scanner.cc".}
+
+proc treeSitterCpp*(): ptr TSLanguage {.importc: "tree_sitter_cpp", header: srcDir / "parser.h".}
diff --git a/nimterop/treesitter/runtime.nim b/nimterop/treesitter/runtime.nim
index d5fbed5..acb2289 100644
--- a/nimterop/treesitter/runtime.nim
+++ b/nimterop/treesitter/runtime.nim
@@ -1,13 +1,13 @@
 {.experimental: "codeReordering".}
 
-import strutils
+import strutils, os
 
-import ".."/setup
+import ".."/[setup,paths]
 
 static:
   treesitterSetup()
 
-const sourcePath = currentSourcePath().split({'\\', '/'})[0..^4].join("/") & "/inc/treesitter"
+const sourcePath = incDir() / "treesitter"
 
 when defined(Linux):
   {.passC: "-std=c11".}
@@ -15,7 +15,10 @@ when defined(Linux):
 {.passC: "-I$1/include" % sourcePath.}
 {.passC: "-I$1/src" % sourcePath.}
 {.passC: "-I$1/../utf8proc" % sourcePath.}
-{.compile: sourcePath & "/src/runtime/runtime.c".}
+# pending https://github.com/nim-lang/Nim/issues/10299 we need to rename the
+# object files (via compile:(foo,bar)) to avoid name collisions, here 
+# and everywhere `compile` is used
+{.compile: sourcePath / "src/runtime/runtime.c".}
 
 type TSInputEncoding* = distinct int
 converter enumToInt(en: TSInputEncoding): int {.used.} = en.int
@@ -27,7 +30,7 @@ type TSLogType* = distinct int
 converter enumToInt(en: TSLogType): int {.used.} = en.int
 
 const
-  headerruntime = sourcePath & "/include/tree_sitter/runtime.h"
+  headerruntime = sourcePath / "include/tree_sitter/runtime.h"
   TREE_SITTER_LANGUAGE_VERSION* = 9
   TSInputEncodingUTF8* = 0.TSInputEncoding
   TSInputEncodingUTF16* = 1.TSInputEncoding
diff --git a/tests/tnimterop_c.nim b/tests/tnimterop_c.nim
index ece78db..eca3324 100644
--- a/tests/tnimterop_c.nim
+++ b/tests/tnimterop_c.nim
@@ -67,9 +67,11 @@ s1.field1 = 5
 s2.field1 = 6
 s3.field1 = 7
 s4.field2[2] = 5
+
+# note: simplify with `defined(c)` for nim >= 0.19.9
 when defined(cpp):
-  discard # TODO
-else: # TODO: what's `defined(cpp)` for c ?
+  discard
+else:
   s4.field3[3] = enum1
 
 s5.tci = test_call_int
@@ -92,9 +94,9 @@ check test_call_int() == 5
 check test_call_param(5).field1 == 5
 check test_call_param2(5, s2).field1 == 11
 check test_call_param3(5, s1).field1 == 10
-# error: assigning to 'enum ENUM' from incompatible type 'NI' (aka 'long long')
 when defined(cpp):
-  discard # TODO
+  # error: assigning to 'enum ENUM' from incompatible type 'NI' (aka 'long long')
+  discard
 else:
   check test_call_param4(e) == e2
 check test_call_param5(5.0).field2 == 5.0
@@ -103,7 +105,6 @@ u.field1 = 4
 check test_call_param7(u) == 4
 
 when defined(cpp):
-  # TODO
    # note: candidate function not viable: no known conversion from 'NI *' (aka 'long long *') to 'int *' for 1st argument
   # check test_call_param8(cast[ptr int](addr i)) == 25.0
   discard
diff --git a/tests/tsoloud.nim b/tests/tsoloud.nim
index 8d2143b..2d1c40d 100644
--- a/tests/tsoloud.nim
+++ b/tests/tsoloud.nim
@@ -1,10 +1,12 @@
-import os, nimterop/[cimport, git]
+import os, nimterop/[cimport, git, paths]
 
 const
-  incl = "soloud/include"
-  src = "soloud/src"
+  baseDir = nimteropBuildDir()/"soloud"
+  incl = baseDir/"include"
+  src = baseDir/"src"
 
-gitPull("https://github.com/jarikomppa/soloud", "soloud", "include/*\nsrc/*\n")
+static:
+  gitPull("https://github.com/jarikomppa/soloud", baseDir, "include/*\nsrc/*\n")
 
 cDebug()
 cDisableCaching()
@@ -50,4 +52,4 @@ when declared(WavStream_stop):
   assert "WavStream_stop() not skipped"
 
 when declared(WavStream_setFilter):
-  assert "WavStream_setFilter not skipped"
\ No newline at end of file
+  assert "WavStream_setFilter not skipped"