threads / patch / 15056

patch, 7 partsRefactor git_set_argv0_path() to git_extract_argv0_path()

Subject: [PATCH 3/7] Refactor git_set_argv0_path() to git_extract_argv0_path()

## tl;dr

16 messages between Aug 17, 2008 and Sep 21, 2008. Diffs are folded; open one to read it.

replies: 15people: 4as markdown or json

Steffen Prohaska· Aug 17, 2008, 12:44 UTC · lore

[PATCH 0/7] prefix discovery at runtime (on Windows)

Hi, This patch series fixes discovery of the installation prefix at runtime on Windows. It introduces a compile flag RUNTIME_PREFIX, which can be set to explicitly request prefix computation at runtime.

Apologies for proposing such large changes that late in the release cycle. Maybe we want to postpone the series until 1.6.0.1 or even 1.6.1. Note however that in this case we should consider not releasing 1.6.0 on Windows because the current solution in master does not reliably read the system wide configuration on Windows.

We probably won't see a Windows installer before mid of September anyway, unless someone temporarily takes over the position of the msysgit maintainer until I return from holidays. I'll be completely offline from August 25 until September 13.

        Steffen
 Makefile       |   21 +++++++++++------
 daemon.c       |    3 ++
 exec_cmd.c     |   67 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 exec_cmd.h     |    2 +-
 fast-import.c  |    4 +++
 git.c          |   20 ++++------------
 hash-object.c  |    4 +++
 index-pack.c   |    4 +++
 receive-pack.c |    3 ++
 unpack-file.c  |    4 +++
 upload-pack.c  |    3 ++
 var.c          |    4 +++
 12 files changed, 107 insertions(+), 32 deletions(-)
 [PATCH 1/7] Windows: Add workaround for MSYS' path conversion
 [PATCH 2/7] system_path(): Add prefix computation at runtime if RUNTIME_PREFIX set
 [PATCH 3/7] Refactor git_set_argv0_path() to git_extract_argv0_path()
 [PATCH 4/7] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.
 [PATCH 5/7] Add calls to git_extract_argv0_path() in programs that call git_config_*
 [PATCH 6/7] Modify setup_path() to only add git_exec_path() to PATH
 [PATCH 7/7] Windows: Revert to default paths and convert them by RUNTIME_PREFIX
Steffen Prohaska· Aug 17, 2008, 12:44 UTC · re: Steffen Prohaska · lore

[PATCH 1/7] Windows: Add workaround for MSYS' path conversion

MSYS' automatic path conversion causes problems when passing paths as '-D' arguments to the compiler. MSYS tries to be smart and converts absolute paths to native Windows paths. But we want the paths as we compute them in the Makefile.

This commit adds a workaround by replacing "/" with its octal representation "\057", effectively hiding the path from MSYS. MSYS does no longer see the absolute path and therefore leaves it alone.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Makefile |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)
Show changes to Makefile +10 −4
diff --git a/Makefile b/Makefile
index 53ab4b5..9df5a9d 100644
--- a/Makefile
+++ b/Makefile
@@ -1042,6 +1042,12 @@ template_dir_SQ = $(subst ','\'',$(template_dir))
 htmldir_SQ = $(subst ','\'',$(htmldir))
 prefix_SQ = $(subst ','\'',$(prefix))
 
+ETC_GITCONFIG_SQ_C = $(subst /,\057,$(ETC_GITCONFIG_SQ))
+bindir_SQ_C = $(subst /,\057,$(bindir_SQ))
+gitexecdir_SQ_C = $(subst /,\057,$(gitexecdir_SQ))
+htmldir_SQ_C = $(subst /,\057,$(htmldir_SQ))
+template_dir_SQ_C = $(subst /,\057,$(template_dir_SQ))
+
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
@@ -1093,7 +1099,7 @@ git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 
 help.o: help.c common-cmds.h GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
