threads / patch / 17877

patch, 2 partsRUNTIME_PREFIX enhancements

Subject: [PATCH 0/2] RUNTIME_PREFIX enhancements

## tl;dr

20 messages between Feb 18, 2009 and Feb 20, 2009. Diffs are folded; open one to read it.

replies: 19people: 3as markdown or json

Johannes Schindelin· Feb 18, 2009, 15:10 UTC · lore

When I ran "make" in msysGit's /git/, I was greeted by a pretty unfriendly message about a RUNTIME_PREFIX that could not be determined.

Not understanding the code in exec_cmd.c, I went back to a comment I made to one of the iterations of the RUNTIME_PREFIX patch series (back when I spent a considerable amount of time to understand the code), and turned it into a patch myself.

I hope you will agree that the code introduced in 1/2 is more understandable than what is removed in 2/2 in favor of the former.

The real meat comes in patch 2/2:

The problem is that Windows will look in the current directory before looking in the PATH when it tries to execute a program. So it will find the executable C:\msysgit\git\git.exe and be unable to strip the suffices "libexec/git-core" or "bin".

I just added "git" (which should not hurt other users, but instead help them if they did not install Git but run it in-place).

Johannes Schindelin (2):
  Introduce the function strip_path_suffix()
  system_path(): simplify using strip_path_suffix(), and add suffix
    "git"
 cache.h    |    2 ++
 exec_cmd.c |   32 +++-----------------------------
 path.c     |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 29 deletions(-)
Johannes Schindelin· Feb 18, 2009, 15:11 UTC · re: Johannes Schindelin · lore

[PATCH 1/2] Introduce the function strip_path_suffix()

The function strip_path_suffix() will try to split the given path into prefix/suffix. The suffix has to be passed to the function, and if the path matches, the prefix is set.

Arbitrary runs of directory separators ("slashes") are assumed identical.
Example:
	strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
		"libexec///git-core", &prefix)
will set prefix to "C:\\msysgit" and return 0.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 cache.h |    2 ++
 path.c  |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)
Show changes to 2 files +35 −0

cache.h, path.c

diff --git a/cache.h b/cache.h
index 4e28c24..2186143 100644
--- a/cache.h
+++ b/cache.h
@@ -629,6 +629,8 @@ const char *make_nonrelative_path(const char *path);
 const char *make_relative_path(const char *abs, const char *base);
 int normalize_path_copy(char *dst, const char *src);
 int longest_ancestor_length(const char *path, const char *prefix_list);
+int strip_path_suffix(const char *path, const char *suffix,
+		const char **prefix);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index 4b9107f..947e5b7 100644
--- a/path.c
+++ b/path.c
@@ -499,3 +499,36 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
 
 	return max_len;
 }
+
+/* strip arbitrary amount of directory separators at end of path */
+static inline int chomp_trailing_dir_sep(const char *path, int len)
+{
+	while (len && is_dir_sep(path[len - 1]))
+		len--;
+	return len;
+}
+
+/* sets prefix if the suffix matches */
+int strip_path_suffix(const char *path, const char *suffix, const char **prefix)
+{
+	int path_len = strlen(path), suffix_len = strlen(suffix);
+
+	while (suffix_len) {
+		if (!path_len)
+			return 1;
+
+		if (is_dir_sep(path[path_len - 1])) {
+			if (!is_dir_sep(suffix[suffix_len - 1]))
+				return 1;
+			path_len = chomp_trailing_dir_sep(path, path_len);
+			suffix_len = chomp_trailing_dir_sep(suffix, suffix_len);
+		}
+		else if (path[--path_len] != suffix[--suffix_len])
+			return 1;
+	}
+
+	if (path_len && !is_dir_sep(path[path_len - 1]))
+		return 1;
+	*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));
+	return 0;
+}
-- 
1.6.1.1.825.g72a9f
Johannes Sixt· Feb 18, 2009, 19:39 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/2] Introduce the function strip_path_suffix()

