-
Notifications
You must be signed in to change notification settings - Fork 2.7k
mingw: take argv[0] from command line parameters #1561
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
Conversation
@dscho Initially I wanted to send this to the git mailing list, however, I noticed that mingw_startup is in git/git but msc_startup is not. |
Instead of basing argv[0] on the executable path, use argv[0] as provided on the command line. Setting argv[0] to git-<builtin> is currently the only way to prevent git from falling back to calling a helper executable. This change will allow the git-wrapper executable, that Git for Windows uses, to signal to git to handle the command as a builtin. Related to git-for-windows#1496 Signed-off-by: Kim Gybels <[email protected]>
@@ -3396,8 +3396,7 @@ int msc_startup(int argc, wchar_t **w_argv, wchar_t **w_env) | |||
*/ | |||
ALLOC_ARRAY(my_utf8_argv, argc + 1); | |||
ALLOC_ARRAY(save, argc + 1); | |||
save[0] = my_utf8_argv[0] = wcstoutfdup_startup(buffer, _wpgmptr, maxlen); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
GetModuleFileNameW is the function that I would recommend to use. That
returns the path for dlls or the current executable as absolute path.
…On Thu, Mar 15, 2018, 8:54 PM kgybels ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In compat/mingw.c
<#1561 (comment)>:
> @@ -3396,8 +3396,7 @@ int msc_startup(int argc, wchar_t **w_argv, wchar_t **w_env)
*/
ALLOC_ARRAY(my_utf8_argv, argc + 1);
ALLOC_ARRAY(save, argc + 1);
- save[0] = my_utf8_argv[0] = wcstoutfdup_startup(buffer, _wpgmptr, maxlen);
Just now, before I saw this comment, I ran into a crash because of it at
exec_cmd.c#L20 <https://github.com/git/git/blob/v2.16.2/exec_cmd.c#L20>,
when I did:
C:\git\bug\test>C:\PortableGit\mingw32\libexec\git-core\git-status.exe
I see that it needs git_extract_argv0_path()
<https://github.com/git/git/blob/v2.16.2/exec_cmd.c#L31> to set up
argv0_path, which was still NULL causing the crash.
It seems wrong to me to take the executable path and put it in argv[0], to
later use argv[0] to calculate the system prefix. If we want to know our
system prefix relative to our executable, then use the executable path
directly instead of using argv[0].
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1561 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHJ-yKMmG9ZBFWz9r8b09o12poXS8_Jeks5tew1bgaJpZM4Ss_rd>
.
|
Thanks for the suggestion! We already use |
Signed-off-by: Kim Gybels <[email protected]>
My apologies for delay - I paused that patch set pending a Perl Makefile
refactor, which landed. I have a rebased version almost ready to go, but
haven't been prioritizing it. Since there is some interest and I'm blocking
people, I'll try and find some time to push another set out soon. Sound
good?
…On Fri, Mar 16, 2018, 8:51 AM kgybels ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In compat/mingw.c
<#1561 (comment)>:
> @@ -3396,8 +3396,7 @@ int msc_startup(int argc, wchar_t **w_argv, wchar_t **w_env)
*/
ALLOC_ARRAY(my_utf8_argv, argc + 1);
ALLOC_ARRAY(save, argc + 1);
- save[0] = my_utf8_argv[0] = wcstoutfdup_startup(buffer, _wpgmptr, maxlen);
Any idea what effort remains for getting exec_cmd: RUNTIME_PREFIX on some
POSIX systems stable enough? I was refactoring in a similar direction (
***@***.***
<kgybels@dca5afd>),
however, @danjacques <https://github.com/danjacques>'s work is much more
evolved.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1561 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJgOMBchIyyKKCiQrymdxDJRkoa6O8FKks5te7VcgaJpZM4Ss_rd>
.
|
@danjacques very! |
@danjacques Sounds great! @dscho I will wait for the rebased version, then add my work on top. |
@dscho and @danjacques already did all the work (6e007ce), so I'm closing this pull request. |
Instead of basing argv[0] on the executable path, use argv[0] as
provided on the command line.
Setting argv[0] to git- is currently the only way to prevent
git from falling back to calling a helper executable. This change will
allow the git-wrapper executable, that Git for Windows uses, to signal
to git to handle the command as a builtin.
Related to #1496