[PATCH 0/9] Make gitexecdir relative to $(bindir) on Windows

[PATCH 0/9] Make gitexecdir relative to $(bindir) on Windows

[Johannes Sixt]

This is a revival of the series of a week or so ago.

I found a workaround for the problem that made me abondon the effort.
The problem was that argv[0] does not have a path in certain cases.
The workaround is in 8/9, namely to redefine main() and replace
argv[0] by the value of _pgmptr that the C runtime on Windows provides
and that always has a full path.

Steffen has suggested not to the path discovery for 'git-*' programs.
No change is necessary to do that because programs with a 'git-' prefix
already take the path via handle_internal_command() early.

9/9 actually is off-topic, but it depends on 2/9 because it replaces
the instance of git-shell in the ./check_bindir line by git-add. If
you think it's worthwhile, you could squash this hunk into 2/9.

I'll follow-up with 10/9, which is git-gui specific.

Johannes Sixt (9):
  Makefile: Do not install a copy of 'git' in $(gitexecdir)
  Makefile: Normalize $(bindir) and $(gitexecdir) before comparing
  Record the command invocation path early
  Fix relative built-in paths to be relative to the command invocation
  Allow the built-in exec path to be relative to the command invocation
    path
  Allow add_path() to add non-existent directories to the path
  Windows: Make $(gitexecdir) relative
  Windows: Make sure argv[0] has a path
  Windows: Do not compile git-shell

 Makefile       |   38 ++++++++++++++++++++++++--------------
 abspath.c      |   36 ++++++++++++++++++++++++++++++++++++
 compat/mingw.h |   12 ++++++++++++
 exec_cmd.c     |   54 +++++++++++++-----------------------------------------
 exec_cmd.h     |    3 ++-
 git.c          |    5 ++---
 path.c         |   36 ------------------------------------
 receive-pack.c |    2 +-
 shell.c        |    4 ++--
 upload-pack.c  |    2 +-
 10 files changed, 93 insertions(+), 99 deletions(-)

[PATCH 1/9] Makefile: Do not install a copy of 'git' in $(gitexecdir)

[Johannes Sixt]

There is already a copy in $(bindir). A subsequent patch will enable git
to derive the exec-path from its invocation path. If git is invoked
recursively, the first invocation puts the exec-path into PATH, so that
the recursive invocation would find the instance in the exec-path. This
second instance would again try to derive an exec-path from its invocation
path, but would base its result on the wrong "bindir".

