search

nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.56k stars 1.47k forks source link

testament fails for packages that require $NIMBLE_DIR/bin in PATH #16447

Open timotheecour opened 3 years ago

timotheecour commented 3 years ago

Example

testament fails for packages such as https://github.com/disruptek/criterion/blob/0170d38964eb19d3276acafe516c70ce1f26afd7/criterion.nimble#L12 which require build (or install) before test eg:

task test, "run unit tests":
  when defined(windows):
    exec "testes.cmd"
  else:
    exec "testes"

Current Output

testes: not found

Expected Output

works

Possible Solution

nimble build runs before nimble test so that $HOME/.nimble//bin/testes is in $PATH before testes can be called

Additional Information

nim: 1.5.1 7256afb00d4451f96b525039b39109d21561a14d

root cause for https://github.com/disruptek/criterion/issues/3 which was breaking nim CI after this is fixed, re-enable criterion (ie, revert https://github.com/nim-lang/Nim/pull/16443)

and also enable jason:

Please do fix this so jason (and other packages of mine that demonstrate regressions) can go in.

/cc @disruptek

disruptek commented 3 years ago

To clarify, the testes binary is supplied by a dependency. My CIs usually call nimble develop in preference to nimble build. YMMV.

disruptek commented 3 years ago

See also https://github.com/alaviss/setup-nim/pull/6 which may be relevant...

/cc @alaviss

alaviss commented 3 years ago

AFAICT testament already installs dependencies:

https://github.com/nim-lang/Nim/blob/fbc8a40c7a351ff7c0f2dc0608bc8926f89d8537/testament/categories.nim#L538

The issue boils down to ~/.nimble/bin not in PATH. I think testament should add this by itself if it wants to properly test things.

It also appears to me that testament use a shared nimble dir for all tests. Not sure if that's a good idea.

timotheecour commented 3 years ago

Not sure if that's a good idea.

indeed; this would cause bugs going un-noticed depending on order in which packages are installed. one option is to keep each package separate as follows:

in pseudocode:

for index, pkg in pkgs:
  cd $root # root = $nim/build/pkgs
  export NIMBLE_DIR=$root/nimble
  removeDir($NIMBLE_DIR)
  export PATH=$NIMBLE_DIR/bin:$PATH
  nimble develop $pkg # this will install deps in $NIMBLE_DIR
  cd $pkg
  nimble test

but this can be fixed later; for now export PATH=$NIMBLE_DIR/bin:$PATH should be enough to fix this issue