Fix problem with Win10 shell #560
Conversation
FlorianWilhelm
force-pushed
the
fix-556-win-shellcommand
branch
from
894bfb8
to
51b0d48
Compare
Pull Request Test Coverage Report for Build 4659659180081152
💛 - Coveralls |
FlorianWilhelm
force-pushed
the
fix-556-win-shellcommand
branch
2 times, most recently
from
0d25549
to
071f20c
Compare
FlorianWilhelm
force-pushed
the
fix-556-win-shellcommand
branch
4 times, most recently
from
dde5603
to
5f02572
Compare
|
Just for my own tracking, this URL should give us the diff between the 2 branches instead of the comparison with master: fix-556-debugging...fix-556-win-shellcommand Edit: without thinking much, I rebased the other PR and removed its branch, which ruined this strategy... The best would be merging it and leaving the branch open until this PR was closed... Now I have to rebase and fix conflicts 😝 |
abravalheri
force-pushed
the
fix-556-win-shellcommand
branch
from
fbdb83e
to
e18b93f
Compare
| # ^ CI setup does not aggregate Windows coverage | ||
| for cmd in ["git.cmd", "git.exe"]: |
There was a problem hiding this comment.
I understand from this removal that "git.cmd" is no longer necessary, and that we expect any distribution of git for Windows to add a .exe to the PATH, right?
There was a problem hiding this comment.
Yes. I was actually trying to find git.cmd by googling but had no luck. There is a "git CMD" but no "git.cmd" so maybe it was a mistake? Also setuptools-scm doesn't seem to be doing this, so at least we would fail both.
| for cmd in ["git.cmd", "git.exe"]: | ||
| git = ShellCommand(cmd, **args) | ||
| for shell in (True, False): | ||
| git = ShellCommand("git.exe", shell=shell, **args) |
There was a problem hiding this comment.
I am very curious to understand what is happening for the command with shell=True to be failing...
Would be a problem with file names?
There was a problem hiding this comment.
Yea, I am also very curious about this. We only now that one of @ksteptoe's two Windows machines seem to have a problem with the shell setup. Somehow an & is added but I have no good explanation for that. I think it will be impossible to figure this out without access to the machine. Also I am no Windows (Power)shell expert.
|
Thank you very much @FlorianWilhelm for debugging and solving this problem even without access to a Windows environment. That was epic! ❤️ 💜 💚 💛 💙 |
This fixes the problem with the Windows shell in issue #556. The basic solution is to add some more environment variables as stolen from setuptools-scm's
utils.do_exand in case of Windows try to find out if it is better to call a command withshell=Trueorshell=False. It seems that this differs from time to time.Note: Only review and merge after having merged PR #559 into master as I needed to rebase it on #559 so the diff is actual way smaller.