We do install the copy of git first, but remove it later, so that we can
use it as the source of the hardlinks for the builtins.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 551bde9..cbab4f9 100644
--- a/Makefile
+++ b/Makefile
@@ -1335,6 +1335,7 @@ endif
 			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
 	fi
 	$(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
+	$(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'
 ifneq (,$X)
 	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
 endif
-- 
1.6.0.rc0.18.g6aef2

[PATCH 2/9] Makefile: Normalize $(bindir) and $(gitexecdir) before comparing

[Johannes Sixt]

The install target needs to check whether the user has opted to make
$(gitexecdir) equal to $(bindir). It did so by a straight string
comparison. Since we are going to allow a relative $(gitexecdir), we have
to normalize paths before comparison, which we do with $(cd there && pwd).

The normalized paths are stored in shell variables. These we can now
reuse in the subsequent install statements, which conveniently shortens
the lines a bit.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index cbab4f9..226dd03 100644
--- a/Makefile
+++ b/Makefile
@@ -1327,19 +1327,19 @@ ifndef NO_TCLTK
 	$(MAKE) -C gitk-git install
 	$(MAKE) -C git-gui install
 endif
-	if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
-	then \
-		ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
-			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' || \
-		cp '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
-			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
-	fi
-	$(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
-	$(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'
 ifneq (,$X)
 	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
 endif
-	./check_bindir 'z$(bindir_SQ)' 'z$(gitexecdir_SQ)' '$(DESTDIR_SQ)$(bindir_SQ)/git-shell$X'
+	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
+	execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
+	if test "z$$bindir" != "z$$execdir"; \
+	then \
+		ln -f "$$bindir/git$X" "$$execdir/git$X" || \
+		cp "$$bindir/git$X" "$$execdir/git$X"; \
+	fi && \
+	{ $(foreach p,$(BUILT_INS), $(RM) "$$execdir/$p" && ln "$$execdir/git$X" "$$execdir/$p" ;) } && \
+	$(RM) "$$execdir/git$X" && \
+	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-shell$X"
 
 install-doc:
 	$(MAKE) -C Documentation install
-- 
1.6.0.rc0.18.g6aef2

[PATCH 3/9] Record the command invocation path early

[Johannes Sixt]

We will need the command invocation path in system_path(). This path was
passed to setup_path(), but  system_path() can be called earlier, for
example via:

    main
      commit_pager_choice
        setup_pager
          git_config
            git_etc_gitconfig
              system_path

Therefore, we introduce git_set_argv0_path() and call it as soon as
possible.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 exec_cmd.c     |   10 ++++++++--
 exec_cmd.h     |    3 ++-
 git.c          |    5 ++---
 receive-pack.c |    2 +-
 shell.c        |    4 ++--
 upload-pack.c  |    2 +-
 6 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 8899e31..dedb01d 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -5,6 +5,7 @@
 
 extern char **environ;
 static const char *argv_exec_path;
+static const char *argv0_path;
 
 static const char *builtin_exec_path(void)
 {
@@ -50,6 +51,11 @@ const char *system_path(const char *path)
 	return path;
 }
 
+void git_set_argv0_path(const char *path)
+{
+	argv0_path = path;
+}
+
 void git_set_argv_exec_path(const char *exec_path)
 {
 	argv_exec_path = exec_path;
@@ -84,7 +90,7 @@ static void add_path(struct strbuf *out, const char *path)
 	}
 }
 
-void setup_path(const char *cmd_path)
+void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path;
@@ -94,7 +100,7 @@ void setup_path(const char *cmd_path)
 	add_path(&new_path, argv_exec_path);
 	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
 	add_path(&new_path, builtin_exec_path());
-	add_path(&new_path, cmd_path);
+	add_path(&new_path, argv0_path);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/exec_cmd.h b/exec_cmd.h
index 7eb94e5..0c46cd5 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -2,8 +2,9 @@
 #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_exec_path(void);
-extern void setup_path(const char *);
+extern void setup_path(void);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 extern const char *system_path(const char *path);
diff --git a/git.c b/git.c
index 74ea0e6..9c0dd88 100644
--- a/git.c
+++ b/git.c
@@ -418,7 +418,6 @@ 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_path = NULL;
 	int done_alias = 0;
 
 	/*
@@ -431,7 +430,7 @@ int main(int argc, const char **argv)
 	while (cmd <= slash && !is_dir_sep(*slash));
 	if (cmd <= slash) {
 		*slash++ = 0;
-		cmd_path = cmd;
+		git_set_argv0_path(cmd);
 		cmd = slash;
 	}
 
@@ -475,7 +474,7 @@ int main(int argc, const char **argv)
 	 * environment, and the $(gitexecdir) from the Makefile at build
 	 * time.
 	 */
-	setup_path(cmd_path);
+	setup_path();
 
 	while (1) {
 		/* See if it's an internal command */
diff --git a/receive-pack.c b/receive-pack.c
index fa653b4..d44c19e 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -482,7 +482,7 @@ int main(int argc, char **argv)
 	if (!dir)
 		usage(receive_pack_usage);
 
-	setup_path(NULL);
+	setup_path();
 
 	if (!enter_repo(dir, 0))
 		die("'%s': unable to chdir or not a git archive", dir);
diff --git a/shell.c b/shell.c
index 91ca7de..6a48de0 100644
--- a/shell.c
+++ b/shell.c
@@ -15,7 +15,7 @@ static int do_generic_cmd(const char *me, char *arg)
 {
 	const char *my_argv[4];
 
-	setup_path(NULL);
+	setup_path();
 	if (!arg || !(arg = sq_dequote(arg)))
 		die("bad argument");
 	if (prefixcmp(me, "git-"))
@@ -37,7 +37,7 @@ static int do_cvs_cmd(const char *me, char *arg)
 	if (!arg || strcmp(arg, "server"))
 		die("git-cvsserver only handles server: %s", arg);
 
-	setup_path(NULL);
+	setup_path();
 	return execv_git_cmd(cvsserver_argv);
 }
 
diff --git a/upload-pack.c b/upload-pack.c
index 9f82941..c911e70 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -638,7 +638,7 @@ int main(int argc, char **argv)
 	if (i != argc-1)
 		usage(upload_pack_usage);
 
-	setup_path(NULL);
+	setup_path();
 
 	dir = argv[i];
 
-- 
1.6.0.rc0.18.g6aef2

[PATCH 4/9] Fix relative built-in paths to be relative to the command invocation

[Johannes Sixt]

$(gitexecdir) (as defined in the Makefile) has gained another path
component, but the relative paths in the MINGW section of the Makefile,
which are interpreted relative to it, do not account for it.

Instead of adding another ../ in front of the path, we change the code that
constructs the absolute paths to do it relative to the command's directory,
which is essentially $(bindir). We do it this way because we will also
allow a relative $(gitexecdir) later.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile   |    2 +-
 exec_cmd.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 226dd03..23f2185 100644
--- a/Makefile
+++ b/Makefile
@@ -1310,7 +1310,7 @@ remove-dashes:
 ### Installation rules
 
 ifeq ($(firstword $(subst /, ,$(template_dir))),..)
-template_instdir = $(gitexecdir)/$(template_dir)
+template_instdir = $(bindir)/$(template_dir)
 else
 template_instdir = $(template_dir)
 endif
diff --git a/exec_cmd.c b/exec_cmd.c
index dedb01d..45f92eb 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -43,9 +43,9 @@ static const char *builtin_exec_path(void)
 
 const char *system_path(const char *path)
 {
-	if (!is_absolute_path(path)) {
+	if (!is_absolute_path(path) && argv0_path) {
 		struct strbuf d = STRBUF_INIT;
-		strbuf_addf(&d, "%s/%s", git_exec_path(), path);
+		strbuf_addf(&d, "%s/%s", argv0_path, path);
 		path = strbuf_detach(&d, NULL);
 	}
 	return path;
-- 
1.6.0.rc0.18.g6aef2

[PATCH 5/9] Allow the built-in exec path to be relative to the command invocation path

[Johannes Sixt]

If $(gitexecdir) is relative, it is interpreted relative to the command's
invocation path, which usually is $(bindir).

The Makefile rules were written with the assumption that $(gitexecdir) is
an absolute path. We introduce a separate variable that names the
(absolute) installation directory.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile   |   16 ++++++++++++----
 exec_cmd.c |   38 ++------------------------------------
 2 files changed, 14 insertions(+), 40 deletions(-)

diff --git a/Makefile b/Makefile
index 23f2185..908c1cb 100644
--- a/Makefile
+++ b/Makefile
@@ -1316,10 +1316,18 @@ template_instdir = $(template_dir)
 endif
 export template_instdir
 
+ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
+gitexec_instdir = $(bindir)/$(gitexecdir)
+else
+gitexec_instdir = $(gitexecdir)
+endif
+gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
+export gitexec_instdir
+
 install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
-	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
@@ -1328,10 +1336,10 @@ ifndef NO_TCLTK
 	$(MAKE) -C git-gui install
 endif
 ifneq (,$X)
-	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
+	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
 endif
 	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
-	execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
+	execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
 	if test "z$$bindir" != "z$$execdir"; \
 	then \
 		ln -f "$$bindir/git$X" "$$execdir/git$X" || \
diff --git a/exec_cmd.c b/exec_cmd.c
index 45f92eb..c236034 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -7,40 +7,6 @@ extern char **environ;
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-static const char *builtin_exec_path(void)
-{
-#ifndef __MINGW32__
-	return GIT_EXEC_PATH;
-#else
-	int len;
-	char *p, *q, *sl;
-	static char *ep;
-	if (ep)
-		return ep;
-
-	len = strlen(_pgmptr);
-	if (len < 2)
-		return ep = ".";
-
-	p = ep = xmalloc(len+1);
-	q = _pgmptr;
-	sl = NULL;
-	/* copy program name, turn '\\' into '/', skip last part */
-	while ((*p = *q)) {
-		if (*q == '\\' || *q == '/') {
-			*p = '/';
-			sl = p;
-		}
-		p++, q++;
-	}
-	if (sl)
-		*sl = '\0';
-	else
-		ep[0] = '.', ep[1] = '\0';
-	return ep;
-#endif
-}
-
 const char *system_path(const char *path)
 {
 	if (!is_absolute_path(path) && argv0_path) {
@@ -75,7 +41,7 @@ const char *git_exec_path(void)
 		return env;
 	}
 
-	return builtin_exec_path();
+	return system_path(GIT_EXEC_PATH);
 }
 
 static void add_path(struct strbuf *out, const char *path)
@@ -99,7 +65,7 @@ void setup_path(void)
 
 	add_path(&new_path, argv_exec_path);
 	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
-	add_path(&new_path, builtin_exec_path());
+	add_path(&new_path, system_path(GIT_EXEC_PATH));
 	add_path(&new_path, argv0_path);
 
 	if (old_path)
-- 
1.6.0.rc0.18.g6aef2

[PATCH 6/9] Allow add_path() to add non-existent directories to the path

[Johannes Sixt]

This function had used make_absolute_path(); but this function dies if
the directory that contains the entry whose relative path was supplied in
the argument does not exist. This is a problem if the argument is, for
example, "../libexec/git-core", and that "../libexec" does not exist.

Since the resolution of symbolic links is not required for elements in
PATH, we can fall back to using make_nonrelative_path(), which simply
prepends $PWD to the path.

We have to move make_nonrelative_path() alongside make_absolute_path() in
abspath.c so that git-shell can be linked. See 5b8e6f85f.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 abspath.c  |   36 ++++++++++++++++++++++++++++++++++++
 exec_cmd.c |    2 +-
 path.c     |   36 ------------------------------------
 3 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/abspath.c b/abspath.c
index 4f95a95..0d56124 100644
--- a/abspath.c
+++ b/abspath.c
@@ -66,3 +66,39 @@ const char *make_absolute_path(const char *path)
 
 	return buf;
 }
+
+static const char *get_pwd_cwd(void)
+{
+	static char cwd[PATH_MAX + 1];
+	char *pwd;
+	struct stat cwd_stat, pwd_stat;
+	if (getcwd(cwd, PATH_MAX) == NULL)
+		return NULL;
+	pwd = getenv("PWD");
+	if (pwd && strcmp(pwd, cwd)) {
+		stat(cwd, &cwd_stat);
+		if (!stat(pwd, &pwd_stat) &&
+		    pwd_stat.st_dev == cwd_stat.st_dev &&
+		    pwd_stat.st_ino == cwd_stat.st_ino) {
+			strlcpy(cwd, pwd, PATH_MAX);
+		}
+	}
+	return cwd;
+}
+
+const char *make_nonrelative_path(const char *path)
+{
+	static char buf[PATH_MAX + 1];
+
+	if (is_absolute_path(path)) {
+		if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
+			die("Too long path: %.*s", 60, path);
+	} else {
+		const char *cwd = get_pwd_cwd();
+		if (!cwd)
+			die("Cannot determine the current working directory");
+		if (snprintf(buf, PATH_MAX, "%s/%s", cwd, path) >= PATH_MAX)
+			die("Too long path: %.*s", 60, path);
+	}
+	return buf;
+}
diff --git a/exec_cmd.c b/exec_cmd.c
index c236034..0ed768d 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -50,7 +50,7 @@ static void add_path(struct strbuf *out, const char *path)
 		if (is_absolute_path(path))
 			strbuf_addstr(out, path);
 		else
-			strbuf_addstr(out, make_absolute_path(path));
+			strbuf_addstr(out, make_nonrelative_path(path));
 
 		strbuf_addch(out, PATH_SEP);
 	}
diff --git a/path.c b/path.c
index 504eae0..9df447b 100644
--- a/path.c
+++ b/path.c
@@ -291,42 +291,6 @@ int adjust_shared_perm(const char *path)
 	return 0;
 }
 
-static const char *get_pwd_cwd(void)
-{
-	static char cwd[PATH_MAX + 1];
-	char *pwd;
-	struct stat cwd_stat, pwd_stat;
-	if (getcwd(cwd, PATH_MAX) == NULL)
-		return NULL;
-	pwd = getenv("PWD");
-	if (pwd && strcmp(pwd, cwd)) {
-		stat(cwd, &cwd_stat);
-		if (!stat(pwd, &pwd_stat) &&
-		    pwd_stat.st_dev == cwd_stat.st_dev &&
-		    pwd_stat.st_ino == cwd_stat.st_ino) {
-			strlcpy(cwd, pwd, PATH_MAX);
-		}
-	}
-	return cwd;
-}
-
-const char *make_nonrelative_path(const char *path)
-{
-	static char buf[PATH_MAX + 1];
-
-	if (is_absolute_path(path)) {
-		if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
-			die ("Too long path: %.*s", 60, path);
-	} else {
-		const char *cwd = get_pwd_cwd();
-		if (!cwd)
-			die("Cannot determine the current working directory");
-		if (snprintf(buf, PATH_MAX, "%s/%s", cwd, path) >= PATH_MAX)
-			die ("Too long path: %.*s", 60, path);
-	}
-	return buf;
-}
-
 const char *make_relative_path(const char *abs, const char *base)
 {
 	static char buf[PATH_MAX + 1];
-- 
1.6.0.rc0.18.g6aef2

[PATCH 7/9] Windows: Make $(gitexecdir) relative

[Johannes Sixt]

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 908c1cb..1bfffff 100644
--- a/Makefile
+++ b/Makefile
@@ -745,6 +745,7 @@ 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
-- 
1.6.0.rc0.18.g6aef2

[PATCH 8/9] Windows: Make sure argv[0] has a path

[Johannes Sixt]

Since the exec-path on Windows is derived from the program invocation path,
we must ensure that argv[0] always has a path. Unfortunately, if a program
is invoked from CMD, argv[0] has no path. But on the other hand, the
C runtime offers a global variable, _pgmptr, that always has the full path
to the program. We hook into main() with a preprocessor macro, where we
replace argv[0].

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 compat/mingw.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 8ffec51..290a9e6 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -223,3 +223,15 @@ void mingw_open_html(const char *path);
 char **copy_environ(void);
 void free_environ(char **env);
 char **env_setenv(char **env, const char *name);
+
+/*
+ * A replacement of main() that ensures that argv[0] has a path
+ */
+
+#define main(c,v) main(int argc, const char **argv) \
+{ \
+	static int mingw_main(); \
+	argv[0] = xstrdup(_pgmptr); \
+	return mingw_main(argc, argv); \
+} \
+static int mingw_main(c,v)
-- 
1.6.0.rc0.18.g6aef2

[PATCH 9/9] Windows: Do not compile git-shell

[Johannes Sixt]

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 1bfffff..f8dd7e6 100644
--- a/Makefile
+++ b/Makefile
@@ -283,7 +283,6 @@ PROGRAMS += git-pack-redundant$X
 PROGRAMS += git-patch-id$X
 PROGRAMS += git-receive-pack$X
 PROGRAMS += git-send-pack$X
-PROGRAMS += git-shell$X
 PROGRAMS += git-show-index$X
 PROGRAMS += git-unpack-file$X
 PROGRAMS += git-update-server-info$X
@@ -812,6 +811,7 @@ EXTLIBS += -lz
 ifndef NO_POSIX_ONLY_PROGRAMS
 	PROGRAMS += git-daemon$X
 	PROGRAMS += git-imap-send$X
+	PROGRAMS += git-shell$X
 endif
 ifndef NO_OPENSSL
 	OPENSSL_LIBSSL = -lssl
@@ -1348,7 +1348,7 @@ endif
 	fi && \
 	{ $(foreach p,$(BUILT_INS), $(RM) "$$execdir/$p" && ln "$$execdir/git$X" "$$execdir/$p" ;) } && \
 	$(RM) "$$execdir/git$X" && \
-	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-shell$X"
+	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
 install-doc:
 	$(MAKE) -C Documentation install
-- 
1.6.0.rc0.18.g6aef2

[PATCH 10/9] git-gui: git.git now uses $(gitexec_instdir) to point to the exec-path

[Johannes Sixt]

In git.git the Makefile variable $(gitexecdir) can be relative. It now
exports the actual install location in $(gitexec_instdir).

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 Makefile |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index b19fb2d..c8660fb 100644
--- a/Makefile
+++ b/Makefile
@@ -30,8 +30,12 @@ ifndef SHELL_PATH
 endif
 
 ifndef gitexecdir
+ifdef gitexec_instdir
+	gitexecdir := $(gitexec_instdir)
+else
 	gitexecdir := $(shell git --exec-path)
 endif
+endif
 
 ifndef sharedir
 	sharedir := $(dir $(gitexecdir))share
-- 
1.6.0.rc0.18.g6aef2

Re: [PATCH 0/9] Make gitexecdir relative to $(bindir) on Windows

[Johannes Schindelin]

Hi,

On Mon, 21 Jul 2008, Johannes Sixt wrote:

> The problem was that argv[0] does not have a path in certain cases.

Note that the same holds true for Linux when calling a program that is in 
the PATH:

-- snip --
#include <stdio.h>

int main(int argc, char **argv)
{
        printf("%s\n", argv[0]);
        return 0;
}
-- snap --

compiled and put into the PATH will output just what the user said, not 
the full path.

I imagine that the proper solution would be to rip out lookup_prog() and 
use it for non-Windows Git, too.  Unless you want to limit the usefulness 
of your patch series to Windows, that is.

In which case you could use lookup_prog() after unstatifying it.

Ciao,
Dscho

Re: [PATCH 2/9] Makefile: Normalize $(bindir) and $(gitexecdir) before comparing

[Johannes Schindelin]

Hi,

On Mon, 21 Jul 2008, Johannes Sixt wrote:

> +	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
> +	execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \

These lack quotes, no?

Ciao,
Dscho

Re: [PATCH 2/9] Makefile: Normalize $(bindir) and $(gitexecdir) before comparing

[Johannes Sixt]

Johannes Schindelin schrieb:
> Hi,
> 
> On Mon, 21 Jul 2008, Johannes Sixt wrote:
> 
>> +	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
>> +	execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
> 
> These lack quotes, no?

No. RHS of an assignment doesn't need quotes as long as the shell syntax
makes clear where the assignment ends, which is the case here because of
the brackets $(...).

-- Hannes

Re: [PATCH 0/9] Make gitexecdir relative to $(bindir) on Windows

[Johannes Sixt]

On Dienstag, 22. Juli 2008, Johannes Schindelin wrote:
> Hi,
>
> On Mon, 21 Jul 2008, Johannes Sixt wrote:
> > The problem was that argv[0] does not have a path in certain cases.
>
> Note that the same holds true for Linux when calling a program that is in
> the PATH:

Oh, boy!

> I imagine that the proper solution would be to rip out lookup_prog() and
> use it for non-Windows Git, too.  Unless you want to limit the usefulness
> of your patch series to Windows, that is.

This certainly goes beyond what I am prepared to do. It is not my itch. The 
series is already much longer than I wanted, when there is a much simpler 
solution that solves *my* problem: to set bindir = $(gitexecdir).

-- Hannes

Re: [PATCH 0/9] Make gitexecdir relative to $(bindir) on Windows

[Junio C Hamano]

Johannes Sixt <johannes.sixt@telecom.at> writes:

> On Dienstag, 22. Juli 2008, Johannes Schindelin wrote:
>> Hi,
>>
>> On Mon, 21 Jul 2008, Johannes Sixt wrote:
>> > The problem was that argv[0] does not have a path in certain cases.
>>
>> Note that the same holds true for Linux when calling a program that is in
>> the PATH:
>
> Oh, boy!
>
>> I imagine that the proper solution would be to rip out lookup_prog() and
>> use it for non-Windows Git, too.  Unless you want to limit the usefulness
>> of your patch series to Windows, that is.
>
> This certainly goes beyond what I am prepared to do. It is not my itch. The 
> series is already much longer than I wanted, when there is a much simpler 
> solution that solves *my* problem: to set bindir = $(gitexecdir).

If you are living in the Windows world, perhaps you could record the
installation location in resource somewhere from the installer and look it
up at runtime?  Or is it considered a bad practice?

Re: [PATCH 5/9] Allow the built-in exec path to be relative to the command invocation path

[Junio C Hamano]

Johannes Sixt <johannes.sixt@telecom.at> writes:

> If $(gitexecdir) is relative, it is interpreted relative to the command's
> invocation path, which usually is $(bindir).
>
> The Makefile rules were written with the assumption that $(gitexecdir) is
> an absolute path. We introduce a separate variable that names the
> (absolute) installation directory.
>  ... 
> +ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
> +gitexec_instdir = $(bindir)/$(gitexecdir)
> +else

Can we please have a brief comment in the Makefile near we define mandir,
infodir, gitexecdir and friends about this "relative to $(bindir)"
business?

Perhaps like:

 Makefile |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 23f2185..8fa8f9a 100644
--- a/Makefile
+++ b/Makefile
@@ -171,6 +171,12 @@ ALL_LDFLAGS = $(LDFLAGS)
 STRIP ?= strip
 
 prefix = $(HOME)
+
+# Among these variables, gitexecdir and/or template_dir can be
+# specified as a relative path ../some/where/else; this is interpreted
+# as relative to $(bindir) and "git" at runtime figures out where they
+# are based on the path to the executable.  This can help installing the
+# suite in a relocatable way.
 bindir = $(prefix)/bin
 mandir = $(prefix)/share/man
 infodir = $(prefix)/share/info

Note that I just listed two variables out of thin air without studying;
you might be making other variables capable of relative path, in which
case they should also be listed there.

Re: [PATCH 0/9] Make gitexecdir relative to $(bindir) on Windows

[Johannes Sixt]

On Mittwoch, 23. Juli 2008, Junio C Hamano wrote:
> Johannes Sixt <johannes.sixt@telecom.at> writes:
> > On Dienstag, 22. Juli 2008, Johannes Schindelin wrote:
> >> On Mon, 21 Jul 2008, Johannes Sixt wrote:
> >> > The problem was that argv[0] does not have a path in certain cases.
> >>
> >> Note that the same holds true for Linux when calling a program that is
> >> in the PATH:
> >
> > Oh, boy!
> >
> >> I imagine that the proper solution would be to rip out lookup_prog() and
> >> use it for non-Windows Git, too.  Unless you want to limit the
> >> usefulness of your patch series to Windows, that is.
> >
> > This certainly goes beyond what I am prepared to do. It is not my itch.
> > The series is already much longer than I wanted, when there is a much
> > simpler solution that solves *my* problem: to set bindir = $(gitexecdir).
>
> If you are living in the Windows world, perhaps you could record the
> installation location in resource somewhere from the installer and look it
> up at runtime?  Or is it considered a bad practice?

Well, looking at value of _pgmptr *is* "look it up at runtime"; no resources 
or help from the installer are needed.

My rant here is more about that I created a *long* patch series only to find 
out that it does not have enough genericity to solve the same problem 
(relocatability) on platforms other than Windows - and I don't want to make 
it even longer. Windows is special enough that *I* could live with a much 
simpler solution even though it is a bit retro.

-- Hannes

[PATCH 5/9 v2] Allow the built-in exec path to be relative to the command invocation path

[Johannes Sixt]

If GIT_EXEC_PATH (the macro that is defined in the Makefile) is relative,
it is interpreted relative to the command's invocation path, which usually
is $(bindir).

The Makefile rules were written with the assumption that $(gitexecdir) is
an absolute path. We introduce a separate variable that names the
(absolute) installation directory.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
On Mittwoch, 23. Juli 2008, Junio C Hamano wrote:
> Johannes Sixt <johannes.sixt@telecom.at> writes:
> > If $(gitexecdir) is relative, it is interpreted relative to the command's
> > invocation path, which usually is $(bindir).
> >
> > The Makefile rules were written with the assumption that $(gitexecdir) is
> > an absolute path. We introduce a separate variable that names the
> > (absolute) installation directory.
> >  ...
> > +ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
> > +gitexec_instdir = $(bindir)/$(gitexecdir)
> > +else
>
> Can we please have a brief comment in the Makefile near we define mandir,
> infodir, gitexecdir and friends about this "relative to $(bindir)"
> business?

Here it is.

It also fixes 'make install' of git-gui as well (sigh!) by not exporting
gitexecdir - assuming that Shawn applies the git-gui patch.

The first two hunks are new compared to v1 of this patch.

-- Hannes

 Makefile   |   28 +++++++++++++++++++++++-----
 exec_cmd.c |   38 ++------------------------------------
 2 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/Makefile b/Makefile
index 23f2185..4f19b52 100644
--- a/Makefile
+++ b/Makefile
@@ -170,6 +170,16 @@ ALL_CFLAGS = $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 STRIP ?= strip
 
+# Among the variables below, these:
+#   gitexecdir
+#   template_dir
+#   htmldir
+#   ETC_GITCONFIG (but not sysconfdir)
+# can be specified as a relative path ../some/where/else (which must begin
+# with ../); this is interpreted as relative to $(bindir) and "git" at
+# runtime figures out where they are based on the path to the executable.
+# This can help installing the suite in a relocatable way.
+
 prefix = $(HOME)
 bindir = $(prefix)/bin
 mandir = $(prefix)/share/man
@@ -205,7 +215,7 @@ GITWEB_FAVICON = git-favicon.png
 GITWEB_SITE_HEADER =
 GITWEB_SITE_FOOTER =
 
-export prefix bindir gitexecdir sharedir htmldir sysconfdir
+export prefix bindir sharedir htmldir sysconfdir
 
 CC = gcc
 AR = ar
@@ -1316,10 +1326,18 @@ template_instdir = $(template_dir)
 endif
 export template_instdir
 
+ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
+gitexec_instdir = $(bindir)/$(gitexecdir)
+else
+gitexec_instdir = $(gitexecdir)
+endif
+gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
+export gitexec_instdir
+
 install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
-	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
-	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) git$X git-upload-pack$X git-receive-pack$X 
git-upload-archive$X '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
@@ -1328,10 +1346,10 @@ ifndef NO_TCLTK
 	$(MAKE) -C git-gui install
 endif
 ifneq (,$X)
-	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), 
$(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
+	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), 
$(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
 endif
 	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
-	execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
+	execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
 	if test "z$$bindir" != "z$$execdir"; \
 	then \
 		ln -f "$$bindir/git$X" "$$execdir/git$X" || \
diff --git a/exec_cmd.c b/exec_cmd.c
index 45f92eb..c236034 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -7,40 +7,6 @@ extern char **environ;
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-static const char *builtin_exec_path(void)
-{
-#ifndef __MINGW32__
-	return GIT_EXEC_PATH;
-#else
-	int len;
-	char *p, *q, *sl;
-	static char *ep;
-	if (ep)
-		return ep;
-
-	len = strlen(_pgmptr);
-	if (len < 2)
-		return ep = ".";
-
-	p = ep = xmalloc(len+1);
-	q = _pgmptr;
-	sl = NULL;
-	/* copy program name, turn '\\' into '/', skip last part */
-	while ((*p = *q)) {
-		if (*q == '\\' || *q == '/') {
-			*p = '/';
-			sl = p;
-		}
-		p++, q++;
-	}
-	if (sl)
-		*sl = '\0';
-	else
-		ep[0] = '.', ep[1] = '\0';
-	return ep;
-#endif
-}
-
 const char *system_path(const char *path)
 {
 	if (!is_absolute_path(path) && argv0_path) {
@@ -75,7 +41,7 @@ const char *git_exec_path(void)
 		return env;
 	}
 
-	return builtin_exec_path();
+	return system_path(GIT_EXEC_PATH);
 }
 
 static void add_path(struct strbuf *out, const char *path)
@@ -99,7 +65,7 @@ void setup_path(void)
 
 	add_path(&new_path, argv_exec_path);
 	add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
-	add_path(&new_path, builtin_exec_path());
+	add_path(&new_path, system_path(GIT_EXEC_PATH));
 	add_path(&new_path, argv0_path);
 
 	if (old_path)
-- 
1.6.0.rc0.18.g6aef2

Re: [PATCH 5/9 v2] Allow the built-in exec path to be relative to the command invocation path

[Junio C Hamano]

Johannes Sixt <johannes.sixt@telecom.at> writes:

> It also fixes 'make install' of git-gui as well (sigh!) by not exporting
> gitexecdir - assuming that Shawn applies the git-gui patch.

Yeah, this seems to break the install quite badly without git-gui patch.

If your PATH does not include the bindir you are installing the freshly
built git, then the Makefile in git-gui runs:

	ifndef gitexecdir
		gitexecdir := $(shell git --exec-path)
	endif

and miserably fails.  We can assume that somebody who builds and installs
git-gui as a standalone project already *has* an installed, working git on
$PATH, so the above ifndef is Ok, but when git-gui is built as part of
git.git tree, we really should avoid triggering that codepath.

Re: [PATCH 5/9 v2] Allow the built-in exec path to be relative to the command invocation path

[Johannes Sixt]

On Donnerstag, 24. Juli 2008, Junio C Hamano wrote:
> Johannes Sixt <johannes.sixt@telecom.at> writes:
> > It also fixes 'make install' of git-gui as well (sigh!) by not exporting
> > gitexecdir - assuming that Shawn applies the git-gui patch.
>
> Yeah, this seems to break the install quite badly without git-gui patch.

If you squash this in, we don't need the git-gui patch.

-- Hannes

diff --git a/Makefile b/Makefile
index aab23a2..904150e 100644
--- a/Makefile
+++ b/Makefile
@@ -1344,7 +1344,7 @@ install: all
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_TCLTK
 	$(MAKE) -C gitk-git install
-	$(MAKE) -C git-gui install
+	$(MAKE) -C git-gui gitexecdir='$(gitexec_instdir_SQ)' install
 endif
 ifneq (,$X)
 	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), 
$(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)

Re: [PATCH 9/9] Windows: Do not compile git-shell

[Steffen Prohaska]

Junio,
Do you plan to apply this patch anytime soon?  Currently,
building on MSYS fails when it comes to compiling git-shell.
I am waiting for this patch to either pop up in your or
Hannes' master.  I need this patch before I can push new
4msysgit branches (next, devel).  Should I apply it directly
in 4msysgit?

This patch is not related to the rest of the exec-path series.

	Steffen

Re: [PATCH 5/9 v2] Allow the built-in exec path to be relative to the command invocation path

[Junio C Hamano]

Johannes Sixt <johannes.sixt@telecom.at> writes:

> On Donnerstag, 24. Juli 2008, Junio C Hamano wrote:
>> Johannes Sixt <johannes.sixt@telecom.at> writes:
>> > It also fixes 'make install' of git-gui as well (sigh!) by not exporting
>> > gitexecdir - assuming that Shawn applies the git-gui patch.
>>
>> Yeah, this seems to break the install quite badly without git-gui patch.
>
> If you squash this in, we don't need the git-gui patch.

Thanks.

I think this patch makes _more_ sense than the git-gui patch, actually.

Within the context of git.git project, we would want to force the
installation directory of git-gui portion to be consistent with the main
project.

> diff --git a/Makefile b/Makefile
> index aab23a2..904150e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1344,7 +1344,7 @@ install: all
>  	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
>  ifndef NO_TCLTK
>  	$(MAKE) -C gitk-git install
> -	$(MAKE) -C git-gui install
> +	$(MAKE) -C git-gui gitexecdir='$(gitexec_instdir_SQ)' install
>  endif
>  ifneq (,$X)
>  	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), 
> $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)

