Change win program resolution to match macOS/Linux

[lread]

After some careful study, I now understand the underlying behaviour of ProcessBuilder on Windows.

It has some nuances:

• it resolves from the current dir before PATH (which CMD Shell does too - but PowerShell opted not to do for security reasons)
• it only resolves a to a.exe
• and has what seem to be bugs wrt to resolving when a directory working dir is specified

These oddities were sometimes leaking through to babashka process when run from Windows.

We now entirely take over the program resolution for Windows in our default :program-resolver which now matches the behaviour of PowerShell (with the exception of launching .ps1 files, we do not do that), macOS and Linux.

Note that the default :program-resolver is also employed for exec on all OSes. Because it matches macOS/Linux behaviour this does not present any problems.

Tests now thoroughly exercise program resolution via explicitly constructed scenarios instead of awkwardly relying on what happens to be on the PATH. See new test-resources dir README for some details.

Bb tasks adjusted accordingly to setup the PATH for tests. New dev:bb and dev:jvm tasks added to launch nREPLs with this same setup.

README is updated with new section on Program Resolution.

Closes #163
Closes #164

[lread]

Ready for your review, @borkdude; as usual, I am looking forward to your questions, comments, and feedback!

[borkdude]

Why is this change necessary? It wasn't there before?

[lread]

We did absolutize before but only for relative programs with a dir.
(This was a change I made a while back f2c8d93, I am now wondering if it is absolutely necessary? 🥁)

It could be I am over-absolutizing here. The thing I liked about it is that it makes it very clear exactly what we are resolving to. I think I added normalize to make it clearer what we were absolutizing to, and that also made some test comparisons easier.

But I also see the value of not absolutizing/normalizing unless we need to. Can explore.

[borkdude]

I prefer not processing the program name unless we have to as this may also affect the name of the program that is seen in exceptions etc. (same reason I don't prefer adding an extra cwd above)

[lread]

Sounds reasonable to me, I'll make the change.

[borkdude]

I committed the first suggested (because all the tests still worked, I felt it was permissible) but this other change fails a bunch of tests. To diagnose via CI I reversed the bb and JVM tests as the JVM tests give better error messages. Then when I changed the resolved return value to (str resolved) half the CI tests started working, except Windows. FWIW.

[lread]

Thanks for the review!

Another thing to think about is that we have a single default program-resolver for

• process on Windows
• exec on all OSes

We could perhaps explore that their needs are different enough to warrant separate default implementations. For example, for exec on macos/Linux, a program-resolver could likely do less and fall through more.

Anyway before I make any further changes, I'll wait for your follow-up responses.

[borkdude]

We could perhaps explore that their needs are different enough to warrant separate default implementations.

Why actually? The need for this isn't entirely obvious to me.

[lread]

We could perhaps explore that their needs are different enough to warrant separate default implementations.

Why actually? The need for this isn't entirely obvious to me.

I was thinking more of your desire to just pass through to the underlying JVM call when possible. I was focusing on Windows when modifying the :program-resolver. I did not carefully study exactly what resolving exec needs on macos/Linux. Maybe it needs exactly the same work to be done (or maybe it could do less and pass through more?) it's just something I did not look into very carefully for this PR.

[borkdude]

I don't know, exec doesn't support :dir anyway so I don't think any bugs were introduced there at some point.

[borkdude]

or do you mean, the exec-ing into a binary in the cwd?

[lread]

Yeah, the same default :program-resolver we use for process on Windows is used for exec on all OSes. It works as is, but wondering out loud if they actually should be the same (and also not suggesting anything different).

[borkdude]

I don't see a reason why they should be different but perhaps something will come to mind. Happy to address it in a future issue.

[lread]

@borkdude, this commit should pass all tests when run from babashka/process, but I expect I'll need to tweak test expectations when running these process tests from babashka, so please don't merge yet. I'll follow up with a comment when I think things are all good.

[lread]

Ok @borkdude, barring any more questions/concerns from you, I think you can merge now.

The absolutize is only necessary for ProcessBuilder on Windows, but should not be triggered for supported use of exec as :dir is not supported for exec.

[borkdude]

Just one final remark, then we're good. I'm starting to see why you would want to split program-resolver for exec now (although, let's abstain until we see something going wrong)

[borkdude]

Thanks a lot for your thorough work!

[lread]

You are always welcome @borkdude! Sorry, it took so long, but we now have a good understanding (and a reference) of how ProcessBuilder works on Windows!

[borkdude]

No problemo!