Johannes Schindelin schrieb:
Show 12 quoted lines
> The function strip_path_suffix() will try to split the given path into
> prefix/suffix.  The suffix has to be passed to the function, and if the
> path matches, the prefix is set.
> 
> Arbitrary runs of directory separators ("slashes") are assumed identical.
> 
> Example:
> 
> 	strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
> 		"libexec///git-core", &prefix)
> 
> will set prefix to "C:\\msysgit" and return 0.

But you implemented it so that prefix is actually "C:\\msysgit/\\" (unless, of course, I'm reading the code wrong).

> +/* sets prefix if the suffix matches */
> +int strip_path_suffix(const char *path, const char *suffix, const char **prefix)

For a general purpose function, the API is very unnatural (and geared towards its only user in patch 2/2). I'd expect that the return value is the result or NULL on failure.

I would not raise this concern if this were a static function near its only call site.

Show 19 quoted lines
> +{
> +	int path_len = strlen(path), suffix_len = strlen(suffix);
> +
> +	while (suffix_len) {
> +		if (!path_len)
> +			return 1;
> +
> +		if (is_dir_sep(path[path_len - 1])) {
> +			if (!is_dir_sep(suffix[suffix_len - 1]))
> +				return 1;
> +			path_len = chomp_trailing_dir_sep(path, path_len);
> +			suffix_len = chomp_trailing_dir_sep(suffix, suffix_len);
> +		}
> +		else if (path[--path_len] != suffix[--suffix_len])
> +			return 1;
> +	}
> +
> +	if (path_len && !is_dir_sep(path[path_len - 1]))
> +		return 1;

Should strip_path_suffix("foo/bar", "foo/bar", &prefix) succeed and prefix be the empty string? This implementation says it should be so. That's just a question...

> +	*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));
> +	return 0;
> +}
-- Hannes
Johannes Schindelin· Feb 18, 2009, 21:13 UTC · re: Johannes Sixt · lore

Re: [PATCH 1/2] Introduce the function strip_path_suffix()

Hi,
On Wed, 18 Feb 2009, Johannes Sixt wrote:
Show 17 quoted lines
> Johannes Schindelin schrieb:
> > The function strip_path_suffix() will try to split the given path into 
> > prefix/suffix.  The suffix has to be passed to the function, and if 
> > the path matches, the prefix is set.
> > 
> > Arbitrary runs of directory separators ("slashes") are assumed 
> > identical.
> > 
> > Example:
> > 
> >  strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
> >   "libexec///git-core", &prefix)
> > 
> > will set prefix to "C:\\msysgit" and return 0.
> 
> But you implemented it so that prefix is actually "C:\\msysgit/\\" 
> (unless, of course, I'm reading the code wrong).
This is supposed to handle that case:
	*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));
Show 7 quoted lines
> > +/* sets prefix if the suffix matches */
> > +int strip_path_suffix(const char *path, const char *suffix, const char
> > **prefix)
> 
> For a general purpose function, the API is very unnatural (and geared 
> towards its only user in patch 2/2). I'd expect that the return value is 
> the result or NULL on failure.
Good point.  Will change.
Show 24 quoted lines
> > +{
> > +	int path_len = strlen(path), suffix_len = strlen(suffix);
> > +
> > +	while (suffix_len) {
> > +		if (!path_len)
> > +			return 1;
> > +
> > +		if (is_dir_sep(path[path_len - 1])) {
> > +			if (!is_dir_sep(suffix[suffix_len - 1]))
> > +				return 1;
> > +			path_len = chomp_trailing_dir_sep(path, path_len);
> > +			suffix_len = chomp_trailing_dir_sep(suffix,
> > suffix_len);
> > +		}
> > +		else if (path[--path_len] != suffix[--suffix_len])
> > +			return 1;
> > +	}
> > +
> > +	if (path_len && !is_dir_sep(path[path_len - 1]))
> > +		return 1;
> 
> Should strip_path_suffix("foo/bar", "foo/bar", &prefix) succeed and 
> prefix be the empty string? This implementation says it should be so. 
> That's just a question...
Yes, I assumed that that makes sense.

Ciao, Dscho

Johannes Sixt· Feb 18, 2009, 22:27 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/2] Introduce the function strip_path_suffix()

Johannes Schindelin schrieb:
Show 12 quoted lines
> On Wed, 18 Feb 2009, Johannes Sixt wrote:
>> Johannes Schindelin schrieb:
>>>  strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
>>>   "libexec///git-core", &prefix)
>>>
>>> will set prefix to "C:\\msysgit" and return 0.
>> But you implemented it so that prefix is actually "C:\\msysgit/\\" 
>> (unless, of course, I'm reading the code wrong).
> 
> This is supposed to handle that case:
> 
> 	*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));
Ah, I missed that final call of chomp_trailing_dir_sep.
-- Hannes
Johannes Schindelin· Feb 18, 2009, 23:18 UTC · re: Johannes Sixt · lore

Re: [PATCH 1/2] Introduce the function strip_path_suffix()

Hi,
On Wed, 18 Feb 2009, Johannes Sixt wrote:
Show 15 quoted lines
> Johannes Schindelin schrieb:
> > On Wed, 18 Feb 2009, Johannes Sixt wrote:
> > > Johannes Schindelin schrieb:
> > > >  strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
> > > >   "libexec///git-core", &prefix)
> > > >
> > > > will set prefix to "C:\\msysgit" and return 0.
> > > But you implemented it so that prefix is actually "C:\\msysgit/\\"
> > > (unless, of course, I'm reading the code wrong).
> > 
> > This is supposed to handle that case:
> > 
> >  *prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));
> 
> Ah, I missed that final call of chomp_trailing_dir_sep.

I have to admit that I was aware of the fact that it is easy to miss, but could not find a better way to present it.

Ciao, Dscho

Johannes Schindelin· Feb 19, 2009, 19:10 UTC · re: Johannes Sixt · lore

[PATCH v2 0/2] RUNTIME_PREFIX enhancements

The idea is to simplify the RUNTIME_PREFIX code, and to make Git handle the case where the binary was found in $ROOT/git/, too.

Changes since v1:
- changed the signature of strip_path_suffix() as suggested by Hannes:
	char *strip_path_suffix(const char *path, const char *suffix)
- added a test to test-path-utils
Johannes Schindelin (2):
  Introduce the function strip_path_suffix()
  system_path(): simplify using strip_path_suffix(), and add suffix
    "git"
 cache.h               |    1 +
 exec_cmd.c            |   33 ++++-----------------------------
 path.c                |   32 ++++++++++++++++++++++++++++++++
 t/t0060-path-utils.sh |    4 ++++
 test-path-utils.c     |    6 ++++++
 5 files changed, 47 insertions(+), 29 deletions(-)
Johannes Schindelin· Feb 19, 2009, 19:10 UTC · re: Johannes Sixt · lore

[PATCH v2 1/2] Introduce the function strip_path_suffix()

The function strip_path_suffix() will try to strip a given suffix from a given path. The suffix must start at a directory boundary (i.e. "core" is not a path suffix of "libexec/git-core", but "git-core" is).

Arbitrary runs of directory separators ("slashes") are assumed identical.
Example:
	strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
		"libexec///git-core", &prefix)
will set prefix to "C:\\msysgit" and return 0.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 cache.h               |    1 +
 path.c                |   32 ++++++++++++++++++++++++++++++++
 t/t0060-path-utils.sh |    4 ++++
 test-path-utils.c     |    6 ++++++
 4 files changed, 43 insertions(+), 0 deletions(-)
Show changes to 4 files +43 −0

cache.h, path.c, t/t0060-path-utils.sh, test-path-utils.c

diff --git a/cache.h b/cache.h
index c926bc7..1fa184d 100644
--- a/cache.h
+++ b/cache.h
@@ -630,6 +630,7 @@ const char *make_nonrelative_path(const char *path);
 const char *make_relative_path(const char *abs, const char *base);
 int normalize_path_copy(char *dst, const char *src);
 int longest_ancestor_length(const char *path, const char *prefix_list);
+char *strip_path_suffix(const char *path, const char *suffix);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index 4b9107f..2030bf1 100644
--- a/path.c
+++ b/path.c
@@ -499,3 +499,35 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
 
 	return max_len;
 }
+
+/* strip arbitrary amount of directory separators at end of path */
+static inline int chomp_trailing_dir_sep(const char *path, int len)
+{
+	while (len && is_dir_sep(path[len - 1]))
+		len--;
+	return len;
+}
+
+/* sets prefix if the suffix matches */
+char *strip_path_suffix(const char *path, const char *suffix)
+{
+	int path_len = strlen(path), suffix_len = strlen(suffix);
+
+	while (suffix_len) {
+		if (!path_len)
+			return NULL;
+
+		if (is_dir_sep(path[path_len - 1])) {
+			if (!is_dir_sep(suffix[suffix_len - 1]))
+				return NULL;
+			path_len = chomp_trailing_dir_sep(path, path_len);
+			suffix_len = chomp_trailing_dir_sep(suffix, suffix_len);
+		}
+		else if (path[--path_len] != suffix[--suffix_len])
+			return NULL;
+	}
+
+	if (path_len && !is_dir_sep(path[path_len - 1]))
+		return NULL;
+	return xstrndup(path, chomp_trailing_dir_sep(path, path_len));
+}
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 4ed1f0b..8336114 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -85,4 +85,8 @@ ancestor /foo/bar :://foo/.:: 4
 ancestor /foo/bar //foo/./::/bar 4
 ancestor /foo/bar ::/bar -1
 
+test_expect_success 'strip_path_suffix' '
+	test c:/msysgit = $(test-path-utils strip_path_suffix \
+		c:/msysgit/libexec//git-core libexec/git-core)
+'
 test_done
diff --git a/test-path-utils.c b/test-path-utils.c
index 5168a8e..d261398 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -26,6 +26,12 @@ int main(int argc, char **argv)
 		return 0;
 	}
 
+	if (argc == 4 && !strcmp(argv[1], "strip_path_suffix")) {
+		char *prefix = strip_path_suffix(argv[2], argv[3]);
+		printf("%s\n", prefix ? prefix : "(null)");
+		return 0;
+	}
+
 	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
 		argv[1] ? argv[1] : "(there was none)");
 	return 1;
-- 
1.6.2.rc1.380.g0299c
Johannes Schindelin· Feb 19, 2009, 20:12 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2 1/2] Introduce the function strip_path_suffix()

Hi,
On Thu, 19 Feb 2009, Johannes Schindelin wrote:
Show 12 quoted lines
> The function strip_path_suffix() will try to strip a given suffix from 
> a given path.  The suffix must start at a directory boundary (i.e. "core" 
> is not a path suffix of "libexec/git-core", but "git-core" is).
> 
> Arbitrary runs of directory separators ("slashes") are assumed identical.
> 
> Example:
> 
> 	strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
> 		"libexec///git-core", &prefix)
> 
> will set prefix to "C:\\msysgit" and return 0.
Aargh, of course I managed to forget to adjust the example.  Here it goes:
	prefix = strip_path_suffix("C:\\msysgit/\\libexec\\git-core",
			"libexec///git-core");
will set prefix to "C:\\msysgit".

Ciao, Dscho

Johannes Sixt· Feb 19, 2009, 20:26 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2 1/2] Introduce the function strip_path_suffix()

Johannes Schindelin schrieb:
> +/* sets prefix if the suffix matches */
/*
  * If path ends with suffix (complete path components), returns the
  * part before suffix (sans trailing directory separators).
  * Otherwise returns NULL.
  */
> +char *strip_path_suffix(const char *path, const char *suffix)
Looks good otherwise.
-- Hannes
Johannes Schindelin· Feb 19, 2009, 20:44 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 1/2] Introduce the function strip_path_suffix()

Hi,
On Thu, 19 Feb 2009, Johannes Sixt wrote:
Show 8 quoted lines
> Johannes Schindelin schrieb:
> > +/* sets prefix if the suffix matches */
> 
> /*
>  * If path ends with suffix (complete path components), returns the
>  * part before suffix (sans trailing directory separators).
>  * Otherwise returns NULL.
>  */

Thanks. It shows that I did not really have time to polish this properly today, does it not?

Ciao, Dscho

Johannes Schindelin· Feb 19, 2009, 19:10 UTC · re: Johannes Sixt · lore