However, I have to wonder if it is the right thing to do, like your patch
does, for "git --exec-path" to return "../libexec/git-core/" in a relative
form, without saying what it is relative to.  Shouldn't we be showing the
full path after resolving that relative path to git executable?

Re: [PATCH 5/9 v2] Allow the built-in exec path to be relative to the command invocation path

[Johannes Sixt]

Zitat von Junio C Hamano <gitster@pobox.com>:
> However, I have to wonder if it is the right thing to do, like your patch
> does, for "git --exec-path" to return "../libexec/git-core/" in a relative
> form, without saying what it is relative to.  Shouldn't we be showing the
> full path after resolving that relative path to git executable?

Does it? "git --exec-path" calls git_exec_path(), and that now returns
system_path(GIT_EXEC_PATH), and that is an absolute path, although it's
not normalized.

Oh, I see: You tested it on Linux, right? This patch series does not work
correctly on Linux (Unix? bash?), as Dscho has pointed out, since argv[0]
does not have a directory part if "git" is in $PATH. In this case, system_path()
just returns its argument, which is the relative path. :-/

-- Hannes

Re: [PATCH 5/9 v2] Allow the built-in exec path to be relative to the command invocation path

[Johannes Sixt]

Johannes Sixt schrieb:
> Oh, I see: You tested it on Linux, right? This patch series does not work
> correctly on Linux (Unix? bash?), as Dscho has pointed out, since argv[0]