-		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
+		'-DGIT_HTML_PATH="$(htmldir_SQ_C)"' \
 		'-DGIT_MAN_PATH="$(mandir_SQ)"' \
 		'-DGIT_INFO_PATH="$(infodir_SQ)"' $<
 
@@ -1197,12 +1203,12 @@ git.o git.spec \
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) '-DGIT_EXEC_PATH="$(gitexecdir_SQ_C)"' -DBINDIR='"$(bindir_SQ_C)"' $<
 builtin-init-db.o: builtin-init-db.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"' $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ_C)"' $<
 
 config.o: config.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"' $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ_C)"' $<
 
 http.o: http.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DGIT_USER_AGENT='"git/$(GIT_VERSION)"' $<
-- 
1.6.0.rc3.22.g053fd
Steffen Prohaska· Aug 17, 2008, 12:44 UTC · re: Steffen Prohaska · lore

[PATCH 2/7] system_path(): Add prefix computation at runtime if RUNTIME_PREFIX set

This commit modifies system_path() to compute the prefix at runtime if explicitly requested to do so. If RUNTIME_PREFIX is defined, system_path() tries to strip known directories that executables can be located in from the path of the executable. If the path is successfully stripped it is used as the prefix.

We print errors if the runtime prefix computation fails. The user needs to know that the global configuration are not picked up, because this can cause unexpected behavior. If a user explicitly wants to ignore system wide paths, he can set GIT_CONFIG_NOSYSTEM, as our tests do.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Makefile   |    3 +++
 exec_cmd.c |   48 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 47 insertions(+), 4 deletions(-)
Show changes to 2 files +47 −4

Makefile, exec_cmd.c

diff --git a/Makefile b/Makefile
index 9df5a9d..8341558 100644
--- a/Makefile
+++ b/Makefile
@@ -982,6 +982,9 @@ ifdef INTERNAL_QSORT
 	COMPAT_CFLAGS += -DINTERNAL_QSORT
 	COMPAT_OBJS += compat/qsort.o
 endif
+ifdef RUNTIME_PREFIX
+	COMPAT_CFLAGS += -DRUNTIME_PREFIX
+endif
 
 ifdef THREADED_DELTA_SEARCH
 	BASIC_CFLAGS += -DTHREADED_DELTA_SEARCH
