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

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

[Steffen Prohaska]

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

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

[Steffen Prohaska]

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

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

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

[Steffen Prohaska]

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

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

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

[Steffen Prohaska]

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

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

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

[Steffen Prohaska]

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

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

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

[Steffen Prohaska]

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

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

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

[Steffen Prohaska]

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

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

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

[Steffen Prohaska]

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

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

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

[Junio C Hamano]

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.

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

[Johannes Sixt]

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

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

[Johannes Sixt]

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

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

[Johannes Sixt]

On Sonntag, 17. August 2008, Steffen Prohaska wrote:
> 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.

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

> +		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)?

>  	}
> +
> +	if (!prefix) {
> +		fprintf(stderr, "RUNTIME_PREFIX requested for path '%s', "
> +				"but prefix computation failed.\n", path);

Again a logic error?

> +		return path;
> +	}
> +
> +	struct strbuf d = STRBUF_INIT;
> +	strbuf_addf(&d, "%s/%s", prefix, path);
> +	path = strbuf_detach(&d, NULL);
> +#endif
>  	return path;
>  }

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

[Steffen Prohaska]

On Aug 17, 2008, at 10:43 PM, Johannes Sixt wrote:

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


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

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

[Junio C Hamano]

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?

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

[Johannes Schindelin]

Hi,

On Mon, 8 Sep 2008, Junio C Hamano wrote:

> 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

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

[Steffen Prohaska]

On Aug 17, 2008, at 10:35 PM, Johannes Sixt wrote:

> 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

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

[Steffen Prohaska]

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

diff --git a/Makefile b/Makefile
index 8181f74..98278f0 100644
--- a/Makefile
+++ b/Makefile
@@ -767,6 +767,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
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch
@@ -775,9 +776,6 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/regex/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.2.GIT

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

[Johannes Sixt]

On Sonntag, 21. September 2008, Steffen Prohaska wrote:
> 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(-)
>
> diff --git a/Makefile b/Makefile
> index 8181f74..98278f0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -767,6 +767,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
>  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
>  	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex
> -Icompat/fnmatch @@ -775,9 +776,6 @@ ifneq (,$(findstring
> MINGW,$(uname_S)))
>  	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o
> compat/regex/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

This cannot be the complete patch. The part that sets $(prefix) to the empty 
string is missing, otherwise the runtime prefix discovery would never 
trigger, right? (But see also my comment on [PATCH 1/7].)

-- Hannes

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

[Steffen Prohaska]

On Sep 21, 2008, at 11:58 PM, Johannes Sixt wrote:

> On Sonntag, 21. September 2008, Steffen Prohaska wrote:
>> 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(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 8181f74..98278f0 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -767,6 +767,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
>> 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
>> 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/ 
>> regex
>> -Icompat/fnmatch @@ -775,9 +776,6 @@ ifneq (,$(findstring
>> MINGW,$(uname_S)))
>> 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o
>> compat/regex/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
>
> This cannot be the complete patch. The part that sets $(prefix) to  
> the empty
> string is missing, otherwise the runtime prefix discovery would never
> trigger, right? (But see also my comment on [PATCH 1/7].)

Right.  There are still some differences between msysgit's Makefile and
the official one, so I forgot to include all necessary changes.

I'll modify the implementation as you propose in your comment
on [PATCH 1/7].

	Steffen