Fixes #802, #803 and #3775 - genscript issues #7677
Conversation
|
Very nice, but it needs a test case to keep this feature from bit-rotting. |
|
Added a new test category for |
tests/testament/categories.nim
Outdated
| script &= ".bat" | ||
|
|
||
| when defined(linux): | ||
| cmd = "bash -c \"" & cmd & "./$#\"" |
There was a problem hiding this comment.
And if bash not installed?
There was a problem hiding this comment.
Bash is available on Travis so test passes. Do we need to change this to "sh -c" instead? Do we run full tests on other systems that don't have bash?
There was a problem hiding this comment.
Do we need to change this to "sh -c" instead?
IMO, it would be better.
There was a problem hiding this comment.
I've changed it to sh, makes sense.
doc/advopt.txt
Outdated
| --noMain do not generate a main procedure | ||
| --genScript generate a compile script (in the 'nimcache' | ||
| subdirectory named 'compile_$$project$$scriptext') | ||
| subdirectory named 'compile_$project$scriptext'), |
There was a problem hiding this comment.
this line causes the build to fail. You need to keep the doubled $$
tests/testament/categories.nim
Outdated
| cmd = "sh -c \"" & cmd & "./$#\"" | ||
| script &= ".sh" | ||
|
|
||
| setFilePermissions(flagsDir / "nimcache" / script, |
tests/testament/categories.nim
Outdated
|
|
||
| var build = "" | ||
| var run = "" | ||
| var cmd = "cd " & flagsDir / "nimcache" & " && " |
There was a problem hiding this comment.
Don't use && and string munging. Call execShellCmd once or twice instead.
tests/testament/categories.nim
Outdated
|
|
||
| # Build | ||
| var (outp, errC) = execCmdEx(build) | ||
| r.total += 1 |
There was a problem hiding this comment.
There should be procs for this, don't mess manually with r.total and r.passed here.
|
Per feeback from @Araq, I've improved the testing of |
| if exitCode != 0: given.err = reExitCodesDiffer | ||
| if given.err == reSuccess: inc(r.passed) | ||
|
|
||
| proc testExec(r: var TResults, test: TTest) = |
There was a problem hiding this comment.
It is still a bit dubious why this is required, but it's much better than before. Will merge once the tests are green.
Fixes #802 - moved compile script into the
nimcachedirectory - makes most sense since the idea is to take thisnimcachedirectory onto the target machine to build so having the script together with the code makes sense. Also easy since the.cfile paths don't need any adjustments in the generated script.Fixes #803 - copies
nimbase.hinto thenimcachedirectory since it is required for build on the target machine.Fixes #3775 -
--genscriptmeansnoAbsolutePaths()so compile commands all failed due to bad paths. But there's no reason to compile anything on this machine, so--genscriptshould imply--compileOnly.