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

error: committer identity unknown; please configure user.name and user.email #407

Closed
char101 opened this issue Jan 24, 2024 · 18 comments
Closed

Comments

@char101
Copy link

char101 commented Jan 24, 2024

Hi,

I tested running stg init (OS: Windows 10) and although I have user.name and user.email configured, I got this message: error: committer identity unknown; please configure user.name and user.email.

R:\test-git>git config --show-origin -l | rg user
file:A:/Home/.gitconfig user.name=Charles
file:A:/Home/.gitconfig user.email=my@email

R:\test-git>echo %HOME%
A:\Home

R:\test-git>stg init
error: committer identity unknown; please configure `user.name` and `user.email`.

Looking at the source it should have found the config file at HOME.

https://docs.rs/gix-config/0.34.0/src/gix_config/source.rs.html#103

For it to work I have to make a symlink from USERPROFILE to HOME, which is not a big deal but the behavior does not conform to git behavior

R:\test-git>mklink %USERPROFILE%\.gitconfig %HOME%\.gitconfig
symbolic link created for C:\Users\char\.gitconfig <<===>> A:\Home\.gitconfig

R:\test-git>stg init
@jpgrayson
Copy link
Collaborator

Thanks for this issue report @char101.

gitoxide uses the home crate, which has very specific policy regarding how the HOME environment variable is (or isn't) used on Windows.

I think the docs for home::home_dir() may be relevant for this issue. Specifically, it looks like the USERPROFILE environment variable is going to be used to find the user's "home" on Windows, followed by some Windows API calls if USERPROFILE isn't set.

So I wonder how your %USERPROFILE% compares to your %HOME%? And whether placing your global .giticonfig in %USERPROFILE% might resolve the StGit problem? And if so, does git still find your global config if it's in %USERPROFILE%?

I do not have a Windows environment on-hand, so I could use help with this additional triage. Thanks!

@char101
Copy link
Author

char101 commented Jan 24, 2024

I think the docs for home::home_dir() may be relevant for this issue. Specifically, it looks like the USERPROFILE environment variable is going to be used to find the user's "home" on Windows, followed by some Windows API calls if USERPROFILE isn't set.

Changing HOME environment variale is usually safe because it is usually only used by linux apps that is ported to Windows. Changing USERPROFILE environment variable is a bit more dangerous because because it is also used by Windows itself. Also it is hit and miss, HOME is more commonly supported and also cross-platform.

So I wonder how your %USERPROFILE% compares to your %HOME%

I generally don't like storing config files in USERPROFFILE because it is a mess.

And whether placing your global .giticonfig in %USERPROFILE% might resolve the StGit problem

It did. I can run stgit by symlinking %HOME%\.gitconfig to '%USERPROFILE%.gitconfig`

The problem is that the HOME environment variable is perfectly supported by git on Windows, so I think any application that read git config should conform to it. But of course if the cause is underlying library then there is probably nothing that stgit can do about it.

And if so, does git still find your global config if it's in %USERPROFILE%

No, actually if I symlink .gitconfig from HOME to USERPROFILE, the USERPROFILE .gitconfig does not appear in git config -l --show-origin list, probably because git already find one in HOME. And placing .gitconfig in USERPROFILE is not something I want to do.

@char101
Copy link
Author

char101 commented Jan 24, 2024

Another problem with stgit on Windows is that it failed to launch the configured editor:

[Thu Jan 25 06:47] R:\test-git [master]
> stg init

[Thu Jan 25 06:47] R:\test-git [master]
> stg new patch1
hint: Waiting for your editor to close the file...
error: program not found

[Thu Jan 25 06:47] R:\test-git [master]
> git config -l | rg editor
core.editor=A:/Editor/vim/launcherwait.exe

@jpgrayson
Copy link
Collaborator

Thank you for the additional triage, @char101. Regarding how HOME is treated, I think that both StGit and gitoxide both strive to do what git does in these kind of situations. Perhaps @Byron would be interested in this issue.


Regarding the editor not being launched as expected when running stg new, here is the relevant StGit code at play:

https://github.com/stacked-git/stgit/blob/master/src/patch/edit/interactive.rs#L97-L105

Which is trying to emulate how git runs an editor subprocess. Note specifically how it tries to run the command via sh. This comes from this git code:

https://github.com/git/git/blob/e02ecfcc534e2021aae29077a958dd11c3897e4c/run-command.c#L275-L300

So I don't know if the problem is with running your editor, or running sh. There is sort of an assumption that StGit will be run in a context where git-for-windows will be run such that sh would be available. Again, I'm not a Windows user and don't have an environment on-hand, so I would again need help with further triage of this editor issue.

@char101
Copy link
Author

char101 commented Jan 25, 2024

Thanks for the links for the editor launch code. There are some points that I can make of it

  • the error: program not found is most likely caused by it not finding sh.exe, not the editor executable itself
  • the shell in Windows is cmd.exe, the command line to launch is cmd.exe /c <args>
  • even when installing git for Windows, there is an option to only expose git.exe on the PATH environment variable, so there is no guarantee that sh.exe is in the PATH
  • I tried to launch gvim.exe using cmd.exe /c gvim.exe and it did not wait for gvim.exe to terminate. I don't think launching an editor application in Windows requires a shell.
  • Using sh.exe to launch my editor did wait for it to terminate, but the application path need to use forward slashes.
[Thu Jan 25 21:09] C:\
> which git
C:\Program Files\Git\cmd\git.exe

[Thu Jan 25 21:15] C:\
> which sh
INFO: Could not find files for the given pattern(s).

[Thu Jan 25 21:16] C:\
> cmd.exe /c A:\Editor\vim\gvim.exe <- does not wait

[Thu Jan 25 21:20] C:\
> "C:\Program Files\Git\usr\bin\sh.exe" -c A:/Editor/vim/gvim.exe <- do wait, but requires forward slashes

If I add sh.exe to PATH, then the editor works

[Thu Jan 25 21:22] R:\test-git [master]
> set PATH=C:\Program Files\Git\usr\bin;%PATH%

[Thu Jan 25 21:22] R:\test-git [master]
> stg new patch-1
hint: Waiting for your editor to close the file... <- works, the gvim editor is displayed
> patch-1 (new)

@char101
Copy link
Author

char101 commented Jan 25, 2024

From https://stackoverflow.com/questions/10564/how-can-i-set-up-an-editor-to-work-with-git-on-windows

using cmd.exe /c start /wait <editor-path> do wait for the editor to terminate, so it might be an alternative to sh.exe

[Thu Jan 25 21:26] R:\test-git [master]
> cmd.exe /c start /wait A:\Editor\vim\gvim.exe <- wait for gvim.exe to teminate

@jpgrayson
Copy link
Collaborator

If I add sh.exe to PATH, then the editor works

I think this is the key. It's not just sh you're getting from C:\Program Files\Git\usr\bin, but also git itself. Although StGit does a lot with the gitoxide library, it also runs git subprocesses for a lot of things. I suspect that even if StGit didn't try to run the editor using sh, the next problem in line would have been StGit failing due to git not being in the PATH.

So from where I'm sitting, it seems like StGit using sh to wrap editor commands remains the correct approach. Note that using sh enables using shell capabilities for core.editor, such as referring to an environment variable. Using cmd.exe or executing the core.editor value directly would cause other incompatibilities with valid core.editor setups.

@char101
Copy link
Author

char101 commented Jan 25, 2024

git.exe is not installed in bin, but in cmd. This has been the (default?) mode for a long time. Adding all msys executables to the PATH might polute the PATH since it is a private git msys installation. In Windows, PATH is also used to load dlls unlike linux paths.

[Thu Jan 25 21:09] C:\
> which git
C:\Program Files\Git\cmd\git.exe

[Thu Jan 25 21:28] R:\test-git [master]
> dir "c:\Program Files\Git\cmd
 Volume in drive C is Win10
 Volume Serial Number is 7291-C0A8

 Directory of c:\Program Files\Git\cmd

2023/12/23  22:44    <DIR>          .
2023/12/23  22:44    <DIR>          ..
2023/11/20  18:47             1.038 aslr-manager.ps1
2023/11/20  18:35           136.200 git-gui.exe
2023/11/20  18:35            45.048 git-lfs.exe
2023/11/20  18:35            33.436 git-receive-pack.exe
2023/11/20  18:35            33.436 git-upload-pack.exe
2023/11/20  18:35            45.048 git.exe
2023/11/20  18:35           136.200 gitk.exe
2023/11/20  18:35            45.048 scalar.exe
2023/11/20  18:35             3.022 start-ssh-agent.cmd
2023/11/20  18:35             2.723 start-ssh-pageant.cmd
              10 File(s)        481.199 bytes
               2 Dir(s)  24.130.527.232 bytes free

@char101
Copy link
Author

char101 commented Jan 25, 2024

Git installer screenshot. The recommeded option is to only add git.exe and some other wrapper to PATH.

2024-01-25 22_11_20- #  Git 2 39 1 Setup  #

@char101
Copy link
Author

char101 commented Jan 25, 2024

As for git executables, it doesn't seem to be a problem

[Thu Jan 25 22:40] R:\test-git [master]
> stg log
427cdeb   Thu, 25 Jan 2024 22:39:32 +0700   refresh patch-1
5bbeebf   Thu, 25 Jan 2024 22:39:32 +0700   refresh refresh-temp (create temporary patch)
56db13b   Thu, 25 Jan 2024 21:22:42 +0700   new: patch-1
a005926   Thu, 25 Jan 2024 06:47:24 +0700   initialize

[Thu Jan 25 22:40] R:\test-git [master]
> which git-log
INFO: Could not find files for the given pattern(s).

[Thu Jan 25 22:40] R:\test-git [master]
> git show 427cdeb
commit 427cdebb2630946a90114b50149d00eb071af4a5
Author: Charles <peacech@gmail.com>
Date:   Thu Jan 25 22:39:32 2024 +0700

    refresh patch-1

@char101
Copy link
Author

char101 commented Jan 25, 2024

git.exe that is accessible from the PATH is just a wrapper that run the real git.exe. The real git.exe itself is not accessible from the PATH.

https://github.com/git-for-windows/MINGW-packages/blob/main/mingw-w64-git/git-wrapper.c

Byron added a commit to Byron/gitoxide that referenced this issue Jan 26, 2024
…home directory.

This will fix issues like the one described here:

stacked-git/stgit#407
Byron added a commit to Byron/gitoxide that referenced this issue Jan 26, 2024
…home directory.

This will fix issues like the one described here:

stacked-git/stgit#407
@Byron
Copy link
Contributor

Byron commented Jan 26, 2024

From this comment:

Thank you for the additional triage, @char101. Regarding how HOME is treated, I think that both StGit and gitoxide both strive to do what git does in these kind of situations. Perhaps @Byron would be interested in this issue.

Thanks for letting me know!

For launching programs 'git-style', gitoxide provides the gix-command crate, available under gix::command otherwise. It already has some custom behaviour related to the usage of shells, (hopefully) mimicking what Git does. I admit though that shell handling might be patched in Git for Windows, which is source code that I didn't yet look at.

If I am not mistaken, gix-command should be able to reproduce the way StGit currently launches the editor, and using it instead would have the benefit of automatically getting improvements to the Windows side of things. For reference, all of gitoxide already use gix-command exactly for that purpose, and it's already tuned to help with programs configured in the git configuration, which might be shell scripts but are best called directly if a shell isn't required.

For the problem of not picking up configuration from HOME, I have a fix in the works for which I will publish a patch-release of the respective crate. It should be picked up automatically once StGit gets a new release with this PR merged.

@jpgrayson
Copy link
Collaborator

Thank you @Byron for your help with this!

I will get the patched gix 0.58 merged into StGit soon.

And thank you for the tip about gix-command; I don't think it existed yet when I was porting StGit to Rust/gitoxide. I will look into using it in StGit.

@jpgrayson
Copy link
Collaborator

Updated to gix 0.58 in 861d26a.

This should fix the issue with finding the global .gitconfig in HOME on WIndows.

jpgrayson added a commit that referenced this issue Jan 30, 2024
gix-command emulate's git's behavior a little more closely w.r.t. when to execute the
editor with a shell and split arguments.

For example, using a shell script as the editor like this now works:

   GIT_EDITOR='printf "log: %s\n" "$1" >some.log && nvim -- "$1"' stg edit

This may also help with running the editor in a Windows environment.

Ref: #407
jpgrayson added a commit that referenced this issue Feb 2, 2024
This emulates more of git's Windows-specific behavior for launching
interactive editors.

When GIT_WINDOWS_NATIVE is defined, git uses `mingw_spawnvpe()` to spawn
the editor which, in turn, calls a `parse_interpreter()`. The proposed
editor command is run as-is if it has a .exe extension. Otherwise, the
command path is opened, read, and parsed to find a shebang line with an
interpreter path. This plays out in run-command.c and compat/mingw.c in
Git's source code.

The Windows CI is updated to set SHELL_PATH to something suitable for
the msys2 environment; `/bin/sh` is **not** a viable path in this
environment.

Refs: #407
@jpgrayson
Copy link
Collaborator

Hi @char101. With these recent changes to StGit and gix, I believe the StGit behaviors on Windows relevant to this issue are going to be different, and possibly better.

But not being a Windows user, I'm really flying blind here. It would be helpful if you might try out the latest msi build from this action run.


@Byron, I'd like to call your attention to the last commit I made to affect this problem (9329c3f). After integrating gix-command the obvious way in 048f182, everything was great on non-Windows platforms, but the Windows CI broke.

In StGit's test suites, there are certain places where shell scripts are dynamically generated and set as EDITOR; i.e. to help test stg edit. On Windows, gix-command would attempt to run such scripts directly, without using a shell; i.e. with ./tempscript as argv[0]. This, of course, fails because Windows doesn't know how to run a random file, especially one without an extension.

I took a look at what git does in this situation and found that it goes the extra mile to try to run the editor by reading the editor executable/script and parsing the first line for a shebang.

For StGit, I emulate this behavior. I imagine that gix-command may eventually want/need to do this as well to be at parity with git on Windows.

@char101
Copy link
Author

char101 commented Feb 2, 2024

Hi @jpgrayson, thanks for the update. I have tried running stg init and stg new patch (to trigger the editor) and both commands worked flawlessly (on Windows).

@jpgrayson
Copy link
Collaborator

Thanks for taking it for a spin @char101. Much appreciated. I'll be cutting a proper StGit release with these changes in the near future.

@Byron
Copy link
Contributor

Byron commented Feb 3, 2024

Thanks so much for sharing!

For StGit, I emulate this behavior. I imagine that gix-command may eventually want/need to do this as well to be at parity with git on Windows.

Indeed, that is something that gix-command ought to be doing. I will see to port the code from Git to be able to do the same, which should allow you to stick to use gix-command::Prep more trivially.
It's great to have StGit as downstream test-suite :) - I am a bit envious as well 😅.

jpgrayson added a commit that referenced this issue Feb 4, 2024
This version of gix-command gains the ability to parse shebang lines on Windows. This
replaces StGit code that did the same, but with perhaps more robustness.

Refs: #407
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

3 participants