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-wrapper: use git-<command> to call builtin #23

Merged
merged 1 commit into from
May 18, 2018

Conversation

kgybels
Copy link

@kgybels kgybels commented Mar 15, 2018

Using a command line of the form: git-<command> <arguments>, will cause
git to to treat the command as a builtin, preventing git from calling an
external helper. Otherwise, git could call back to the wrapper leading
to an infinite mutual recursion.

Fixes git-for-windows/git#1496

@kgybels
Copy link
Author

kgybels commented Mar 15, 2018

@dscho For this to work we need the changes from git-for-windows/git#1561.

Using a command line of the form: git-<command> <arguments>, will cause
git to to treat the command as a builtin, preventing git from calling an
external helper. Otherwise, git could call back to the wrapper leading
to an infinite mutual recursion.

Fixes git-for-windows/git#1496

Signed-off-by: Kim Gybels <kgybels@infogroep.be>
@dscho
Copy link
Member

dscho commented May 18, 2018

Hmm. The way I read https://github.com/kgybels/MINGW-packages/blob/034f46f9599cf7242871562e75eb5113dc6bfcdc/mingw-w64-git/git-wrapper.c#L147, it would result in a command line git git-staTUS... am I wrong?

@dscho
Copy link
Member

dscho commented May 18, 2018

A quick test shows that the latest snapshot still displays the bug, so I am eager to get this fixed. But I need to understand why your fix works before merging ;-)

@kgybels
Copy link
Author

kgybels commented May 18, 2018

To handle builtin is_git_command is set to false.
(https://github.com/kgybels/MINGW-packages/blob/034f46f9599cf7242871562e75eb5113dc6bfcdc/mingw-w64-git/git-wrapper.c#L560)

This also means the command line will no longer have a full path, but this is not a problem because when calling CreateProcess the full path is passed in as the first argument in addition to the command line. (https://github.com/kgybels/MINGW-packages/blob/034f46f9599cf7242871562e75eb5113dc6bfcdc/mingw-w64-git/git-wrapper.c#L638)

@dscho
Copy link
Member

dscho commented May 18, 2018

To handle builtin is_git_command is set to false.

That's what I was missing! Thanks!

@dscho dscho merged commit c3929e0 into git-for-windows:master May 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants