[PATCH 0/2] RUNTIME_PREFIX enhancements

[PATCH 0/2] RUNTIME_PREFIX enhancements

[Johannes Schindelin]

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(-)

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

[Johannes Schindelin]

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(-)

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

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

[Johannes Schindelin]

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(-)

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

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

[Johannes Sixt]

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...)

> 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

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

[Johannes Sixt]

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).

> +/* 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.

> +{
> +	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

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

[Johannes Schindelin]

Hi,

On Wed, 18 Feb 2009, Johannes Sixt wrote:

> 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

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

[Johannes Schindelin]

Hi,

On Wed, 18 Feb 2009, Johannes Sixt wrote:

> 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));

> > +/* 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.

> > +{
> > +	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

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

[Johannes Sixt]

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.

-- Hannes

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

[Johannes Sixt]

Johannes Schindelin schrieb:
> 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

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

[Johannes Schindelin]

Hi,

On Wed, 18 Feb 2009, Johannes Sixt wrote:

> 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

Re: [PATCH 0/2] RUNTIME_PREFIX enhancements

[Johannes Schindelin]

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

[PATCH v2 0/2] RUNTIME_PREFIX enhancements

[Johannes Schindelin]

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(-)

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

[Johannes Schindelin]

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(-)

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

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

[Johannes Schindelin]

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(-)

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

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

[Johannes Schindelin]

Hi,

On Thu, 19 Feb 2009, Johannes Schindelin wrote:

> 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

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

[Johannes Sixt]

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

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

[Johannes Sixt]

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

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

[Johannes Schindelin]

Hi,

On Thu, 19 Feb 2009, Johannes Sixt wrote:

> 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

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

[Johannes Schindelin]

Hi,

On Thu, 19 Feb 2009, Johannes Sixt wrote:

> 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

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

[Junio C Hamano]

Thanks; both patches applied.