[PATCH v2 2/2] system_path(): simplify using strip_path_suffix(), and add suffix "git"

At least for the author of this patch, the logic in system_path() was too hard to understand. Using the function strip_path_suffix() documents the idea of the code better.

The real change is to add the suffix "git", so that a runtime prefix will be computed correctly even when the executable was called in /git/ as is the case in msysGit (Windows insists to search the current directory before the PATH when looking for an executable).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 exec_cmd.c |   33 ++++-----------------------------
 1 files changed, 4 insertions(+), 29 deletions(-)
Show changes to exec_cmd.c +4 −29
diff --git a/exec_cmd.c b/exec_cmd.c
index f234066..217c125 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -23,35 +23,10 @@ const char *system_path(const char *path)
 	assert(argv0_path);
 	assert(is_absolute_path(argv0_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;
-				/* We also skip the trailing directory separator. */
-				assert(sargv - argv0_path - 1 >= 0);
-				strbuf_add(&d, argv0_path, sargv - argv0_path - 1);
-				prefix = strbuf_detach(&d, NULL);
-				break;
-			}
-		}
-	}
-
-	if (!prefix) {
+	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"))) {
 		prefix = PREFIX;
 		fprintf(stderr, "RUNTIME_PREFIX requested, "
 				"but prefix computation failed.  "
-- 
1.6.2.rc1.380.g0299c
Johannes Sixt· Feb 19, 2009, 20:39 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2 2/2] system_path(): simplify using strip_path_suffix(), and add suffix "git"

Johannes Schindelin schrieb:
> The real change is to add the suffix "git", so that a runtime prefix will
> be computed correctly even when the executable was called in /git/ as is
> the case in msysGit (Windows insists to search the current directory
> before the PATH when looking for an executable).

msysgit is important enough to be supported by upstream git, in particular since it is *the* user of RUNTIME_PREFIX, so that this special-casing is warranted. Therefore:

Acked-by: Johannes Sixt <j6t@kdbg.org>
-- Hannes
Johannes Schindelin· Feb 19, 2009, 20:48 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 2/2] system_path(): simplify using strip_path_suffix(), and add suffix "git"

Hi,
On Thu, 19 Feb 2009, Johannes Sixt wrote:
Show 11 quoted lines
> Johannes Schindelin schrieb:
> > The real change is to add the suffix "git", so that a runtime prefix will
> > be computed correctly even when the executable was called in /git/ as is
> > the case in msysGit (Windows insists to search the current directory
> > before the PATH when looking for an executable).
> 
> msysgit is important enough to be supported by upstream git, in particular
> since it is *the* user of RUNTIME_PREFIX, so that this special-casing is
> warranted. Therefore:
> 
> Acked-by: Johannes Sixt <j6t@kdbg.org>
Thanks.

Note, however, that IMHO RUNTIME_PREFIX would be good for non-Windows, too, as it would make Git relocatable.

Ciao, Dscho

Johannes Schindelin· Feb 18, 2009, 15:11 UTC · re: Johannes Schindelin · lore

[PATCH 2/2] system_path(): simplify using strip_path_suffix(), and add suffix "git"

At least for the author of this patch, the logic in system_path() was too hard to understand. Using the function strip_path_suffix() documents the idea of the code better.

The real change is to add the suffix "git", so that a runtime prefix will be computed correctly even when the executable was called in /git/ as is the case in msysGit (Windows insists to search the current directory before the PATH when looking for an executable).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 exec_cmd.c |   32 +++-----------------------------
 1 files changed, 3 insertions(+), 29 deletions(-)
