Skip to content
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

git-bindir is being ignored on Windows #1258

Open
zilti opened this issue Sep 19, 2022 · 7 comments
Open

git-bindir is being ignored on Windows #1258

zilti opened this issue Sep 19, 2022 · 7 comments

Comments

@zilti
Copy link

zilti commented Sep 19, 2022

Since I currently have the misfortune of having to use Windows (and at that one without admin rights) I installed both Git and Git-Cola there. Unfortunately Git doesn't stay on the path, so as advised in the error dialog box, I tried creating the git-bindir file with the following content:

C:\Users\ziltenerd\AppData\Local\Programs\Git\bin

I created it both at the C:\Users\ziltenerd\.config\git-cola directory that gets auto-created as well as at C:\Users\ziltenerd\AppData\Roaming\.config\git-cola. Both had no effect at all.

@davvid
Copy link
Member

davvid commented Sep 29, 2022

This has been a curiosity for me, but I'll need your help diagnosing this. I think this is basically a duplicate of #927 .

If you could kindly hop over there and diagnose what os.path.expanduser('~/.config/git-cola/git-bindir') expands to in your environment then we might be able to solve it.

The relevant code is in cola/app.py (search for git-bindir) and we're just asking python to expand the value.

In case you just need something working, I just pushed a fix that makes cola honor XDG_CONFIG_HOME in your environment. That'll control the directory where it expects to find git-bindir file, in case that's the root cause.

Thanks for the heads-up. If you can help with testing in #927 then I'd be open to resurrecting the old patch that I whipped up in that issue, but I don't want to bifurcate the threads so I'm closing this issue.

There wasn't anyone available to test it, so I wasn't comfortable applying the patch that I worked on back then, but if you're able to run from git and test stuff out then we can probably get this sorted pretty easily.

@davvid davvid closed this as completed Sep 29, 2022
davvid added a commit to davvid/git-cola that referenced this issue Sep 29, 2022
Allow users to control where git-cola looks for git-bindir by supporting
the XDG_CONFIG_HOME environment variable.

Related-to: git-cola#1258
Signed-off-by: David Aguilar <davvid@gmail.com>
@zilti
Copy link
Author

zilti commented Sep 30, 2022

I ran it:

>>> import os
>>> os.path.expanduser('~/.config/git-cola/git-bindir')
'C:\\Users\\ziltenerd/.config/git-cola/git-bindir'

So it is in the same directory as the settings file.

@zilti
Copy link
Author

zilti commented Sep 30, 2022

I've tried setting XDG_CONFIG_HOME to C:\Users\ziltenerd\.config but that made no difference.

@davvid
Copy link
Member

davvid commented Sep 30, 2022

Quick question: are you launching cola from the shell or from the shortcut icon?

I'm reading through the code and our main() function doesn't do the win32 git-bindir $PATH fixups. If you're launching it through the git-cola command then I suspect that I need to change where we initialize that.

The launcher icons all go through app.shortcut_launch() which is where the $PATH adjustment happens (via winmain()).

Let me know if you're launching it a different way. If so we probably just need to adjust where we do that initialization

@zilti
Copy link
Author

zilti commented Oct 4, 2022

I've been starting it from the shortcut icon so far.

davvid added a commit to davvid/git-cola that referenced this issue Oct 11, 2022
We have been running into configuraiton issues on platforms where the
.gitconfig might be stored in a location that is opaque and unknown to
cola a-priori.

The root cause is that we read each config file separately, with three
different "git config --file ..." calls, and this logic requires that we
know *exactly* which config files we are going to read.

Another downside to reading the config files one-at-a-time is that it
requires three calls to "git".

Rework the config reader so that it reads all of the configuration using
a single call to "git config --show-origin --show-scope --list".

"git config" will output the file paths that it reads when run this way,
which alleviates cola from needing to know about the paths itself.

Furthermore, this ensures that we will read the config value in all
situations. The file paths are still used to maintain a cache (to
prevent repeated calls to "git config") but they are no longer necessary
for reading the values.

Closes git-cola#927
Related-to: git-cola#1258
Related-to: git-cola#1263
Signed-off-by: David Aguilar <davvid@gmail.com>
@davvid davvid reopened this Oct 11, 2022
@davvid
Copy link
Member

davvid commented Oct 11, 2022

Thanks for those details -- I was hoping that 4726cd5 might help because maybe we weren't always adding the configured git-bindir to the $PATH, but since you're launching from the shortcut icon then it should have already been going through the $PATH setup code.

In any case, I've adjusted the logic so that this code path is always active. I kinda doubt that'll make a difference.

The other thing that changed is that the config reader was just rewritten to ensure that we can always read gitconfig-provided configuration values. The fixes in #927 seemed related because they both involve reading from ~ ($HOME) but they don't really affect the git-bindir code path because that reads from the $XDG_CONFIG_HOME/git-cola/git-bindir file directly.

Thus, I've re-opened this issue since what we're hitting here seems unique. This is a bit surprising.

All we're really doing is reading ~/.config/git-cola/git-bindir and then adding the value we read from the file to the front of the PATH environment variable.

All of the points to inspect are in cola/app.py. The two functions of interest are def find_git() and def prepend_path(path).

find_git has an early-out if it doesn't think we're on Windows. What does this print in your install?

import sys
print(sys.platform)

We only consider win32 and cygwin values in def is_win32() from cola/utils.py -- is there a chance you have a different value? If so that would cause find_git() to return None and it won't add the value to the PATH. That said, I tried looking at the value in a VM and I saw win32 there, so probably not that.

The other place it could fail is in def prepend_path(path). This function is fairly straightforward. It reads the git-bindir file and prepends to the PATH environment variable. It's kinda surprising that this function would fail, but anything is possible.


Here's a some other details that might help out.

Earlier you mentioned that you could set environment variables (XDG_CONFIG_HOME).

If you can do that, are you able to prepend to PATH locally and then see if that works sans git-bindir?

Alternatively, I noticed that we do support one other environment variable in cola/git.py:

GIT = core.getenv('GIT_COLA_GIT', 'git')

If you define GIT_COLA_GIT in your environment and point it directly to the full, absolute path to git.exe then we will use that value when executing git. When undefined we just use git and expect to find it in $PATH.

In theory we should be able to get this working by messing with just the GIT_COLA_GIT environment variable. There's much less machinery involved in that code path so it might shed some light on what's going on.

Can you try using that environment variable to see if it helps? Note that you will need to launch from a Git Bash shell in order for it to pickup your shell environment.

Are you able to run from the source tree? If so it'd be worth testing the latest from git in case the recent fixes helped in some way, but GIT_COLA_GIT has been around for a very long time so if you can control the environment then that should be usable with your current version.

Thanks again for trying this stuff out.

@zilti
Copy link
Author

zilti commented Oct 25, 2022

print(sys.platform) prints win32 on my platform. And setting GIT_COLA_GIT does indeed work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants