PATH's different names in Windows and Linux causing git-branchless hook execution to fail

[c00t]

I do a lot of game engine programming so i can't avoid using git under native windows. When using git-branchless, I also had similar problem as (#370 , #1144 ). Begin debugging...

In #370

---

Definitely seems like a git on Windows bug. Just sort of sucks because it makes git branchless sort of broken on Windows. I'm moved to exclusively using git in WSL/Ubuntu so it's no longer a problem for me on my home computer. I'm starting a new job next week which is going to be on Windows and I don't really know how I'll be interacting with source control and I don't know much about how the environment is set up. Depending on how that turns out, I may dig more into what it takes to make this work properly.

No, This is not a Git for Windows problem, after I debugged it, I realized that it is due to the naming of the Path environment variable, in the following code, git-branchless reads the environment variable named PATH, but in windows, the variable name should be Path.

https://github.com/arxanas/git-branchless/blob/e7c3653c5e42f67a967d30ea57ded5a34cc6f9aa/git-branchless-lib/src/git/run.rs#L407-L422

I believe this is due to the fact that the windows shell is insensitive to the case of environment variable names (both pwsh and cmd), which many developers don't pay attention to on windows, you can read the environment variables correctly in the shell with Path PATH or even PaTh.

quick fix:

        let GitRunInfo {
            // We're calling a Git hook, but not Git itself.
            path_to_git: _,
            // We always want to call the hook in the Git working copy,
            // regardless of where the Git executable was invoked.
            working_directory: _,
            env,
        } = self;
        let path = {
            let mut path_components: Vec<PathBuf> =
                vec![std::fs::canonicalize(&hook_dir).wrap_err("Canonicalizing hook dir")?];
            if let Some(path) = env.get(OsStr::new("PATH")) {
                path_components.extend(std::env::split_paths(path));
            }
            #[cfg(target_os = "windows")]
            if let Some(path) = env.get(OsStr::new("Path")) {
                path_components.extend(std::env::split_paths(path));
            }
            std::env::join_paths(path_components).wrap_err("Joining path components")?
        };

---

yep, windows target's std crate also:

fn main() {
    println!("{:?}", std::env::var_os("Path")); // valid
    println!("{:?}", std::env::var_os("PATH")); // valid
    println!("{:?}", std::env::var_os("PaTh")); // valid
    // but 
    let x = std::env::vars_os().collect();
    // put "Path" inside x
}

Originally posted by @c00t in https://github.com/arxanas/git-branchless/issues/370#issuecomment-1971239404

[c00t]

Since #370 is marked as no-planned-fix, pull request still needed? Or just a quick fix in your planned commit?

[AaronLieberman]

After restoring my hooks to their original state (without the full path workaround I had been using), I deleted my local Path variable and added PATH and re-ran one of my failing test cases. The hook git branchless was able to find git correctly and everything seemed to work. Nice find, C00t! Is someone who has git-branchless set up for development able to make a PR for this? I guess I could... but I'd have to figure out how to get Rust set up for development again.

[c00t]

@AaronLieberman I've done a quick fix in my fork, and if you use cargo you can install it using below command. But I'm not sure if this fix is correct (i.e., only modifying the implementation of the run_hook function) and if such a small modification would need to be merged as a PR.

cargo install --git https://github.com/c00t/git-branchless -f

[oltolm]

A workaround for this bug is to create and execute a file fix-path.cmd with the following contents

@echo off
set OLDPATH=%PATH%
set PATH=
set PATH=%OLDPATH%
set OLDPATH=

[AaronLieberman]

@arxanas @c00t's fix works for me. I tested it locally based on that branch and it's working for me on Windows as well on Ubuntu on WSL2. It looks like quite a safe change as from the code it looks like it wouldn't affect non-Windows platforms. Having this in the mainline would be a big improvement so that Windows would work reliably.

Would you be able to merge it into mainline? I tried to open up a PR for it but I'm not in the contributors group so it wouldn't let me. This change fixes issues #1256, #370, and #1144.

[arxanas]

@c00t can you open a PR, or a otherwise affirmatively state that your code is fine to merge under the git-branchless license?

[c00t]

@arxanas done #1267