Fix tests and execution on Windows #20
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
waych
force-pushed
the
master
branch
2 times, most recently
from
69e760f to
d8f41d0
Compare
This way the test can pass on Windows as well
This seems to show up in Window git.exe output.
Replace guessing at paths for `true` when setting GIT_EDITOR and use ':' instead. ':' is normally an equivalent to `true`, and git treats GIT_EDITOR=":" especially to skip the editor invocation (see strcmp() against ":" in `git/editor.c:launch_specified_editor()`. Using it allows us to remove the dependency on `true`, making this code simpler and portable to Windows.
In tests, when the "system git" is desired, PATH_TO_GIT is ignored and a hardcoded path to the system git is used (typically /usr/bin/git). "system git" is only used in two places, and it is unclear how important those locations be handled by "system git" rather than "git under test", so just do the conservative thing and add a path to the default location of git.exe on Windows.
waych
force-pushed
the
master
branch
4 times, most recently
from
67d8a3b to
77d541d
Compare
To run correctly on Windows, all invocations of hooks need to be executed as an argument to bash. This changes the code to build commands using get_bash() with the `-c` flag. When discovering which bash to use, we are careful on Windows to pick "git-bash" aka the bash.exe that comes with git. This is neccesary as git-bash uses different path translations than regular bash.exe, and using the wrong one would result in git.exe not being able to discover the git-branchless.exe binary correctly (when invoked in hook). If the target_os isn't windows, `sh` is for executing hooks instead.
Contributor
Author
|
PTAL Rebased and addressed feedback. |
Owner
|
LGTM, thanks again! |
Owner
|
@waych Do you think there's anything else which should be done to say that it supports Windows? If not, I can tag a new release and announce support. |
Contributor
Author
|
@arxanas Can't think of anything else that needs to be done at this point other than updates to the changelog. 😃 |
nh2
reviewed
Jun 23, 2021
Closed
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes various issues affecting execution of git-branchless commands and tests on Windows.
With these changes all tests pass and commands appear to work as intended.