# [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails 14 messages from 2010-11-29 to 2010-12-03. Participants: Jeremy Huddleston, Thiago Farina, Jonathan Nieder, Junio C Hamano, Kevin Ballard. Thread: https://gitlist.dev/t/25870 ## Jeremy Huddleston, 2010-11-29 16:57 Subject: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <051964C9-0507-4CCB-A111-55CA36652F00@apple.com> URL: https://gitlist.dev/e/051964C9-0507-4CCB-A111-55CA36652F00%40apple.com ``` Signed-off-by: Jeremy Huddleston Reviewed-by: Matt Wright --- exec_cmd.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index bf22570..1e24a8f 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -3,6 +3,10 @@ #include "quote.h" #define MAX_ARGS 32 +#if defined(__APPLE__) && defined(RUNTIME_PREFIX) +#include +#endif + extern char **environ; static const char *argv_exec_path; static const char *argv0_path; @@ -53,6 +57,19 @@ const char *git_extract_argv0_path(const char *argv0) if (slash >= argv0) { argv0_path = xstrndup(argv0, slash - argv0); return slash + 1; +#ifdef __APPLE__ + } else { + char new_argv0[PATH_MAX]; + uint32_t new_argv0_s = PATH_MAX; + if(_NSGetExecutablePath(new_argv0, &new_argv0_s) == 0) { + slash = new_argv0 + new_argv0_s; + while (new_argv0 <= slash && !is_dir_sep(*slash)) + slash--; + + if (slash >= new_argv0) + argv0_path = xstrndup(new_argv0, slash - new_argv0); + } +#endif } return argv0; -- 1.7.3.2 ``` ## Thiago Farina, 2010-11-29 17:09 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: URL: https://gitlist.dev/e/AANLkTimwRJqje1-HhzKj-L-5-2CvhTC0%2BPr0Cvj7d_kc%40mail.gmail.com In-Reply-To: <051964C9-0507-4CCB-A111-55CA36652F00@apple.com> ``` On Mon, Nov 29, 2010 at 2:57 PM, Jeremy Huddleston wrote: > > Signed-off-by: Jeremy Huddleston > Reviewed-by: Matt Wright > --- >  exec_cmd.c |   17 +++++++++++++++++ >  1 files changed, 17 insertions(+), 0 deletions(-) > > diff --git a/exec_cmd.c b/exec_cmd.c > index bf22570..1e24a8f 100644 > --- a/exec_cmd.c > +++ b/exec_cmd.c > @@ -3,6 +3,10 @@ >  #include "quote.h" >  #define MAX_ARGS       32 > > +#if defined(__APPLE__) && defined(RUNTIME_PREFIX) > +#include > +#endif > + >  extern char **environ; >  static const char *argv_exec_path; >  static const char *argv0_path; > @@ -53,6 +57,19 @@ const char *git_extract_argv0_path(const char *argv0) >        if (slash >= argv0) { >                argv0_path = xstrndup(argv0, slash - argv0); >                return slash + 1; > +#ifdef __APPLE__ Why not #if defined(__APPLE__), like above? ``` ## Jonathan Nieder, 2010-11-29 17:12 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <20101129171211.GL8037@burratino> URL: https://gitlist.dev/e/20101129171211.GL8037%40burratino In-Reply-To: ``` Thiago Farina wrote: > On Mon, Nov 29, 2010 at 2:57 PM, Jeremy Huddleston wrote: >> Signed-off-by: Jeremy Huddleston >> Reviewed-by: Matt Wright I like the idea, but could you add a short commit message explaining the existing behavior and what improvement this makes? > Why not #if defined(__APPLE__), like above? More importantly, please search for #ifdef in existing code to get some examples of how we like to do platform-specific things. The section "2) #ifdefs are ugly" of linux-2.6/Documentation/SubmittingPatches explains the rationale. Regards, Jonathan ``` ## Jeremy Huddleston, 2010-11-29 18:29 Subject: [PATCH updated] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: URL: https://gitlist.dev/e/DD9829B7-8851-41CF-BCC6-2676B6015028%40apple.com In-Reply-To: <20101129171211.GL8037@burratino> ``` This adds better support for RUNTIME_PREFIX on Mac OS X. The previous codepath would only work if argv[0] contained the full path to the executable or $PATH already contained /path/to/libexec/git-core. We use _NSGetExecutablePath here to find the full path (and thus prepend the correct libexec/git-core to $PATH) in the case where argv[0] does not contain the full path to the executable. Signed-off-by: Jeremy Huddleston Reviewed-by: Matt Wright --- exec_cmd.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index bf22570..182fd3a 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -3,6 +3,10 @@ #include "quote.h" #define MAX_ARGS 32 +#if defined(__APPLE__) && defined(RUNTIME_PREFIX) +#include +#endif + extern char **environ; static const char *argv_exec_path; static const char *argv0_path; @@ -53,6 +57,19 @@ const char *git_extract_argv0_path(const char *argv0) if (slash >= argv0) { argv0_path = xstrndup(argv0, slash - argv0); return slash + 1; +#if defined(__APPLE__) + } else { + char new_argv0[PATH_MAX]; + uint32_t new_argv0_s = PATH_MAX; + if(_NSGetExecutablePath(new_argv0, &new_argv0_s) == 0) { + slash = new_argv0 + strlen(new_argv0); + while (new_argv0 <= slash && !is_dir_sep(*slash)) + slash--; + + if (slash >= new_argv0) + argv0_path = xstrndup(new_argv0, slash - new_argv0); + } +#endif } return argv0; -- 1.7.3.2 ``` ## Jeremy Huddleston, 2010-11-29 18:34 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <37CD335A-00A7-4074-BD39-4FCDEEB3D083@apple.com> URL: https://gitlist.dev/e/37CD335A-00A7-4074-BD39-4FCDEEB3D083%40apple.com In-Reply-To: <20101129171211.GL8037@burratino> ``` On Nov 29, 2010, at 12:12, Jonathan Nieder wrote: > Thiago Farina wrote: >> On Mon, Nov 29, 2010 at 2:57 PM, Jeremy Huddleston wrote: > >>> Signed-off-by: Jeremy Huddleston >>> Reviewed-by: Matt Wright > > I like the idea, but could you add a short commit message > explaining the existing behavior and what improvement this makes? Hopefully what I resent is sufficient for explaining the changes. >> Why not #if defined(__APPLE__), like above? Originally for style. I like #ifdef better than #if defined(), but I changed it for you in the resend. > More importantly, please search for #ifdef in existing code to get > some examples of how we like to do platform-specific things. Yeah, I see some: #ifdef _WIN32 which is why I used __APPLE__. Do you have a better suggestion? > The section "2) #ifdefs are ugly" of > linux-2.6/Documentation/SubmittingPatches explains the rationale. I agree, but I don't really see a way around it here since this API is specific to OS X. ``` ## Jonathan Nieder, 2010-11-29 18:49 Subject: Re: [PATCH updated] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <20101129184929.GA9441@burratino> URL: https://gitlist.dev/e/20101129184929.GA9441%40burratino In-Reply-To: ``` Jeremy Huddleston wrote: > This adds better support for RUNTIME_PREFIX on Mac OS X. The previous codepath > would only work if argv[0] contained the full path to the executable or $PATH > already contained /path/to/libexec/git-core. We use _NSGetExecutablePath here > to find the full path (and thus prepend the correct libexec/git-core to $PATH) > in the case where argv[0] does not contain the full path to the executable. Closer. But that is perhaps too much at the level of code rather than the user: Subject: MacOSX: Use _NSGetExecutablePath to get full argv[0] path When RUNTIME_PREFIX support is enabled (which is common on Mac OS X) the exec-path is derived from the program invocation path. Unfortunately, usual Unix semantics are for argv[0] to contain the path used to invoke a program rather than the path to the executable. So usual invocations of git would not result in helpers from exec-path being found correctly: $ git fast-import ... example output here ... So in the spirit of v1.6.0-rc1~21 (Windows: make sure argv[0] has a path, 2008-07-21), use _NSGetExecutablePath to find the full path to the git binary, avoiding such trouble. > --- a/exec_cmd.c > +++ b/exec_cmd.c [...] > @@ -53,6 +57,19 @@ const char *git_extract_argv0_path(const char *argv0) > if (slash >= argv0) { > argv0_path = xstrndup(argv0, slash - argv0); > return slash + 1; > +#if defined(__APPLE__) > + } else { > + char new_argv0[PATH_MAX]; > + uint32_t new_argv0_s = PATH_MAX; > + if(_NSGetExecutablePath(new_argv0, &new_argv0_s) == 0) { > + slash = new_argv0 + strlen(new_argv0); > + while (new_argv0 <= slash && !is_dir_sep(*slash)) > + slash--; > + > + if (slash >= new_argv0) > + argv0_path = xstrndup(new_argv0, slash - new_argv0); > + } > +#endif Can't this ifdef be avoided? The ideal is for such code to be abstracted away into helper functions in git-compat-util.h and compat/*.c. Jonathan ``` ## Jonathan Nieder, 2010-11-29 18:50 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <20101129185030.GB9441@burratino> URL: https://gitlist.dev/e/20101129185030.GB9441%40burratino In-Reply-To: <37CD335A-00A7-4074-BD39-4FCDEEB3D083@apple.com> ``` Jeremy Huddleston wrote: > On Nov 29, 2010, at 12:12, Jonathan Nieder wrote: >> The section "2) #ifdefs are ugly" of >> linux-2.6/Documentation/SubmittingPatches explains the rationale. > > I agree, but I don't really see a way around it here since this API is specific to OS X. Did you actually read that section? :) ``` ## Jeremy Huddleston, 2010-11-29 20:07 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: URL: https://gitlist.dev/e/BED9E026-6804-4ABF-A26D-8F20699BAD0B%40apple.com In-Reply-To: <20101129185030.GB9441@burratino> ``` On Nov 29, 2010, at 13:50, Jonathan Nieder wrote: > Jeremy Huddleston wrote: >> On Nov 29, 2010, at 12:12, Jonathan Nieder wrote: > >>> The section "2) #ifdefs are ugly" of >>> linux-2.6/Documentation/SubmittingPatches explains the rationale. >> >> I agree, but I don't really see a way around it here since this API is specific to OS X. > > Did you actually read that section? :) Yes, but I don't have the time to "do it right" right now ... I'm contributing the patch that we are using back to the community in the spirit of OSS development, but I don't have the time resources currently to "do it right" at present. I'll come back to it once time allows if nobody else picks it up. Thanks, Jeremy ``` ## Jonathan Nieder, 2010-11-29 20:19 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <20101129201958.GA10334@burratino> URL: https://gitlist.dev/e/20101129201958.GA10334%40burratino In-Reply-To: ``` Jeremy Huddleston wrote: > On Nov 29, 2010, at 13:50, Jonathan Nieder wrote: >> Jeremy Huddleston wrote: >>> On Nov 29, 2010, at 12:12, Jonathan Nieder wrote: >>>> The section "2) #ifdefs are ugly" of >>>> linux-2.6/Documentation/SubmittingPatches explains the rationale. >>> >>> I agree, but I don't really see a way around it here since this API is specific to OS X. >> >> Did you actually read that section? :) > > Yes, but I don't have the time to "do it right" right now ... I'm > contributing the patch that we are using back to the community in > the spirit of OSS development, but I don't have the time resources > currently to "do it right" at present. I'll come back to it once > time allows if nobody else picks it up. Okay. Thanks for reporting. My guess is that the Windows version could be simplified, too, if we introduce a function to get the path to the binary. On Linux it should use "readlink /proc/$$/exe", on Darwin the function you pointed to, on Win32 _pgmptr, as a fallback look for argv[0] in $PATH if someone on another platform is interested. Jonathan ``` ## Junio C Hamano, 2010-11-29 20:24 Subject: Re: [PATCH updated] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <7veia32803.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7veia32803.fsf%40alter.siamese.dyndns.org In-Reply-To: <20101129184929.GA9441@burratino> ``` Jonathan Nieder writes: >> --- a/exec_cmd.c >> +++ b/exec_cmd.c > [...] >> @@ -53,6 +57,19 @@ const char *git_extract_argv0_path(const char *argv0) >> if (slash >= argv0) { >> argv0_path = xstrndup(argv0, slash - argv0); >> return slash + 1; >> +#if defined(__APPLE__) >> + } else { >> + char new_argv0[PATH_MAX]; >> + uint32_t new_argv0_s = PATH_MAX; >> + if(_NSGetExecutablePath(new_argv0, &new_argv0_s) == 0) { >> + slash = new_argv0 + strlen(new_argv0); >> + while (new_argv0 <= slash && !is_dir_sep(*slash)) >> + slash--; >> + >> + if (slash >= new_argv0) >> + argv0_path = xstrndup(new_argv0, slash - new_argv0); >> + } >> +#endif > > Can't this ifdef be avoided? The ideal is for such code to be > abstracted away into helper functions in git-compat-util.h and compat/*.c. I had exactly the same reaction. Also doesn't the above need to be protected by defined(RUNTIME_PREFIX), too? ``` ## Jeremy Huddleston, 2010-11-29 21:02 Subject: Re: [PATCH updated] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <5873960D-848E-4039-B884-AA5B4E1787B8@apple.com> URL: https://gitlist.dev/e/5873960D-848E-4039-B884-AA5B4E1787B8%40apple.com In-Reply-To: <7veia32803.fsf@alter.siamese.dyndns.org> ``` On Nov 29, 2010, at 15:24, Junio C Hamano wrote: > Jonathan Nieder writes: > >>> --- a/exec_cmd.c >>> +++ b/exec_cmd.c >> [...] >>> @@ -53,6 +57,19 @@ const char *git_extract_argv0_path(const char *argv0) >>> if (slash >= argv0) { >>> argv0_path = xstrndup(argv0, slash - argv0); >>> return slash + 1; >>> +#if defined(__APPLE__) >>> + } else { >>> + char new_argv0[PATH_MAX]; >>> + uint32_t new_argv0_s = PATH_MAX; >>> + if(_NSGetExecutablePath(new_argv0, &new_argv0_s) == 0) { >>> + slash = new_argv0 + strlen(new_argv0); >>> + while (new_argv0 <= slash && !is_dir_sep(*slash)) >>> + slash--; >>> + >>> + if (slash >= new_argv0) >>> + argv0_path = xstrndup(new_argv0, slash - new_argv0); >>> + } >>> +#endif >> >> Can't this ifdef be avoided? The ideal is for such code to be >> abstracted away into helper functions in git-compat-util.h and compat/*.c. > > I had exactly the same reaction. Also doesn't the above need to be > protected by defined(RUNTIME_PREFIX), too? It already is inside of an #ifdef RUNTIME_PREFIX block. ``` ## Kevin Ballard, 2010-11-29 23:13 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <7534C13D-A52A-4BA4-B8C5-14E17CFA0351@sb.org> URL: https://gitlist.dev/e/7534C13D-A52A-4BA4-B8C5-14E17CFA0351%40sb.org In-Reply-To: <20101129171211.GL8037@burratino> ``` On Nov 29, 2010, at 9:12 AM, Jonathan Nieder wrote: > The section "2) #ifdefs are ugly" of > linux-2.6/Documentation/SubmittingPatches explains the rationale. Might this be worth pulling into git.git/Documentation/CodingGuidelines? -Kevin Ballard ``` ## Jonathan Nieder, 2010-12-03 07:42 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: <20101203074259.GB18202@burratino> URL: https://gitlist.dev/e/20101203074259.GB18202%40burratino In-Reply-To: <7534C13D-A52A-4BA4-B8C5-14E17CFA0351@sb.org> ``` Kevin Ballard wrote: > On Nov 29, 2010, at 9:12 AM, Jonathan Nieder wrote: >> The section "2) #ifdefs are ugly" of >> linux-2.6/Documentation/SubmittingPatches explains the rationale. > > Might this be worth pulling into git.git/Documentation/CodingGuidelines? Yes, the example there includes good advice I wish I had received sooner, and Documentation/CodingGuidelines seems like a good place to help people find it. Do you have some wording in mind? ``` ## Kevin Ballard, 2010-12-03 07:50 Subject: Re: [PATCH] Fallback on _NSGetExecutablePath to get the executable path if using argv[0] fails Message-ID: URL: https://gitlist.dev/e/CC3783DE-FE16-45E5-B53B-EC8AA133B876%40sb.org In-Reply-To: <20101203074259.GB18202@burratino> ``` On Dec 2, 2010, at 11:42 PM, Jonathan Nieder wrote: > Kevin Ballard wrote: >> On Nov 29, 2010, at 9:12 AM, Jonathan Nieder wrote: > >>> The section "2) #ifdefs are ugly" of >>> linux-2.6/Documentation/SubmittingPatches explains the rationale. >> >> Might this be worth pulling into git.git/Documentation/CodingGuidelines? > > Yes, the example there includes good advice I wish I had received > sooner, and Documentation/CodingGuidelines seems like a good place to > help people find it. Do you have some wording in mind? Not particularly. It just seems that if we're going to point people at the linux-2.6 documentation, then what we're referencing should be pulled into our own docs. It's not reasonable to expect people to read the documentation from another project to find out what they should do in this one. -Kevin Ballard ```