[WIP] fix https://github.com/nitely/nim-regex/issues/60

[timotheecour]

[DO NOT REVIEW YET]

fix nitely/nim-regex#60

nim-regex (after an awesome performance update) is now using import pkg/foo instead of import foo which broke nim CI because it was using nim c src/regex and for some reason src isn't in the path (maybe because the logic for important_packages doesn't install foo ?)

EDIT: doesn't work yet... but i've now found the root cause and it's a bug in important_packages logic, not in regex package:
nimble install regex installs the latest release tag (13.1), as it should (we should only test latest releases, HEAD would be too unstable)
but then git clone regex clones HEAD (which is different from latest release tag), and it fails with import pkg/foo since that refers to the installed regex, not the cloned one.

the fix

either use nimble develop after the clone (but that has the drawback of testing HEAD instead of latest release), or (preferred), after git clone we must git reset --hard rev where rev is the revision used in nimble install

there are other (preexisting) issues though:
if nimble install bar installs foo at a specific version, (say regex at 10.0) that is different from the one implied by nimble install foo, it can cause issues

the obvious short term workaround is to disable regex, but the underlying issue is still there and will keep popping up unless it is fixed

EDIT

upstream regex was updated to remove import pkg/foo so the bug disappeared but underlying issue is still there.
if we're disallowing import pkg/foo (btw we should have a linter to issue warnings when this is detected), that means we're forced to use import foo which can cause ambiguities; maybe what we need is:

import thispkg/foo to refer to current nimble package (implementation is easy, built on top of getNimbleFile from #13223). I will explain this in more details later.

[Araq]

Once again the root cause is Nimble with it's stupid $nimbleDir.

[genotrance]

Every package should be testable without installation. Especially important packages should have a cfg with --path:src or equivalent if not already.

[timotheecour]

@Araq

Once again the root cause is Nimble with it's stupid $nimbleDir.

That's not the root cause, and NIMBLE_DIR is essential for sandboxing in particular on user's machine eg export NIMBLE_DIR=$HOME/.nimble_alt && nimble install foo && nimble test

The root cause is what I described above or here timotheecour#167, and keeps breaking important_packages, as just happened again today with inim (inim-repl/INim#74), see my explanation here: inim-repl/INim#74 (comment)

• important_packages.nim does this:
• nimble install inim => this installs inim at latest release tag
• git clone inim https://github.com/inim-repl/INim && cd INim && nimble test => we've installed inim at latest release tag but test inim at HEAD, which is obviously a bug: latest commit added requires "https://github.com/PMunch/ansiparse" so depends on ansiparse, but that package won't be installed by nimble install since that dependency wasn't added after latest release tag. Or maybe that dependency is there, for totally unrelated ephemeral reasons, because some other important_package installed it.

the short term fix is simple:
after git clone foo, we need to also do git checkout rev where rev is whatever revision got installed by nimble install foo

@genotrance

Every package should be testable without installation.

that can't work if package foo depends on bar.
What should work is nimble test foo where installation still happens, but under-the-scene.

note: testing head vs testing latest release tag

IMO important_packages should only be concerned with testing packages at their latest release tag (eg 1.2.3), not at HEAD. That's what my proposed solution would enable.
Testing at HEAD would just break a lot more often especially wrt depenencies.

Especially important packages should have a cfg with --path:src or equivalent if not already.

I'm on the fence on this. should be discussed further.

[Araq]

IMO important_packages should only be concerned with testing packages at their latest release tag (eg 1.2.3), not at HEAD. That's what my proposed solution would enable.
Testing at HEAD would just break a lot more often especially wrt depenencies.

Ok, that is an acceptable design. However, this PR does nothing like that and stalled.

[timotheecour]

and yet another instance of this: c-blake/cligen#142