Extend runtime prefix computation

PATCH In review Michael Weiser 3 participants 11 messages Jun 15, 2016

Share & tools

Michael Weiser

author

2016-06-15 22:55 UTC #0 lore raw

PATCH 1/1 Extend runtime prefix computation

Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path). Implement fallback to compiled-in prefix if determination fails or is impossible.

Signed-off-by Michael Weiser <[email protected]>

---
- Has two very minor memory leaks - function is called only once per program execution. Do we care? Alternative: Use static buffer instead.

 exec_cmd.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 53 insertions(+), 15 deletions(-)

exec_cmd.c +53 ~~-15~~

index 125fa6f..d50d7f8 100644
@@ -4,28 +4,22 @@
 #define MAX_ARGS	32
 
 static const char *argv_exec_path;
-static const char *argv0_path;
+static const char *argv0_path = NULL;
 
 const 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 path;
 
 #ifdef RUNTIME_PREFIX
-	assert(argv0_path);
-	assert(is_absolute_path(argv0_path));
-
-	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,20 +35,64 @@ const char *system_path(const char *path)
 const char *git_extract_argv0_path(const char *argv0)
 {
 	const char *slash;
+	char *abs_argv0 = NULL;
 
 	if (!argv0 || !*argv0)
 		return NULL;
 	slash = argv0 + strlen(argv0);
 
+	/* walk to the first slash from the end */
 	while (argv0 <= slash && !is_dir_sep(*slash))
 		slash--;
 
+	/* if there was a slash ... */
 	if (slash >= argv0) {
-		argv0_path = xstrndup(argv0, slash - argv0);
-		return slash + 1;
+		/* ... it's either an absolute path */
+		if (is_absolute_path(argv0)) {
+			/* FIXME: memory leak here */
+			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 */
+		abs_argv0 = xstrdup(real_path(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)) {
+			abs_argv0 = xstrdup(real_path("/proc/self/exe"));
+		}
+#elif defined(__APPLE__)
+		/* Mac OS X has realpath, which incidentally allocates its own
+		 * memory, which in turn is why we do all the xstrdup's in the
+		 * other cases. */
+		abs_argv0 = realpath(argv0, NULL);
+#endif
+
+		/* if abs_argv0 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 (!abs_argv0) {
+			argv0_path = NULL;
+			return argv0;
+		}
 	}
 
-	return argv0;
+	/* abs_argv0 is an absolute path now for which memory was allocated
+	 * with malloc */
+
+	slash = abs_argv0 + strlen(abs_argv0);
+	while (abs_argv0 <= slash && !is_dir_sep(*slash))
+		slash--;
+
+	/* FIXME: memory leaks here */
+	argv0_path = xstrndup(abs_argv0, slash - abs_argv0);
+	slash = xstrdup(slash + 1);
+	free(abs_argv0);
+	return slash;
 }
 
 void git_set_argv_exec_path(const char *exec_path)

Signature · 13 lines

1.7.3.4
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

Erik Faye-Lund

2016-06-15 22:55 UTC #1 lore raw

Quoted 83 lines · Michael Weiser

Signed-off-by Michael Weiser <[email protected]>

---
- Has two very minor memory leaks - function is called only once per program execution. Do we care? Alternative: Use static buffer instead.

 exec_cmd.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 53 insertions(+), 15 deletions(-)

exec_cmd.c +23 ~~-14~~

index 125fa6f..d50d7f8 100644
@@ -4,28 +4,22 @@
 #define MAX_ARGS       32
 
 static const char *argv_exec_path;
-static const char *argv0_path;
+static const char *argv0_path = NULL;
 
 const 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 path;
 
 #ifdef RUNTIME_PREFIX
-       assert(argv0_path);
-       assert(is_absolute_path(argv0_path));
-
-       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,20 +35,64 @@ const char *system_path(const char *path)
 const char *git_extract_argv0_path(const char *argv0)
 {
        const char *slash;
+       char *abs_argv0 = NULL;
 
        if (!argv0 || !*argv0)
                return NULL;
        slash = argv0 + strlen(argv0);
 
+       /* walk to the first slash from the end */
        while (argv0 <= slash && !is_dir_sep(*slash))
                slash--;
 
+       /* if there was a slash ... */
        if (slash >= argv0) {
-               argv0_path = xstrndup(argv0, slash - argv0);
-               return slash + 1;
+               /* ... it's either an absolute path */
+               if (is_absolute_path(argv0)) {
+                       /* FIXME: memory leak here */
+                       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 */
+               abs_argv0 = xstrdup(real_path(argv0));
+       } else {
+               /* argv0 is no path at all, just a name. Resolve it into a
+                * path. Unfortunately, this gets system specific. */
+#if defined(__linux__)

exec_cmd.c const char *system_path(const char *path) · line 73

Quoted 10 lines

exec_cmd.c +10 -0

+               struct stat st;
+               if (!stat("/proc/self/exe", &st)) {
+                       abs_argv0 = xstrdup(real_path("/proc/self/exe"));
+               }
+#elif defined(__APPLE__)
+               /* Mac OS X has realpath, which incidentally allocates its own
+                * memory, which in turn is why we do all the xstrdup's in the
+                * other cases. */
+               abs_argv0 = realpath(argv0, NULL);
+#endif

...perhaps a "GetModuleFileName(NULL, ...)" for Windows is in place here?

Michael Weiser

author

2016-06-15 22:55 UTC #2 lore raw

Hello Erik,

Quoted 4 lines · Erik Faye-Lund

Quoted 4 lines

exec_cmd.c +4 -0

+#if defined(__linux__)
+               struct stat st;
+               if (!stat("/proc/self/exe", &st)) {
+                       abs_argv0 = xstrdup(real_path("/proc/self/exe"));

Quoted 8 lines

exec_cmd.c const char *system_path(const char *path) · line 73

Quoted 7 lines

exec_cmd.c +7 -0

+               }
+#elif defined(__APPLE__)
+               /* Mac OS X has realpath, which incidentally allocates its own
+                * memory, which in turn is why we do all the xstrdup's in the
+                * other cases. */
+               abs_argv0 = realpath(argv0, NULL);
+#endif

...perhaps a "GetModuleFileName(NULL, ...)" for Windows is in place here?

Agreed. However, I do not use git on Windows and don't have a Windows devel toolchain in place. So I guess this should be added in a separate patch by someone actually in need of it and in a position to develop and test it?

Thanks,

Signature · 17 lines

Michael Weiser                science + computing ag
Senior Systems Engineer       Geschaeftsstelle Duesseldorf
                              Martinstrasse 47-55, Haus A
phone: +49 211 302 708 32     D-40223 Duesseldorf
fax:   +49 211 302 708 50     www.science-computing.de
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

Michael Weiser

author

2016-06-15 22:56 UTC #3 lore raw

In reply to Michael Weiser

Hello,

Quoted 3 lines · Michael Weiser

Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path). Implement fallback to compiled-in prefix if determination fails or is impossible.

I see this hasn't made it into git, yet. Is there any reason? What can I do to get it included?

Thanks,

Signature · 17 lines

Michael Weiser                science + computing ag
Senior Systems Engineer       Geschaeftsstelle Duesseldorf
                              Martinstrasse 47-55, Haus A
phone: +49 211 302 708 32     D-40223 Duesseldorf
fax:   +49 211 302 708 50     www.science-computing.de
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

Junio C Hamano

2016-06-15 22:56 UTC #4 lore raw

Quoted 7 lines · Michael Weiser

Quoted 3 lines · Michael Weiser
Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path). Implement fallback to compiled-in prefix if determination fails or is impossible.
I see this hasn't made it into git, yet. Is there any reason?

Most likely nobody was interested.

The default for any change is not to include it. Is there any reason why we want this change?

Michael Weiser

author

2016-06-15 22:56 UTC #5 lore raw

Hello Junio,

Quoted 5 lines · Junio C Hamano

Quoted 2 lines
Quoted 2 lines
Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path).
The default for any change is not to include it. Is there any reason why we want this change?

It makes a binary git installation fully relocatable. Seeing how git already has basic support for it I thought other people might be interested in this.

Thanks,

Signature · 17 lines

Michael Weiser                science + computing ag
Senior Systems Engineer       Geschaeftsstelle Duesseldorf
                              Martinstrasse 47-55, Haus A
phone: +49 211 302 708 32     D-40223 Duesseldorf
fax:   +49 211 302 708 50     www.science-computing.de
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

Michael Weiser

author

2016-06-15 22:56 UTC #6 lore raw

Hello Junio,
Hello list,

Quoted 7 lines · Michael Weiser

Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path).
The default for any change is not to include it. Is there any reason why we want this change?
It makes a binary git installation fully relocatable. Seeing how git already has basic support for it I thought other people might be interested in this.

I am still interested in getting this accepted into git. Where do I push to get it committed?

Thanks,

Signature · 17 lines

Michael Weiser                science + computing ag
Senior Systems Engineer       Geschaeftsstelle Duesseldorf
                              Martinstrasse 47-55, Haus A
phone: +49 211 302 708 32     D-40223 Duesseldorf
fax:   +49 211 302 708 50     www.science-computing.de
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

Junio C Hamano

2016-06-15 22:56 UTC #7 lore raw

Quoted 7 lines · nobody

Hello Junio,
Hello list,
Quoted 2 lines · Michael Weiser
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path).

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
The default for any change is not to include it. Is there any reason why we want this change?
It makes a binary git installation fully relocatable. Seeing how git already has basic support for it I thought other people might be interested in this.
I am still interested in getting this accepted into git. Where do I push to get it committed?

