Conversation
This removes `normalize(...)` around a command as I suspect this is a
bug.
Basically, `x('./foo')` would result in `x('foo')` as far as I can tell.
Which means the OS will search `PATH` for `foo` rather than calling
exactly the one in `cwd`.
This has existed since the beginning of tinyexec, so it doesn't seem to
be a noticed bug. I also can't think of a reason we need to `normalize`
since Windows and cross-spawn handle slashes, relative resolution, etc.
253224c to
59b92f9
Compare
|
@iiroj and @AriPerkkio if i publish a pre-release of this, any chance you could confirm it doesn't break any tests on your end? this code was here since the start so there's no reasoning why we ever did it. im fairly confident its fine to be removed but would be nice to know some consumers have tried it |
|
@43081j sure, we can run lint-staged CI tests in a MR. |
|
i've published a pre-release here: you can use let me know if all is well 🙏 |
|
Lint-staged tests also pass, but I don't think they hit this scenario. |
|
although it is fixing on scenario, it changes a branch all calls hit. thats what i was more concerned about. so, given both are passing i think im happy to release it 👍 meanwhile, i did prove that the new tests fail in main (showing that the fix works). thanks so much to both of you for testing it 🙏 |
This removes
normalize(...)around a command as I suspect this is abug.
Basically,
x('./foo')would result inx('foo')as far as I can tell.Which means the OS will search
PATHforfoorather than callingexactly the one in
cwd.This has existed since the beginning of tinyexec, so it doesn't seem to
be a noticed bug. I also can't think of a reason we need to
normalizesince Windows and cross-spawn handle slashes, relative resolution, etc.