does not work correctly on Linux... *iff gitexecdir is relative*

> does not have a directory part if "git" is in $PATH. In this case, system_path()
> just returns its argument, which is the relative path. :-/

-- Hannes

Re: [PATCH 1/9] Makefile: Do not install a copy of 'git' in $(gitexecdir)

[A Large Angry SCM]

Johannes Sixt wrote:
[...]
> 
> diff --git a/Makefile b/Makefile
> index 551bde9..cbab4f9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1335,6 +1335,7 @@ endif
>  			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
>  	fi
>  	$(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
> +	$(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'
>  ifneq (,$X)
>  	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
>  endif

This new action needs to be in a conditional to keep it from removing 
the ONLY git executable when bindir and execdir are the same dir.

Re: [PATCH 1/9] Makefile: Do not install a copy of 'git' in $(gitexecdir)

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> Johannes Sixt wrote:
> [...]
>>
>> diff --git a/Makefile b/Makefile
>> index 551bde9..cbab4f9 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1335,6 +1335,7 @@ endif
>>  			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
>>  	fi
>>  	$(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
>> +	$(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'
>>  ifneq (,$X)
>>  	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
>>  endif
>
> This new action needs to be in a conditional to keep it from removing
> the ONLY git executable when bindir and execdir are the same dir.

Heh, I love bug reports that come immediately after I tag the tip of
'master' as -rc1.

Re: [PATCH 1/9] Makefile: Do not install a copy of 'git' in $(gitexecdir)

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> This new action needs to be in a conditional to keep it from removing
>> the ONLY git executable when bindir and execdir are the same dir.
>
> Heh, I love bug reports that come immediately after I tag the tip of
> 'master' as -rc1.

This should do, but to be very honest, I really hate the output from the
foreach that precedes this section.

 Makefile |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 798a2f2..92df61c 100644
--- a/Makefile
+++ b/Makefile
@@ -1362,7 +1362,10 @@ endif
 		cp "$$bindir/git$X" "$$execdir/git$X"; \
 	fi && \
 	{ $(foreach p,$(BUILT_INS), $(RM) "$$execdir/$p" && ln "$$execdir/git$X" "$$execdir/$p" ;) } && \
-	$(RM) "$$execdir/git$X" && \
+	if test "z$$bindir" != "z$$execdir"; \
+	then \
+		$(RM) "$$execdir/git$X"; \
+	fi && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
 install-doc:

Re: [PATCH 5/9 v2] Allow the built-in exec path to be relative to the command invocation path

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Sixt <johannes.sixt@telecom.at> writes:
>
>> On Donnerstag, 24. Juli 2008, Junio C Hamano wrote:
>>> Johannes Sixt <johannes.sixt@telecom.at> writes:
>>> > It also fixes 'make install' of git-gui as well (sigh!) by not exporting
>>> > gitexecdir - assuming that Shawn applies the git-gui patch.
>>>
>>> Yeah, this seems to break the install quite badly without git-gui patch.
>>
>> If you squash this in, we don't need the git-gui patch.
>
> Thanks.
>
> I think this patch makes _more_ sense than the git-gui patch, actually.
>
> Within the context of git.git project, we would want to force the
> installation directory of git-gui portion to be consistent with the main
> project.

What I wanted to say with the above is that the main Makefile is what has
its own special need from git-gui/Makefile's point of view, so passing
such customization from the main Makefile makes a lot of sense.

I think this is also needed; I noticed it while trying the "build on a
machine without any git installed" exercise.

diff --git a/Makefile b/Makefile
index 798a2f2..52c67c1 100644
--- a/Makefile
+++ b/Makefile
@@ -1067,7 +1067,7 @@ endif
 
 all::
 ifndef NO_TCLTK
-	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) all
+	$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
 	$(QUIET_SUBDIR0)gitk-git $(QUIET_SUBDIR1) all
 endif
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all

Re: [PATCH 1/9] Makefile: Do not install a copy of 'git' in $(gitexecdir)

[A Large Angry SCM]

Junio C Hamano wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
> 
>> Johannes Sixt wrote:
>> [...]
>>> diff --git a/Makefile b/Makefile
>>> index 551bde9..cbab4f9 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1335,6 +1335,7 @@ endif
>>>  			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
>>>  	fi
>>>  	$(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
>>> +	$(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'
>>>  ifneq (,$X)
>>>  	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
>>>  endif
>> This new action needs to be in a conditional to keep it from removing
>> the ONLY git executable when bindir and execdir are the same dir.
> 
> Heh, I love bug reports that come immediately after I tag the tip of
> 'master' as -rc1.
> 

Well, I'm currently about a week behind on all things git so this is as 
timely as I could make it. ;-)

Re: [PATCH 1/9] Makefile: Do not install a copy of 'git' in $(gitexecdir)

[A Large Angry SCM]

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>>> This new action needs to be in a conditional to keep it from removing
>>> the ONLY git executable when bindir and execdir are the same dir.
>> Heh, I love bug reports that come immediately after I tag the tip of
>> 'master' as -rc1.
> 
> This should do, but to be very honest, I really hate the output from the
> foreach that precedes this section.
> 
>  Makefile |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 798a2f2..92df61c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1362,7 +1362,10 @@ endif
>  		cp "$$bindir/git$X" "$$execdir/git$X"; \
>  	fi && \
>  	{ $(foreach p,$(BUILT_INS), $(RM) "$$execdir/$p" && ln "$$execdir/git$X" "$$execdir/$p" ;) } && \
> -	$(RM) "$$execdir/git$X" && \
> +	if test "z$$bindir" != "z$$execdir"; \
> +	then \
> +		$(RM) "$$execdir/git$X"; \
> +	fi && \
>  	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
>  
>  install-doc:
> 

I'll test this this evening.

Re: [PATCH 1/9] Makefile: Do not install a copy of 'git' in $(gitexecdir)

[A Large Angry SCM]

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>>> This new action needs to be in a conditional to keep it from removing
>>> the ONLY git executable when bindir and execdir are the same dir.
>> Heh, I love bug reports that come immediately after I tag the tip of
>> 'master' as -rc1.
> 
> This should do, but to be very honest, I really hate the output from the
> foreach that precedes this section.
> 
>  Makefile |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 798a2f2..92df61c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1362,7 +1362,10 @@ endif
>  		cp "$$bindir/git$X" "$$execdir/git$X"; \
>  	fi && \
>  	{ $(foreach p,$(BUILT_INS), $(RM) "$$execdir/$p" && ln "$$execdir/git$X" "$$execdir/$p" ;) } && \
> -	$(RM) "$$execdir/git$X" && \
> +	if test "z$$bindir" != "z$$execdir"; \
> +	then \
> +		$(RM) "$$execdir/git$X"; \
> +	fi && \
>  	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
>  
>  install-doc:
> 

Tested by: A Large Angry SCM <gitzilla@gmail.com>