New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MSYS2 "fix" broke lint-staged
on my MSYS2 setup
#1267
Comments
Interesting, I've have been using the same setup (but updated to the latest version) since then and it still works for me. I could try to install a new fresh installation of msys2 and give your updated example a go. I agree with the conclusion that it might be something else that causes the need for needing the extra escaping. |
I just tried to comment the workaround in I'm using the MSYS2 provided git (currently 2.39.2) of that could make a difference. |
That could be a clue, I'm using everything from outside (I have the Windows PATH appended to my MSYS path). I figured those play better with my sources also in the "Windows side". Also better perf in general. Before reinstalling, you could check what happens if you use the external git instead. |
I can confirm it is working when I run git for windows (by prepending it to working (git for windows):
non-working (git installed via msys2 pacman)
Both of these tests are running with the workaround from #1121 commented (by editing |
Nice find, I could repro the same! InvestigationI actually boiled it down to this: const execaLib = require("execa");
const {spawnSync} = require('child_process');
spawnSync("git", ['--no-pager', 'show', 'refs/stash@{0}'], {stdio: 'inherit', stderr: 'inherit', windowsVerbatimArguments: true})
spawnSync("git", ['--no-pager', 'show', 'refs/stash@{0}'], {stdio: 'inherit', stderr: 'inherit', windowsVerbatimArguments: false})
spawnSync("git", ['--no-pager', 'show', '"refs/stash@{0}"'], {stdio: 'inherit', stderr: 'inherit', windowsVerbatimArguments: true})
spawnSync("git", ['--no-pager', 'show', '"refs/stash@{0}"'], {stdio: 'inherit', stderr: 'inherit', windowsVerbatimArguments: false}) No quoting - verbatim - GOOD No quoting - verbatim - FAIL So with quoting and IdeaThis execution / escaping is a mess. It'd be nice to figure this out for good, but I also had another idea, that is likely a much less worrying change: Why don't we simply use a number to refer to the stash, you can do that in git 2.11+ (mid-2016). See this StackOverflow thread essentially asking about the same question we're running into: https://stackoverflow.com/q/17454235/10614791 A simple What do you think? |
@marcelltoth I don't think we've previously established version requirements for |
Fixed in #1270 |
Description
I'm not sure what issue @ext was facing in #1121 but whatever configuration I tried, my MSYS does not reproduce that.
OTOH the fix implemented in #1178 completely breaks
lint-staged
for me (using MSYS2) due to the extra escaping, as my terminal / shell behave normally.Steps to reproduce
Install MSYS2, use whatever environment, install
lint-staged
, add some files and try runningnpx/yarn lint-staged
. I haven't prepared a repro forlint-staged
just yet, but I played around with the test script @ext shared in his issue, slightly modifying it so that it now uses ESM (which is a requirement for currentexeca
.You can see the gist here: https://gist.github.com/marcelltoth/f987cf03e43783a91d3cb7a0594bb6e5
I tried running this in:
zsh
as the login shell.bash
as the shell.In every single case, git was indeed executed with
refs/stash@{0}
, no lost curly braces. I don't even see where it would get lost, asexeca
executes the command without a shell (good choice IMO), so why does the whole thing even has anything to do with a shell environment?Note
Another interesting point is this comment here #1121 (comment) that suggests the person needed a similar fix, even though he was not using MSYS (otherwise putting code under the fix wouldn't have done him any favors).
So I'm assuming we have misidentified the problem and it has to do with something else (git version? Node?), not MSYS.
While we're figuring that out, can we undo the "fix"? It breaks my env that behaves completely normally, to fix someone else's that's somehow messed up.
Debug Logs
expand to view
Environment
The text was updated successfully, but these errors were encountered: