fix #13222: make relativePath more robust and flexible

[timotheecour]

• relativePath(foo) now works
• relativePath(rel, abs) and relativePath(abs, rel) now work (fixes relativePath("foo", "/") and relativePath("/", "foo") is wrong #13222)
• relativePath, absolutePath, getCurrentDir now available in more targets (eg: vm, nodejs etc)
• [EDIT] isAbsolute now works with -d:js (previously was always returning false)
  see tests

[Araq]

Oh no. I don't want to expose getCurrentDir in the VM. It is a recipe for desaster builds that need a specific environment/setup. If you do nim c -r tests/foobar.nim or cd tests && nim c -r foobar.nim shouldn't be different, both should work equally well.

[timotheecour]

Oh no. I don't want to expose getCurrentDir in the VM

removed it to unblock this PR; I understand the concern and this could be addressed similarly to cpuTime (where reproducible builds was the concern) via something analog to

  if optBenchmarkVM in c.config.globalOptions:
    wrap0(cpuTime, timesop)
  else:
    proc cpuTime(): float = 5.391245e-44  # Randomly chosen
    wrap0(cpuTime, timesop)

for users who understand that and still need the feature, but I've left this out of this PR to unblock it

[timotheecour]

PTAL:

• resolve conflict
• fixed a new bug i found: isAbsolute was always returning false with nim js (ie on a browser) ; now it works with nim js; and with nim js -d:nodejs on posix, but not on windows, which is due to a separate, pre-existing issue, see defined(windows), defined(posix) etc should work for nim js -d:nodejs; otherwise code breaks #13469
• added tests

[disruptek]

We talked about adding getCurrentDir for bump's use-case. I would use this in nimph and testutils and golden and openapi, too. I guess I'm just a bad person.

I think you have to concede that if this isn't provided, a work-around will be developed, and that's clearly a worse option. Can you really champion NimScript as the right tool for writing build scripts when one cannot even find the current working directory in NimScript?

[Araq]

1. It shouldn't have been part of this PR.
2. The feature must be called staticGetCurrentDir or similar to prevent accidents like const x = getCurrentDir(). And we had real bugs like that in the past, it's why I changed my mind on these things. I'm not evil. ;-)

[Araq]

This patch that is really hard to review. Can't you achieve the same without rewriting the code?

[timotheecour]

This patch that is really hard to review. Can't you achieve the same without rewriting the code?

I agree it's hard to review but it's due to github diff view not dealing well with blocks of code being re-indented, I did not rewrite the code.

I know 2 options:

hidden github feature: ?w=1

I've just found a really interesting solution: adding ?w=1 which I say here https://github.blog/2011-10-21-github-secrets/
As you can see in https://github.com/nim-lang/Nim/pull/13451/files?w=1 I did not rewrite code, it's just re-indentation change; this should be super helpful in other code reviews.

caveat: this solution is a bit blunt, see caveats here: timotheecour#37

git diff --color-moved-ws=allow-indentation-change --color-moved=blocks

This is the hint I added to https://nim-lang.github.io/Nim/contributing.html which helps if you locally fetch the PR, it helps by coloring differently code that was moved.
see caveats here: timotheecour#37

[timotheecour]

PTAL (you can just review last 2 commits)

[Araq]

You missed to update tests/js/tos.nim

[timotheecour]

You missed to update tests/js/tos.nim

done, PTAL (and see comments above)

[Araq]

Gah, I screwed up the changelog cleanup...

[timotheecour]

@Araq

Gah, I screwed up the changelog cleanup...

TLDR: we should rebase, not merge

I've rebased (instead of merged) to keep git history linear. Merge makes it really hard to understand individual commits as they get mixed with unrelated changes (even if this is squashed + merged in github UI, the individual commits of PR can still be seen in github UI, and are useful to follow the PR.

@disruptek

We talked about adding getCurrentDir for bump's use-case. I would use this in nimph and testutils and golden and openapi, too. I guess I'm just a bad person.

this is now possible since #13813 via --experimental:vmopsDanger