I do not have a strong objection to what it tries to achieve, but I'd prefer to see no "#ifdef platform" code in a very generic part of the system like exec_cmd.

Erik Faye-Lund

2016-06-15 22:56 UTC #8 lore raw

In reply to Michael Weiser

Quoted 7 lines · Michael Weiser

Hello Junio,
Quoted 2 lines · Junio C Hamano
Quoted 2 lines
Quoted 2 lines
Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path).

Quoted 6 lines

Quoted 2 lines
The default for any change is not to include it. Is there any reason why we want this change?
It makes a binary git installation fully relocatable. Seeing how git already has basic support for it I thought other people might be interested in this.

I think the motivation for the feature in the first place is Windows, where the installation path isn't known at build-time. In the unix-world, this is generally known (/usr/bin or something like that). What's the reason you want it on other platforms?

Michael Weiser

author

2016-06-15 22:56 UTC #9 lore raw

Hello Erik,

Quoted 6 lines · Erik Faye-Lund

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path).
I think the motivation for the feature in the first place is Windows, where the installation path isn't known at build-time. In the unix-world, this is generally known (/usr/bin or something like that). What's the reason you want it on other platforms?

It's part of an in-house development toolchain featuring git that I want to hand to users as a binary installation for a number of platforms but give them the choice where to install it.

Secondly, once the infrastructure is in place, it's easier to do or enhance relocatability support for other platforms such as Windows.

Finally: Others do it. Perl's done it and I've already needed that as well - on Mac OS X.

Thanks,

Signature · 17 lines

Michael Weiser                science + computing ag
Senior Systems Engineer       Geschaeftsstelle Duesseldorf
                              Martinstrasse 47-55, Haus A
phone: +49 211 302 708 32     D-40223 Duesseldorf
fax:   +49 211 302 708 50     www.science-computing.de
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196

Michael Weiser

author

2016-06-15 22:58 UTC #10 lore raw

Hi,

Quoted 8 lines · Michael Weiser

Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Support determining the binaries' installation path at runtime even if called without any path components (i.e. via search path).
What's the reason you want it on other platforms?
It's part of an in-house development toolchain featuring git that I want to hand to users as a binary installation for a number of platforms but give them the choice where to install it.

I'd still like to have this included in mainline git. What can I do to get it there?

Thanks,

Signature · 17 lines

Michael Weiser                science + computing ag
Senior Systems Engineer       Geschaeftsstelle Duesseldorf
                              Martinstrasse 47-55, Haus A
phone: +49 211 302 708 32     D-40223 Duesseldorf
fax:   +49 211 302 708 50     www.science-computing.de
-- 
Vorstandsvorsitzender/Chairman of the board of management:
Gerd-Lothar Leonhart
Vorstand/Board of Management:
Dr. Bernd Finkbeiner, Michael Heinrichs, 
Dr. Arno Steitz, Dr. Ingrid Zech
Vorsitzender des Aufsichtsrats/
Chairman of the Supervisory Board:
Philippe Miltin
Sitz/Registered Office: Tuebingen
Registergericht/Registration Court: Stuttgart
Registernummer/Commercial Register No.: HRB 382196