Show changes to exec_cmd.c +3 −29
diff --git a/exec_cmd.c b/exec_cmd.c
index f234066..f89268b 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -23,35 +23,9 @@ const char *system_path(const char *path)
 	assert(argv0_path);
 	assert(is_absolute_path(argv0_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;
-				/* We also skip the trailing directory separator. */
-				assert(sargv - argv0_path - 1 >= 0);
-				strbuf_add(&d, argv0_path, sargv - argv0_path - 1);
-				prefix = strbuf_detach(&d, NULL);
-				break;
-			}
-		}
-	}
-
-	if (!prefix) {
+	if (!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 = PREFIX;
 		fprintf(stderr, "RUNTIME_PREFIX requested, "
 				"but prefix computation failed.  "
-- 
1.6.1.1.825.g72a9f
Johannes Sixt· Feb 18, 2009, 19:38 UTC · re: Johannes Schindelin · lore

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

Johannes Schindelin schrieb:
> When I ran "make" in msysGit's /git/, I was greeted by a pretty unfriendly 
> message about a RUNTIME_PREFIX that could not be determined.
;)
I have a patch in my private tree that removes the warning.

Actually I wouldn't mind the warning because it is only visible for developers. Unfortunately, it is poison for gitk (you know, Tcl/Tk treats any output on stderr as program failure...)

Show 9 quoted lines
> The real meat comes in patch 2/2:
> 
> The problem is that Windows will look in the current directory before 
> looking in the PATH when it tries to execute a program.  So it will find 
> the executable C:\msysgit\git\git.exe and be unable to strip the suffices 
> "libexec/git-core" or "bin".
> 
> I just added "git" (which should not hurt other users, but instead help 
> them if they did not install Git but run it in-place).

This only silences the warning, but there is no guarantee that the resulting git suite works because your msysgit developer may not have installed stuff in C:\msysgit\libexec\git-core, yet. What the patch does is exactly the same as if the compiled-in prefix that the warning mentions were used, which in the msysgit case on your machine happens to be C:\msysgit. Or am I missing something?

I think that the better solution is to remove the warning instead of introducing this special case suffix "git".

-- Hannes
Johannes Schindelin· Feb 18, 2009, 21:11 UTC · re: Johannes Sixt · lore

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

Hi,
On Wed, 18 Feb 2009, Johannes Sixt wrote:
Show 7 quoted lines
> Johannes Schindelin schrieb:
> > When I ran "make" in msysGit's /git/, I was greeted by a pretty unfriendly
> > message about a RUNTIME_PREFIX that could not be determined.
> 
> ;)
> 
> I have a patch in my private tree that removes the warning.

I briefly considered this. But my patch actually does something different than hiding an error condition. It copes with the very peculiar situation we have on Windows.

I still would like to see the warning if something goes wrong -- _outside_ /git/.

> This only silences the warning, but there is no guarantee that the 
> resulting git suite works because your msysgit developer may not have 
> installed stuff in C:\msysgit\libexec\git-core, yet.

The good thing: it still works. Why? Because Git actually does _not_ search the Git binaries in libexec/git-core. It adds that directory to PATH and lets the PATH lookup handle the searching. So it kind of works, but in a different manner than you think it works.

But that cannot be helped.

Ciao, Dscho

Johannes Sixt· Feb 18, 2009, 22:52 UTC · re: Johannes Schindelin · lore

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

Johannes Schindelin schrieb:
Show 9 quoted lines
> On Wed, 18 Feb 2009, Johannes Sixt wrote:
>> This only silences the warning, but there is no guarantee that the 
>> resulting git suite works because your msysgit developer may not have 
>> installed stuff in C:\msysgit\libexec\git-core, yet.
> 
> The good thing: it still works.  Why?  Because Git actually does _not_ 
> search the Git binaries in libexec/git-core.  It adds that directory to 
> PATH and lets the PATH lookup handle the searching.  So it kind of works, 
> but in a different manner than you think it works.

I see why it works: Because we add argv0_path to PATH. I had forgotten about this fact. It comes after git_exec_path(), and so we pick up the binaries and shell scripts from the build directory if there is nothing in $prefix/libexec/git-core.

It's sad that your patch requires the source to live in a directory named 'git'. But I don't know how to do it better. :-(

-- Hannes
Johannes Schindelin· Feb 18, 2009, 23:19 UTC · re: Johannes Sixt · lore

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

Hi,
On Wed, 18 Feb 2009, Johannes Sixt wrote:
> It's sad that your patch requires the source to live in a directory 
> named 'git'. But I don't know how to do it better. :-(
Let's move it to /bin/

Ducks, Dscho

← back to recent threads