Re: [PATCH] Extend runtime prefix computation
From: Junio C Hamano <hidden>
Date: 2016-06-16 02:18:48
Michael Weiser [off-list ref] writes:
Make git fully relocatable at runtime extending the runtime prefix calculation. Handle absolute and relative paths in argv0. Handle no path at all in argv0 in a system-specific manner. Replace assertions with initialised variables and checks that lead to fallback to the static prefix.
That's a dense description of "what" without saying much about "why". Hint: start by describing what case(s) the current code fails to find the correct runtime prefix. That would give readers a better understanding of what problem you are trying to solve. Missing sign-off.
quoted hunk
diff --git a/exec_cmd.c b/exec_cmd.c index 9d5703a..f0db070 100644 --- a/exec_cmd.c +++ b/exec_cmd.c@@ -3,30 +3,27 @@ #include "quote.h" #include "argv-array.h" #define MAX_ARGS 32 +#if defined(__APPLE__) +#include <mach-o/dyld.h> +#endif
We tend to avoid system specific includes in individual *.c files. Perhaps implement platform specific bits in compat/? E.g. each platform specific file in compat/ may implement and export the same git_extract_argv_path() function, perhaps, so that this file does not even need to know what platforms it supports?
char *system_path(const char *path)
{
-#ifdef RUNTIME_PREFIX
- static const char *prefix;
-#else
static const char *prefix = PREFIX;
-#endif
struct strbuf d = STRBUF_INIT;
if (is_absolute_path(path))
return xstrdup(path);
#ifdef RUNTIME_PREFIX
- assert(argv0_path);
- assert(is_absolute_path(argv0_path));Aren't these protecting against future and careless change that forgets to call extract-argv0-path or make that function return something that is not an absolute path? Is it a good idea to drop these checks, and if so why?
quoted hunk
- if (!prefix && - !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) && - !(prefix = strip_path_suffix(argv0_path, BINDIR)) && - !(prefix = strip_path_suffix(argv0_path, "git"))) { + if (!argv0_path || + !is_absolute_path(argv0_path) || + (!(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) && + !(prefix = strip_path_suffix(argv0_path, BINDIR)) && + !(prefix = strip_path_suffix(argv0_path, "git")))) { prefix = PREFIX; trace_printf("RUNTIME_PREFIX requested, " "but prefix computation failed. "@@ -41,6 +38,8 @@ char *system_path(const char *path) const char *git_extract_argv0_path(const char *argv0) { const char *slash; + char *to_resolve = NULL; + const char *resolved; if (!argv0 || !*argv0) return NULL;@@ -48,11 +47,56 @@ const char *git_extract_argv0_path(const char *argv0) slash = find_last_dir_sep(argv0); if (slash) { - argv0_path = xstrndup(argv0, slash - argv0); - return slash + 1; + /* ... it's either an absolute path */ + if (is_absolute_path(argv0)) { + argv0_path = xstrndup(argv0, slash - argv0); + return slash + 1; + } + + /* ... or a relative path, in which case we have to make it + * absolute first and do the whole thing again */ + to_resolve = xstrdup(argv0); + } else { + /* argv0 is no path at all, just a name. Resolve it into a + * path. Unfortunately, this gets system specific. */ +#if defined(__linux__) + struct stat st; + if (!stat("/proc/self/exe", &st)) + to_resolve = xstrdup("/proc/self/exe"); +#elif defined(__APPLE__) + /* call with len == 0 queries the necessary buffer size */ + uint32_t len = 0; + if(_NSGetExecutablePath(NULL, &len) != 0) { + to_resolve = malloc(len); + if (to_resolve) { + /* get the executable path now we have a buffer + * of proper size */ + if(_NSGetExecutablePath(to_resolve, &len) != 0) { + free(to_resolve); + return argv0; + } + } + } +#endif + + /* if to_resolve is still NULL here, something failed above or + * we are on an unsupported system. system_path() will warn + * and fall back to the static prefix */ + if (!to_resolve) + return argv0; } - return argv0; + /* resolve path from absolute to canonical */ + resolved = real_path(to_resolve); + free(to_resolve); + + slash = find_last_dir_sep(resolved); + if (!slash) + return argv0; + + argv0_path = xstrndup(resolved, slash - resolved); + slash = xstrdup(slash + 1); + return slash; } void git_set_argv_exec_path(const char *exec_path)