Skip to content

Adjusted hook tests to work on windows #236

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 14 commits into from
Aug 18, 2020

Conversation

dr-BEat
Copy link
Contributor

@dr-BEat dr-BEat commented Aug 16, 2020

This PR fixes #235.
The extra newline at the beginning of the hook caused the following error on windows:
error: cannot spawn .git/hooks/commit-msg: No such file or directory

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 16, 2020

Apparently my local setup is different from the CI system.
It still returns the same error: Windows Subsystem for Linux has no installed distributions.\r\r\nDistributions can be installed by visiting the Microsoft Store:\r\r\nhttps://aka.ms/wslstore\r\r\n

@extrawurst
Copy link
Collaborator

@dr-BEat thanks for looking into it. Is the wsl default installed on every Windows nowadays? Or do we have to simulate it by using a vanilla windows without wsl?

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 16, 2020

@extrawurst no problem. Its not installed by default, however I think they added a bash.exe by default that gives the error above.
Usually any git installation on Windows brings a real bash, so it works out normally.
The run_hook function in hooks.rs tries to call bash directly and it gets the fake Windows one.

@extrawurst
Copy link
Collaborator

Any idea how to fix this? Maybe just install some git using chocolaty in the ci?

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 16, 2020

Yeah installing git might fix it.

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 16, 2020

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 16, 2020

According to this blog by Jon Skeet https://codeblog.jonskeet.uk/2019/10/12/using-git-bash-from-appveyor/ the order of directories in the Path environment can break stuff.
I tried to add the git path as the first entry, but it does not seem to work.

@extrawurst
Copy link
Collaborator

@dr-BEat any more ideas?

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 18, 2020

@extrawurst nothing concrete but I will try some tricks, probably the path is not correct still

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 18, 2020

@extrawurst the path environment is correct, the problem is that Command::new("bash") in hooks.rs line 105 somehow does not follow normal windows rules in finding which bash to execute.
If I hardcode the full path to the git bash with Command::new(r"C:\Program Files\Git\bin\bash.exe") it works perfectly.

@extrawurst
Copy link
Collaborator

@dr-BEat interesting. maybe because we run the cargo test in the git shell, did you try setting the shell: cmd in the ci to make him use the windows stuff?

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 18, 2020

@extrawurst I do not think its related to the shell. I can reproduce the issue locally when just running cargo test from Powershell

@extrawurst
Copy link
Collaborator

@dr-BEat nice a repro is what we needed!!🥳 I wish I could add any windows knowledge whatsoever :(

@dr-BEat
Copy link
Contributor Author

dr-BEat commented Aug 18, 2020

@extrawurst found it! Calling env on the command fixed it. This is probably related to the discussion here rust-lang/rust#37519. The behaviour of Command::spawn() changes because the environment is passed differently to Windows if env is called.

@extrawurst
Copy link
Collaborator

OMFG!!! you are the man!!

@@ -105,6 +105,7 @@ fn run_hook(
let output = Command::new("bash")
.args(bash_args)
.current_dir(path)
.env("DUMMYENV", "FixPathHandlingOnWindows") // This call forces Command to handle the Path environment correctly on windows, the specific env set here does not matter
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf... this is crazy! 🙈

@extrawurst extrawurst merged commit 14cfb39 into gitui-org:master Aug 18, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows commit hooks without wsl
2 participants