diff --git a/exec_cmd.c b/exec_cmd.c
index ce6741e..1622481 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -9,11 +9,51 @@ static const char *argv0_path;
 
 const char *system_path(const char *path)
 {
-	if (!is_absolute_path(path) && argv0_path) {
-		struct strbuf d = STRBUF_INIT;
-		strbuf_addf(&d, "%s/%s", argv0_path, path);
-		path = strbuf_detach(&d, NULL);
+#ifdef RUNTIME_PREFIX
+	static const char *prefix;
+
+	if (!argv0_path) {
+		fprintf(stderr, "RUNTIME_PREFIX requested for path '%s', "
+				"but argv0_path not set.\n", path);
+		return path;
+	}
+
+	if (!prefix) {
+		const char *strip[] = {
+			GIT_EXEC_PATH,
+			BINDIR,
+			0
+		};
+		const char **s;
+
+		for (s = strip; *s; s++) {
+			const char *sargv = argv0_path + strlen(argv0_path);
+			const char *ss = *s + strlen(*s);
+			while (argv0_path < sargv && *s < ss
+				&& (*sargv == *ss ||
+				    (is_dir_sep(*sargv) && is_dir_sep(*ss)))) {
+				sargv--;
+				ss--;
+			}
+			if (*s == ss) {
+				struct strbuf d = STRBUF_INIT;
+				strbuf_add(&d, argv0_path, sargv - argv0_path);
+				prefix = strbuf_detach(&d, NULL);
+				break;
+			}
+		}
 	}
+
+	if (!prefix) {
+		fprintf(stderr, "RUNTIME_PREFIX requested for path '%s', "
+				"but prefix computation failed.\n", path);
+		return path;
+	}
+
+	struct strbuf d = STRBUF_INIT;
+	strbuf_addf(&d, "%s/%s", prefix, path);
+	path = strbuf_detach(&d, NULL);
+#endif
 	return path;
 }
 
-- 
1.6.0.rc3.22.g053fd
Steffen Prohaska· Aug 17, 2008, 12:44 UTC · re: Steffen Prohaska · lore
From: Steve Haslam <shaslam@lastminute.com>

This commit moves the code that computes the dirname of argv[0] from git.c's main() to git_set_argv0_path() and renames the function to git_extract_argv0_path(). This makes the code in git.c's main less cluttered, and we can use the direname computation from other main() functions too.

[spr: split Steve's original commit and wrote new commit message. ]
Signed-off-by: Steve Haslam <shaslam@lastminute.com>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 exec_cmd.c |   15 +++++++++++++--
 exec_cmd.h |    2 +-
 git.c      |   20 +++++---------------
 3 files changed, 19 insertions(+), 18 deletions(-)
Show changes to 3 files +19 −18

exec_cmd.c, exec_cmd.h, git.c

diff --git a/exec_cmd.c b/exec_cmd.c
index 1622481..d84e9e9 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -57,9 +57,20 @@ const char *system_path(const char *path)
 	return path;
 }
 
-void git_set_argv0_path(const char *path)
+const char *git_extract_argv0_path(const char *argv0)
 {
-	argv0_path = path;
+	const char *slash = argv0 + strlen(argv0);
+
+	do
+		--slash;
+	while (slash >= argv0 && !is_dir_sep(*slash));
+
+	if (slash >= argv0) {
+		argv0_path = xstrndup(argv0, slash - argv0);
+		return slash + 1;
+	}
+
+	return argv0;
 }
 
 void git_set_argv_exec_path(const char *exec_path)
diff --git a/exec_cmd.h b/exec_cmd.h
index 594f961..392e903 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -2,7 +2,7 @@
 #define GIT_EXEC_CMD_H
 
 extern void git_set_argv_exec_path(const char *exec_path);
-extern void git_set_argv0_path(const char *path);
+extern const char* git_extract_argv0_path(const char *path);
 extern const char* git_exec_path(void);
 extern void setup_path(void);
 extern const char **prepare_git_cmd(const char **argv);
diff --git a/git.c b/git.c
index 37b1d76..b1eff4a 100644
--- a/git.c
+++ b/git.c
@@ -416,23 +416,13 @@ static void execv_dashed_external(const char **argv)
 
 int main(int argc, const char **argv)
 {
-	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
-	char *slash = (char *)cmd + strlen(cmd);
+	const char *cmd;
 	int done_alias = 0;
 
-	/*
-	 * Take the basename of argv[0] as the command
-	 * name, and the dirname as the default exec_path
-	 * if we don't have anything better.
-	 */
-	do
-		--slash;
-	while (cmd <= slash && !is_dir_sep(*slash));
-	if (cmd <= slash) {
-		*slash++ = 0;
-		git_set_argv0_path(cmd);
-		cmd = slash;
-	}
+	if (argv[0] && *argv[0])
+		cmd = git_extract_argv0_path(argv[0]);
+	else
+		cmd = "git-help";
 
 	/*
 	 * "git-xxxx" is the same as "git xxxx", but we obviously:
-- 
1.6.0.rc3.22.g053fd
Steffen Prohaska· Aug 17, 2008, 12:44 UTC · re: Steffen Prohaska · lore

[PATCH 4/7] Glean libexec path from argv[0] for git-upload-pack and git-receive-pack.

From: Steve Haslam <shaslam@lastminute.com>

If the user specified the full path to git-upload-pack as the -u option to "git clone" when cloning a remote repository, and git was not on the default PATH on the remote machine, git-upload-pack was failing to exec git-pack-objects.

By making the argv[0] path (if any) available to setup_path(), this will allow finding the "git" executable in the same directory as "git-upload-pack". The default built in to exec_cmd.c is to look for "git" in the ".../libexec/git-core" directory, but it is not installed there (any longer).

Much the same applies to invoking git-receive-pack from a non-PATH location using the "--exec" argument to "git push".

[ spr: split Steve's original commit into two commits. ]
Signed-off-by: Steve Haslam <shaslam@lastminute.com>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 receive-pack.c |    3 +++
 upload-pack.c  |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)
Show changes to 2 files +6 −0

receive-pack.c, upload-pack.c

diff --git a/receive-pack.c b/receive-pack.c
index d44c19e..3699b16 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -467,6 +467,9 @@ int main(int argc, char **argv)
 	int i;
 	char *dir = NULL;
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	argv++;
 	for (i = 1; i < argc; i++) {
 		char *arg = *argv++;
diff --git a/upload-pack.c b/upload-pack.c
index c911e70..086eff6 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -616,6 +616,9 @@ int main(int argc, char **argv)
 	int i;
 	int strict = 0;
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
 
-- 
1.6.0.rc3.22.g053fd
Steffen Prohaska· Aug 17, 2008, 12:44 UTC · re: Steffen Prohaska · lore

[PATCH 5/7] Add calls to git_extract_argv0_path() in programs that call git_config_*

Programs that use git_config need to find the global configuration. When runtime prefix computation is enabled, this requires that git_extract_argv0_path() is called early in the program's main().

This commit adds the necessary calls in the programs that use git_config.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 daemon.c      |    3 +++
 fast-import.c |    4 ++++
 hash-object.c |    4 ++++
 index-pack.c  |    4 ++++
 unpack-file.c |    4 ++++
 var.c         |    4 ++++
 6 files changed, 23 insertions(+), 0 deletions(-)
Show changes to 6 files +23 −0

daemon.c, fast-import.c, hash-object.c, index-pack.c, unpack-file.c, var.c

diff --git a/daemon.c b/daemon.c
index 8dcde73..172854e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1055,6 +1055,9 @@ int main(int argc, char **argv)
 	gid_t gid = 0;
 	int i;
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	/* Without this we cannot rely on waitpid() to tell
 	 * what happened to our children.
 	 */
diff --git a/fast-import.c b/fast-import.c
index 7089e6f..7789d60 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -150,6 +150,7 @@ Format of STDIN stream:
 #include "refs.h"
 #include "csum-file.h"
 #include "quote.h"
+#include "exec_cmd.h"
 
 #define PACK_ID_BITS 16
 #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
@@ -2394,6 +2395,9 @@ int main(int argc, const char **argv)
 {
 	unsigned int i, show_stats = 1;
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	setup_git_directory();
 	git_config(git_pack_config, NULL);
 	if (!pack_compression_seen && core_compression_seen)
diff --git a/hash-object.c b/hash-object.c
index 46c06a9..0f77a46 100644
--- a/hash-object.c
+++ b/hash-object.c
@@ -7,6 +7,7 @@
 #include "cache.h"
 #include "blob.h"
 #include "quote.h"
+#include "exec_cmd.h"
 
 static void hash_object(const char *path, enum object_type type, int write_object)
 {
@@ -65,6 +66,9 @@ int main(int argc, char **argv)
 	int hashstdin = 0;
 	int stdin_paths = 0;
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	git_config(git_default_config, NULL);
 
 	for (i = 1 ; i < argc; i++) {
diff --git a/index-pack.c b/index-pack.c
index 52064be..bd5983b 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -8,6 +8,7 @@
 #include "tree.h"
 #include "progress.h"
 #include "fsck.h"
+#include "exec_cmd.h"
 
 static const char index_pack_usage[] =
 "git index-pack [-v] [-o <index-file>] [{ ---keep | --keep=<msg> }] [--strict] { <pack-file> | --stdin [--fix-thin] [<pack-file>] }";
@@ -877,6 +878,9 @@ int main(int argc, char **argv)
 	struct pack_idx_entry **idx_objects;
 	unsigned char sha1[20];
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	git_config(git_index_pack_config, NULL);
 
 	for (i = 1; i < argc; i++) {
diff --git a/unpack-file.c b/unpack-file.c
index bcdc8bb..f8bfda7 100644
--- a/unpack-file.c
+++ b/unpack-file.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "blob.h"
+#include "exec_cmd.h"
 
 static char *create_temp_file(unsigned char *sha1)
 {
@@ -25,6 +26,9 @@ int main(int argc, char **argv)
 {
 	unsigned char sha1[20];
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	if (argc != 2)
 		usage("git-unpack-file <sha1>");
 	if (get_sha1(argv[1], sha1))
diff --git a/var.c b/var.c
index f1eb314..33457dc 100644
--- a/var.c
+++ b/var.c
@@ -4,6 +4,7 @@
  * Copyright (C) Eric Biederman, 2005
  */
 #include "cache.h"
+#include "exec_cmd.h"
 
 static const char var_usage[] = "git var [-l | <variable>]";
 
@@ -56,6 +57,9 @@ int main(int argc, char **argv)
 		usage(var_usage);
 	}
 
+	if (argv[0] && *argv[0])
+		git_extract_argv0_path(argv[0]);
+
 	setup_git_directory_gently(&nongit);
 	val = NULL;
 
-- 
1.6.0.rc3.22.g053fd
Steffen Prohaska· Aug 17, 2008, 12:44 UTC · re: Steffen Prohaska · lore

[PATCH 6/7] Modify setup_path() to only add git_exec_path() to PATH

Searching git programs only in the highest-priority location is sufficient. If the user explicitly overrides the default location (by --exec-path or GIT_EXEC_PATH), we can safely expect that the required programs are there.

This change allows testing of executables built with RUNTIME_PREFIX. Calling system_path(GIT_EXEC_PATH) is avoided if a higher-priority location is provided, which is the case for the tests.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 exec_cmd.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
Show changes to exec_cmd.c +1 −3
diff --git a/exec_cmd.c b/exec_cmd.c
index d84e9e9..63efe23 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -114,9 +114,7 @@ void setup_path(void)
 
 	strbuf_init(&new_path, 0);
 
-	add_path(&new_path, argv_exec_path);
-	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
-	add_path(&new_path, system_path(GIT_EXEC_PATH));
+	add_path(&new_path, git_exec_path());
 	add_path(&new_path, argv0_path);
 
 	if (old_path)
-- 
1.6.0.rc3.22.g053fd
Steffen Prohaska· Aug 17, 2008, 12:44 UTC · re: Steffen Prohaska · lore

[PATCH 7/7] Windows: Revert to default paths and convert them by RUNTIME_PREFIX

The RUNTIME_PREFIX mechanism allows us to use the default (absolute) paths on Windows too. Defining RUNTIME_PREFIX explicitly requests for translation of paths during runtime, depending on the path to the executable.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Makefile |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
Show changes to Makefile +1 −3
diff --git a/Makefile b/Makefile
index 8341558..8d57557 100644
--- a/Makefile
+++ b/Makefile
@@ -748,6 +748,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	SNPRINTF_RETURNS_BOGUS = YesPlease
 	NO_SVN_TESTS = YesPlease
 	NO_PERL_MAKEMAKER = YesPlease
+	RUNTIME_PREFIX = YesPlease
 	NO_POSIX_ONLY_PROGRAMS = YesPlease
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat
 	COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1
@@ -755,9 +756,6 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o compat/winansi.o
 	EXTLIBS += -lws2_32
 	X = .exe
-	gitexecdir = ../libexec/git-core
-	template_dir = ../share/git-core/templates/
-	ETC_GITCONFIG = ../etc/gitconfig
 endif
 ifneq (,$(findstring arm,$(uname_M)))
 	ARM_SHA1 = YesPlease
-- 
1.6.0.rc3.22.g053fd
Johannes Sixt· Aug 17, 2008, 20:28 UTC · re: Steffen Prohaska · lore

Re: [PATCH 6/7] Modify setup_path() to only add git_exec_path() to PATH

On Sonntag, 17. August 2008, Steffen Prohaska wrote:
> Searching git programs only in the highest-priority location is
> sufficient.  If the user explicitly overrides the default location
> (by --exec-path or GIT_EXEC_PATH), we can safely expect that the
> required programs are there.
...
Show 13 quoted lines
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -114,9 +114,7 @@ void setup_path(void)
>
>  	strbuf_init(&new_path, 0);
>
> -	add_path(&new_path, argv_exec_path);
> -	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
> -	add_path(&new_path, system_path(GIT_EXEC_PATH));
> +	add_path(&new_path, git_exec_path());
>  	add_path(&new_path, argv0_path);
>
>  	if (old_path)

I thought about a change like this, too. But I recall that this area has been changed several times. One thread I found here:

http://thread.gmane.org/gmane.comp.version-control.git/62472
See also the commit message of 77cb17e940, which introduces exec_cmd.*.

But probably the points raised there are already moot, and your change is good.

-- Hannes
Johannes Sixt· Aug 17, 2008, 20:43 UTC · re: Steffen Prohaska · lore

Re: [PATCH 2/7] system_path(): Add prefix computation at runtime if RUNTIME_PREFIX set

On Sonntag, 17. August 2008, Steffen Prohaska wrote:
Show 5 quoted lines
> This commit modifies system_path() to compute the prefix at runtime
> if explicitly requested to do so.  If RUNTIME_PREFIX is defined,
> system_path() tries to strip known directories that executables can
> be located in from the path of the executable.  If the path is
> successfully stripped it is used as the prefix.

An example would be helpful. From your description it is difficult to understand what is stripped from where.

Show 18 quoted lines
> diff --git a/exec_cmd.c b/exec_cmd.c
> index ce6741e..1622481 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -9,11 +9,51 @@ static const char *argv0_path;
>
>  const char *system_path(const char *path)
>  {
> -	if (!is_absolute_path(path) && argv0_path) {
> -		struct strbuf d = STRBUF_INIT;
> -		strbuf_addf(&d, "%s/%s", argv0_path, path);
> -		path = strbuf_detach(&d, NULL);
> +#ifdef RUNTIME_PREFIX
> +	static const char *prefix;
> +
> +	if (!argv0_path) {
> +		fprintf(stderr, "RUNTIME_PREFIX requested for path '%s', "
> +				"but argv0_path not set.\n", path);
If this happens, isn't this a logic error: assert(argv0_path)?
Show 27 quoted lines
> +		return path;
> +	}
> +
> +	if (!prefix) {
> +		const char *strip[] = {
> +			GIT_EXEC_PATH,
> +			BINDIR,
> +			0
> +		};
> +		const char **s;
> +
> +		for (s = strip; *s; s++) {
> +			const char *sargv = argv0_path + strlen(argv0_path);
> +			const char *ss = *s + strlen(*s);
> +			while (argv0_path < sargv && *s < ss
> +				&& (*sargv == *ss ||
> +				    (is_dir_sep(*sargv) && is_dir_sep(*ss)))) {
> +				sargv--;
> +				ss--;
> +			}
> +			if (*s == ss) {
> +				struct strbuf d = STRBUF_INIT;
> +				strbuf_add(&d, argv0_path, sargv - argv0_path);
> +				prefix = strbuf_detach(&d, NULL);
> +				break;
> +			}
> +		}

Doesn't this do a thing very similar to longest_ancestor_length() (which still does not work for Windows paths, yet)?

Show 5 quoted lines
>  	}
> +
> +	if (!prefix) {
> +		fprintf(stderr, "RUNTIME_PREFIX requested for path '%s', "
> +				"but prefix computation failed.\n", path);
Again a logic error?
Show 9 quoted lines
> +		return path;
> +	}
> +
> +	struct strbuf d = STRBUF_INIT;
> +	strbuf_addf(&d, "%s/%s", prefix, path);
> +	path = strbuf_detach(&d, NULL);
> +#endif
>  	return path;
>  }
Steffen Prohaska· Aug 18, 2008, 05:28 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/7] system_path(): Add prefix computation at runtime if RUNTIME_PREFIX set

On Aug 17, 2008, at 10:43 PM, Johannes Sixt wrote:
Show 22 quoted lines
> On Sonntag, 17. August 2008, Steffen Prohaska wrote:
>>
>> diff --git a/exec_cmd.c b/exec_cmd.c
>> index ce6741e..1622481 100644
>> --- a/exec_cmd.c
>> +++ b/exec_cmd.c
>> @@ -9,11 +9,51 @@ static const char *argv0_path;
>>
>> const char *system_path(const char *path)
>> {
>> -	if (!is_absolute_path(path) && argv0_path) {
>> -		struct strbuf d = STRBUF_INIT;
>> -		strbuf_addf(&d, "%s/%s", argv0_path, path);
>> -		path = strbuf_detach(&d, NULL);
>> +#ifdef RUNTIME_PREFIX
>> +	static const char *prefix;
>> +
>> +	if (!argv0_path) {
>> +		fprintf(stderr, "RUNTIME_PREFIX requested for path '%s', "
>> +				"but argv0_path not set.\n", path);
>
> If this happens, isn't this a logic error: assert(argv0_path)?
We could consider this a logic error.
Show 7 quoted lines
>> 	}
>> +
>> +	if (!prefix) {
>> +		fprintf(stderr, "RUNTIME_PREFIX requested for path '%s', "
>> +				"but prefix computation failed.\n", path);
>
> Again a logic error?
The user can move the executable, so it's not a logic error.
	Steffen
Johannes Sixt· Aug 17, 2008, 20:35 UTC · re: Steffen Prohaska · lore

Re: [PATCH 1/7] Windows: Add workaround for MSYS' path conversion

On Sonntag, 17. August 2008, Steffen Prohaska wrote:
> MSYS' automatic path conversion causes problems when passing paths as '-D'
> arguments to the compiler.  MSYS tries to be smart and converts absolute
> paths to native Windows paths.  But we want the paths as we compute them in
> the Makefile.

Huh? Doesn't the Makefile compute paths like "/c/path/to/git"? We certainly don't want to compile such paths into git that in Windows speak actually mean "c:/path/to/git" - git is not an MSYS program and wouldn't understand the former. Which form of conversion are you refering to?

-- Hannes
Steffen Prohaska· Sep 21, 2008, 07:48 UTC · re: Johannes Sixt · lore

Re: [PATCH 1/7] Windows: Add workaround for MSYS' path conversion

On Aug 17, 2008, at 10:35 PM, Johannes Sixt wrote:
Show 16 quoted lines
> On Sonntag, 17. August 2008, Steffen Prohaska wrote:
>> MSYS' automatic path conversion causes problems when passing paths  
>> as '-D'
>> arguments to the compiler.  MSYS tries to be smart and converts  
>> absolute
>> paths to native Windows paths.  But we want the paths as we compute  
>> them in
>> the Makefile.
>
> Huh? Doesn't the Makefile compute paths like "/c/path/to/git"? We  
> certainly
> don't want to compile such paths into git that in Windows speak  
> actually
> mean "c:/path/to/git" - git is not an MSYS program and wouldn't  
> understand
> the former. Which form of conversion are you refering to?

MSYS' automatic path conversion causes problems when passing paths as defines ('-D' arguments to the compiler). MSYS tries to be smart and converts absolute paths to native Windows paths, e.g. if MSYS sees "/bin" it converts it to "c:/msysgit/bin". But we want completely unmodified paths; e.g. if we set bindir in the Makefile to "/bin", the define BINDIR shall expand to "/bin". Conversion to absolute Windows path will takes place later, during runtime.

I'll update the commit message.
	Steffen
Junio C Hamano· Aug 17, 2008, 18:01 UTC · re: Steffen Prohaska · lore

Re: [PATCH 0/7] prefix discovery at runtime (on Windows)

Steffen Prohaska <prohaska@zib.de> writes:
> Apologies for proposing such large changes that late in the release cycle.
> Maybe we want to postpone the series until 1.6.0.1 or even 1.6.1.

Well, from the cursory look, it does not seem to be 1.6.0.1 material, even though it is possible to fork a topic at 1.6.0 and use the changes in 'next', then 'master', and eventually to 'maint' to produce 1.6.0.X, if all of this hapapens before 1.6.1.

I wouldn't mind at all if the binary distribution on Windows is based on "git.git plus port specific patchset that will eventually be sent upstream" like it used to be. In fact it might even be preferrable, as I won't be testing ports to that platform myself anyway.

Junio C Hamano· Sep 8, 2008, 22:50 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/7] prefix discovery at runtime (on Windows)

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> Steffen Prohaska <prohaska@zib.de> writes:
>
>> Apologies for proposing such large changes that late in the release cycle.
>> Maybe we want to postpone the series until 1.6.0.1 or even 1.6.1.
>
> Well, from the cursory look, it does not seem to be 1.6.0.1 material, even
> though it is possible to fork a topic at 1.6.0 and use the changes in
> 'next', then 'master', and eventually to 'maint' to produce 1.6.0.X, if
> all of this hapapens before 1.6.1.
>
> I wouldn't mind at all if the binary distribution on Windows is based on
> "git.git plus port specific patchset that will eventually be sent
> upstream" like it used to be.  In fact it might even be preferrable, as I
> won't be testing ports to that platform myself anyway.

If the depth difference between /usr/libexec/git-cat-file and /bin/git is the major source of this headache, I do not think it is unreasonable for the mingw git port to use "gitexecdir=$(bindir)" layout by default. After all, Windows users do not really care where bulk of things are, as long as they see one single clickable icon on the desktop, don't they?

Johannes Schindelin· Sep 9, 2008, 14:49 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/7] prefix discovery at runtime (on Windows)

Hi,
On Mon, 8 Sep 2008, Junio C Hamano wrote:
Show 23 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Steffen Prohaska <prohaska@zib.de> writes:
> >
> >> Apologies for proposing such large changes that late in the release 
> >> cycle. Maybe we want to postpone the series until 1.6.0.1 or even 
> >> 1.6.1.
> >
> > Well, from the cursory look, it does not seem to be 1.6.0.1 material, 
> > even though it is possible to fork a topic at 1.6.0 and use the 
> > changes in 'next', then 'master', and eventually to 'maint' to produce 
> > 1.6.0.X, if all of this hapapens before 1.6.1.
> >
> > I wouldn't mind at all if the binary distribution on Windows is based 
> > on "git.git plus port specific patchset that will eventually be sent 
> > upstream" like it used to be.  In fact it might even be preferrable, 
> > as I won't be testing ports to that platform myself anyway.
> 
> If the depth difference between /usr/libexec/git-cat-file and /bin/git 
> is the major source of this headache, I do not think it is unreasonable 
> for the mingw git port to use "gitexecdir=$(bindir)" layout by default.  
> After all, Windows users do not really care where bulk of things are, as 
> long as they see one single clickable icon on the desktop, don't they?

I think the main point is that we could (finally!) adopt a saner default on Unix: instead of hardcoding an absolute exec path, a relative would do. So I'd like to see this supported for Linux...

Ciao, Dscho

← back to recent threads