[PATCH 1/7] "git" returns 1; "git help" and "git help -a" return 0

[PATCH 1/7] "git" returns 1; "git help" and "git help -a" return 0

[Scott R Parish]

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 builtin.h |    1 +
 git.c     |    7 ++++---
 help.c    |    6 +++---
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/builtin.h b/builtin.h
index 65cc0fb..9a6213a 100644
--- a/builtin.h
+++ b/builtin.h
@@ -6,6 +6,7 @@
 extern const char git_version_string[];
 extern const char git_usage_string[];
 
+extern void list_common_cmds_help(void);
 extern void help_unknown_cmd(const char *cmd);
 extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
diff --git a/git.c b/git.c
index 23a430c..efed91c 100644
--- a/git.c
+++ b/git.c
@@ -450,9 +450,10 @@ int main(int argc, const char **argv)
 		if (!prefixcmp(argv[0], "--"))
 			argv[0] += 2;
 	} else {
-		/* Default command: "help" */
-		argv[0] = "help";
-		argc = 1;
+		/* The user didn't specify a command; give them help */
+		printf("usage: %s\n\n", git_usage_string);
+		list_common_cmds_help();
+		exit(1);
 	}
 	cmd = argv[0];
 
diff --git a/help.c b/help.c
index 1cd33ec..d4b1818 100644
--- a/help.c
+++ b/help.c
@@ -147,7 +147,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 	putchar('\n');
 }
 
-static void list_common_cmds_help(void)
+void list_common_cmds_help(void)
 {
 	int i, longest = 0;
 
@@ -204,14 +204,14 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
-		exit(1);
+		exit(0);
 	}
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
 			list_commands(exec_path, "git-*");
-		exit(1);
+		exit(0);
 	}
 
 	else
-- 
gitgui.0.8.4.11178.g9a1bf-dirty

[PATCH 2/7] remove unused/unneeded "pattern" argument of list_commands

[Scott R Parish]

list_commands() currently accepts and ignores a "pattern" argument,
and then hard codes a prefix as well as some magic numbers. This
hardcodes the prefix inside of the function and removes the magic
numbers.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/help.c b/help.c
index d4b1818..b636774 100644
--- a/help.c
+++ b/help.c
@@ -93,10 +93,12 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 	}
 }
 
-static void list_commands(const char *exec_path, const char *pattern)
+static void list_commands(const char *exec_path)
 {
 	unsigned int longest = 0;
 	char path[PATH_MAX];
+	const char *prefix = "git-";
+	int prefix_len = strlen(prefix);
 	int dirlen;
 	DIR *dir = opendir(exec_path);
 	struct dirent *de;
@@ -120,7 +122,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 		struct stat st;
 		int entlen;
 
-		if (prefixcmp(de->d_name, "git-"))
+		if (prefixcmp(de->d_name, prefix))
 			continue;
 		strcpy(path+dirlen, de->d_name);
 		if (stat(path, &st) || /* stat, not lstat */
@@ -128,14 +130,14 @@ static void list_commands(const char *exec_path, const char *pattern)
 		    !(st.st_mode & S_IXUSR))
 			continue;
 
-		entlen = strlen(de->d_name);
+		entlen = strlen(de->d_name) - prefix_len;
 		if (has_extension(de->d_name, ".exe"))
 			entlen -= 4;
 
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + 4, entlen-4);
+		add_cmdname(de->d_name + prefix_len, entlen);
 	}
 	closedir(dir);
 
@@ -143,7 +145,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 	printf("----------------------------");
 	mput_char('-', strlen(exec_path));
 	putchar('\n');
-	pretty_print_string_list(cmdname, longest - 4);
+	pretty_print_string_list(cmdname, longest);
 	putchar('\n');
 }
 
@@ -210,7 +212,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
-			list_commands(exec_path, "git-*");
+			list_commands(exec_path);
 		exit(0);
 	}
 
-- 
gitgui.0.8.4.11178.g9a1bf-dirty

[PATCH 3/7] "current_exec_path" is a misleading name, use "argv_exec_path"

[Scott R Parish]

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |   12 ++++++------
 exec_cmd.h |    2 +-
 git.c      |    2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 9b74ed2..8b681d0 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -5,11 +5,11 @@
 
 extern char **environ;
 static const char *builtin_exec_path = GIT_EXEC_PATH;
-static const char *current_exec_path;
+static const char *argv_exec_path = 0;
 
-void git_set_exec_path(const char *exec_path)
+void git_set_argv_exec_path(const char *exec_path)
 {
-	current_exec_path = exec_path;
+	argv_exec_path = exec_path;
 }
 
 
@@ -18,8 +18,8 @@ const char *git_exec_path(void)
 {
 	const char *env;
 
-	if (current_exec_path)
-		return current_exec_path;
+	if (argv_exec_path)
+		return argv_exec_path;
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
 	if (env && *env) {
@@ -34,7 +34,7 @@ int execv_git_cmd(const char **argv)
 {
 	char git_command[PATH_MAX + 1];
 	int i;
-	const char *paths[] = { current_exec_path,
+	const char *paths[] = { argv_exec_path,
 				getenv(EXEC_PATH_ENVIRONMENT),
 				builtin_exec_path };
 
diff --git a/exec_cmd.h b/exec_cmd.h
index 849a839..da99287 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -1,7 +1,7 @@
 #ifndef GIT_EXEC_CMD_H
 #define GIT_EXEC_CMD_H
 
-extern void git_set_exec_path(const char *exec_path);
+extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
diff --git a/git.c b/git.c
index efed91c..c7cabf5 100644
--- a/git.c
+++ b/git.c
@@ -51,7 +51,7 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 		if (!prefixcmp(cmd, "--exec-path")) {
 			cmd += 11;
 			if (*cmd == '=')
-				git_set_exec_path(cmd + 1);
+				git_set_argv_exec_path(cmd + 1);
 			else {
 				puts(git_exec_path());
 				exit(0);
-- 
gitgui.0.8.4.11178.g9a1bf-dirty

[PATCH 4/7] list_commands(): simplify code by using chdir()

[Scott R Parish]

The current code builds absolute path strings for each file to
stat(), this can easily be avoided by chdir()ing into the directory.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |   18 +++---------------
 1 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/help.c b/help.c
index b636774..ce3d795 100644
--- a/help.c
+++ b/help.c
@@ -96,36 +96,24 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 static void list_commands(const char *exec_path)
 {
 	unsigned int longest = 0;
-	char path[PATH_MAX];
 	const char *prefix = "git-";
 	int prefix_len = strlen(prefix);
-	int dirlen;
 	DIR *dir = opendir(exec_path);
 	struct dirent *de;
 
-	if (!dir) {
+	if (!dir || chdir(exec_path)) {
 		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
 		exit(1);
 	}
 
-	dirlen = strlen(exec_path);
-	if (PATH_MAX - 20 < dirlen) {
-		fprintf(stderr, "git: insanely long exec-path '%s'\n",
-			exec_path);
-		exit(1);
-	}
-
-	memcpy(path, exec_path, dirlen);
-	path[dirlen++] = '/';
-
 	while ((de = readdir(dir)) != NULL) {
 		struct stat st;
 		int entlen;
 
 		if (prefixcmp(de->d_name, prefix))
 			continue;
-		strcpy(path+dirlen, de->d_name);
-		if (stat(path, &st) || /* stat, not lstat */
+
+		if (stat(de->d_name, &st) || /* stat, not lstat */
 		    !S_ISREG(st.st_mode) ||
 		    !(st.st_mode & S_IXUSR))
 			continue;
-- 
gitgui.0.8.4.11178.g9a1bf-dirty

[PATCH 5/7] use only the $PATH for exec'ing git commands

[Scott R Parish]

We need to correctly set up $PATH for non-c based git commands.
Since we already do this, we can just use that $PATH and execvp,
instead of looping over the paths with execve.

This patch adds a setup_path() function to exec_cmd.c, which sets
the $PATH order correctly for our search order. execv_git_cmd() is
stripped down to setting up argv and calling execvp(). git.c's
main() only only needs to call setup_path().

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |  121 ++++++++++++++++++++++++++----------------------------------
 exec_cmd.h |    1 +
 git.c      |   43 +++------------------
 3 files changed, 60 insertions(+), 105 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 8b681d0..c228dbf 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -29,85 +29,68 @@ const char *git_exec_path(void)
 	return builtin_exec_path;
 }
 
+static void add_path(struct strbuf *out, const char *path)
+{
+	if (path && strlen(path)) {
+		if (is_absolute_path(path))
+			strbuf_addstr(out, path);
+		else
+			strbuf_addstr(out, make_absolute_path(path));
+
+		strbuf_addch(out, ':');
+	}
+}
+
+void setup_path(const char *cmd_path)
+{
+	const char *old_path = getenv("PATH");
+	struct strbuf new_path;
+
+	strbuf_init(&new_path, 0);
+
+	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);
+
+	if (old_path)
+		strbuf_addstr(&new_path, old_path);
+	else
+		strbuf_addstr(&new_path, "/usr/local/bin:/usr/bin:/bin");
+
+	setenv("PATH", new_path.buf, 1);
+
+	strbuf_release(&new_path);
+}
 
 int execv_git_cmd(const char **argv)
 {
-	char git_command[PATH_MAX + 1];
-	int i;
-	const char *paths[] = { argv_exec_path,
-				getenv(EXEC_PATH_ENVIRONMENT),
-				builtin_exec_path };
-
-	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
-		size_t len;
-		int rc;
-		const char *exec_dir = paths[i];
-		const char *tmp;
-
-		if (!exec_dir || !*exec_dir) continue;
-
-		if (*exec_dir != '/') {
-			if (!getcwd(git_command, sizeof(git_command))) {
-				fprintf(stderr, "git: cannot determine "
-					"current directory: %s\n",
-					strerror(errno));
-				break;
-			}
-			len = strlen(git_command);
-
-			/* Trivial cleanup */
-			while (!prefixcmp(exec_dir, "./")) {
-				exec_dir += 2;
-				while (*exec_dir == '/')
-					exec_dir++;
-			}
-
-			rc = snprintf(git_command + len,
-				      sizeof(git_command) - len, "/%s",
-				      exec_dir);
-			if (rc < 0 || rc >= sizeof(git_command) - len) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-		} else {
-			if (strlen(exec_dir) + 1 > sizeof(git_command)) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-			strcpy(git_command, exec_dir);
-		}
-
-		len = strlen(git_command);
-		rc = snprintf(git_command + len, sizeof(git_command) - len,
-			      "/git-%s", argv[0]);
-		if (rc < 0 || rc >= sizeof(git_command) - len) {
-			fprintf(stderr,
-				"git: command name given is too long.\n");
-			break;
-		}
+	struct strbuf cmd;
+	const char *tmp;
 
-		/* argv[0] must be the git command, but the argv array
-		 * belongs to the caller, and my be reused in
-		 * subsequent loop iterations. Save argv[0] and
-		 * restore it on error.
-		 */
+	strbuf_init(&cmd, 0);
+	strbuf_addf(&cmd, "git-%s", argv[0]);
 
-		tmp = argv[0];
-		argv[0] = git_command;
+	/* argv[0] must be the git command, but the argv array
+	 * belongs to the caller, and my be reused in
+	 * subsequent loop iterations. Save argv[0] and
+	 * restore it on error.
+	 */
+	tmp = argv[0];
+	argv[0] = cmd.buf;
 
-		trace_argv_printf(argv, -1, "trace: exec:");
+	trace_argv_printf(argv, -1, "trace: exec:");
 
-		/* execve() can only ever return if it fails */
-		execve(git_command, (char **)argv, environ);
+	/* execvp() can only ever return if it fails */
+	execvp(cmd.buf, (char **)argv);
 
-		trace_printf("trace: exec failed: %s\n", strerror(errno));
+	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-		argv[0] = tmp;
-	}
-	return -1;
+	argv[0] = tmp;
 
+	strbuf_release(&cmd);
+
+	return -1;
 }
 
 
diff --git a/exec_cmd.h b/exec_cmd.h
index da99287..a892355 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,6 +3,7 @@
 
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
+extern void setup_path(const char *);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 
diff --git a/git.c b/git.c
index c7cabf5..4e10581 100644
--- a/git.c
+++ b/git.c
@@ -6,28 +6,6 @@
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
-static void prepend_to_path(const char *dir, int len)
-{
-	const char *old_path = getenv("PATH");
-	char *path;
-	int path_len = len;
-
-	if (!old_path)
-		old_path = "/usr/local/bin:/usr/bin:/bin";
-
-	path_len = len + strlen(old_path) + 1;
-
-	path = xmalloc(path_len + 1);
-
-	memcpy(path, dir, len);
-	path[len] = ':';
-	memcpy(path + len + 1, old_path, path_len - len);
-
-	setenv("PATH", path, 1);
-
-	free(path);
-}
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
@@ -408,7 +386,7 @@ int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";
 	char *slash = strrchr(cmd, '/');
-	const char *exec_path = NULL;
+	const char *cmd_path = NULL;
 	int done_alias = 0;
 
 	/*
@@ -418,10 +396,7 @@ int main(int argc, const char **argv)
 	 */
 	if (slash) {
 		*slash++ = 0;
-		if (*cmd == '/')
-			exec_path = cmd;
-		else
-			exec_path = xstrdup(make_absolute_path(cmd));
+		cmd_path = cmd;
 		cmd = slash;
 	}
 
@@ -458,16 +433,12 @@ int main(int argc, const char **argv)
 	cmd = argv[0];
 
 	/*
-	 * We execute external git command via execv_git_cmd(),
-	 * which looks at "--exec-path" option, GIT_EXEC_PATH
-	 * environment, and $(gitexecdir) in Makefile while built,
-	 * in this order.  For scripted commands, we prepend
-	 * the value of the exec_path variable to the PATH.
+	 * We use PATH to find git commands, but we prepend some higher
+	 * precidence paths: the "--exec-path" option, the GIT_EXEC_PATH
+	 * environment, and the $(gitexecdir) from the Makefile at build
+	 * time.
 	 */
-	if (exec_path)
-		prepend_to_path(exec_path, strlen(exec_path));
-	exec_path = git_exec_path();
-	prepend_to_path(exec_path, strlen(exec_path));
+	setup_path(cmd_path);
 
 	while (1) {
 		/* See if it's an internal command */
-- 
gitgui.0.8.4.11178.g9a1bf-dirty

[PATCH 6/7] walk $PATH to generate list of commands for "help -a"

[Scott R Parish]

Git had previously been using the $PATH for scripts--a previous
patch moved exec'ed commands to also use the $PATH. For consistancy
"help -a" should also use the $PATH.

We walk all the paths in $PATH collecting the names of "git-*"
commands. To help distinguish between the main git commands
and commands picked up elsewhere (probably extensions) we
print them seperately. The main commands are the ones that
are found in the first directory in $PATH that contains the
"git" binary.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |  158 ++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 125 insertions(+), 33 deletions(-)

diff --git a/help.c b/help.c
index ce3d795..ee4fce0 100644
--- a/help.c
+++ b/help.c
@@ -37,24 +37,29 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdname {
-	size_t len;
-	char name[1];
-} **cmdname;
-static int cmdname_alloc, cmdname_cnt;
-
-static void add_cmdname(const char *name, int len)
+static struct cmdnames {
+	int alloc;
+	int cnt;
+	char *dir;
+	struct cmdname {
+		size_t len;
+		char name[1];
+	} **names;
+} main_cmds, other_cmds;
+
+static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
 	struct cmdname *ent;
-	if (cmdname_alloc <= cmdname_cnt) {
-		cmdname_alloc = cmdname_alloc + 200;
-		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
+	if (cmds->alloc <= cmds->cnt) {
+		cmds->alloc = cmds->alloc + 200;
+		cmds->names = xrealloc(cmds->names,
+				       cmds->alloc * sizeof(*cmds->names));
 	}
 	ent = xmalloc(sizeof(*ent) + len);
 	ent->len = len;
 	memcpy(ent->name, name, len);
 	ent->name[len] = 0;
-	cmdname[cmdname_cnt++] = ent;
+	cmds->names[cmds->cnt++] = ent;
 }
 
 static int cmdname_compare(const void *a_, const void *b_)
@@ -64,7 +69,42 @@ static int cmdname_compare(const void *a_, const void *b_)
 	return strcmp(a->name, b->name);
 }
 
-static void pretty_print_string_list(struct cmdname **cmdname, int longest)
+static void uniq(struct cmdnames *cmds)
+{
+	int i, j;
+
+	if (!cmds->cnt)
+		return;
+
+	for (i = j = 1; i < cmds->cnt; i++) {
+		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
+			cmds->names[j++] = cmds->names[i];
+		}
+	}
+
+	cmds->cnt = j;
+}
+
+static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
+	int ai, aj, bi;
+
+	ai = aj = bi = 0;
+	while (ai < a->cnt && bi < b->cnt) {
+		if (0 > strcmp(a->names[ai]->name, b->names[bi]->name))
+			a->names[aj++] = a->names[ai++];
+		else if (0 > strcmp(a->names[ai]->name, b->names[bi]->name))
+			bi++;
+		else
+			ai++, bi++;
+	}
+
+	while (ai < a->cnt)
+		a->names[aj++] = a->names[ai++];
+
+	a->cnt = aj;
+}
+
+static void pretty_print_string_list(struct cmdnames *cmds, int longest)
 {
 	int cols = 1, rows;
 	int space = longest + 1; /* min 1 SP between words */
@@ -73,9 +113,7 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 
 	if (space < max_cols)
 		cols = max_cols / space;
-	rows = (cmdname_cnt + cols - 1) / cols;
-
-	qsort(cmdname, cmdname_cnt, sizeof(*cmdname), cmdname_compare);
+	rows = (cmds->cnt + cols - 1) / cols;
 
 	for (i = 0; i < rows; i++) {
 		printf("  ");
@@ -83,31 +121,39 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 		for (j = 0; j < cols; j++) {
 			int n = j * rows + i;
 			int size = space;
-			if (n >= cmdname_cnt)
+			if (n >= cmds->cnt)
 				break;
-			if (j == cols-1 || n + rows >= cmdname_cnt)
+			if (j == cols-1 || n + rows >= cmds->cnt)
 				size = 1;
-			printf("%-*s", size, cmdname[n]->name);
+			printf("%-*s", size, cmds->names[n]->name);
 		}
 		putchar('\n');
 	}
 }
 
-static void list_commands(const char *exec_path)
+static unsigned int list_commands_in_dir(const char *dir)
 {
 	unsigned int longest = 0;
 	const char *prefix = "git-";
 	int prefix_len = strlen(prefix);
-	DIR *dir = opendir(exec_path);
+	DIR *dirp = opendir(dir);
+	struct cmdnames *cmds;
 	struct dirent *de;
+	struct stat st;
 
-	if (!dir || chdir(exec_path)) {
-		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
-		exit(1);
+	if (!dirp || chdir(dir))
+		return 0;
+
+	if (!main_cmds.cnt &&
+	    !stat("git", &st) &&
+	    S_ISREG(st.st_mode) && (st.st_mode & S_IXUSR)) {
+		cmds = &main_cmds;
+		cmds->dir = xstrdup(dir);
 	}
+	else
+		cmds = &other_cmds;
 
-	while ((de = readdir(dir)) != NULL) {
-		struct stat st;
+	while ((de = readdir(dirp)) != NULL) {
 		int entlen;
 
 		if (prefixcmp(de->d_name, prefix))
@@ -122,18 +168,66 @@ static void list_commands(const char *exec_path)
 		if (has_extension(de->d_name, ".exe"))
 			entlen -= 4;
 
+		if (has_extension(de->d_name, ".perl") ||
+		    has_extension(de->d_name, ".sh"))
+			continue;
+
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + prefix_len, entlen);
+		add_cmdname(cmds, de->d_name + prefix_len, entlen);
+	}
+	closedir(dirp);
+
+	return longest;
+}
+
+static void list_commands()
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *env_path = getenv("PATH");
+	char *paths, *path, *colon;
+
+	if (!env_path) {
+		fprintf(stderr, "PATH not set\n");
+		exit(1);
 	}
-	closedir(dir);
 
-	printf("git commands available in '%s'\n", exec_path);
+	path = paths = xstrdup(env_path);
+	while ((char *)1 != path) {
+		if ((colon = strchr(path, ':')))
+			*colon = 0;
+
+		len = list_commands_in_dir(path);
+		longest = MAX(longest, len);
+
+		path = colon + 1;
+	}
+	free(paths);
+
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(*main_cmds.names), cmdname_compare);
+	uniq(&main_cmds);
+
+	qsort(other_cmds.names, other_cmds.cnt,
+	      sizeof(*other_cmds.names), cmdname_compare);
+	uniq(&other_cmds);
+	subtract_cmds(&other_cmds, &main_cmds);
+
+	printf("available git commands in '%s'\n", main_cmds.dir);
 	printf("----------------------------");
-	mput_char('-', strlen(exec_path));
+	mput_char('-', strlen(main_cmds.dir));
+	putchar('\n');
+	pretty_print_string_list(&main_cmds, longest);
 	putchar('\n');
-	pretty_print_string_list(cmdname, longest);
+
+	if (!other_cmds.cnt)
+		return;
+
+	printf("git commands available from elsewhere on your $PATH\n");
+	printf("---------------------------------------------------\n");
+	pretty_print_string_list(&other_cmds, longest);
 	putchar('\n');
 }
 
@@ -189,7 +283,6 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	const char *help_cmd = argc > 1 ? argv[1] : NULL;
-	const char *exec_path = git_exec_path();
 
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
@@ -199,8 +292,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
-		if(exec_path)
-			list_commands(exec_path);
+		list_commands();
 		exit(0);
 	}
 
-- 
gitgui.0.8.4.11178.g9a1bf-dirty

[PATCH 7/7] shell should call the new setup_path() to setup $PATH

[Scott R Parish]

Shell currently does its own manual thing for setting up the $PATH;
it can now call setup_path().

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 shell.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/shell.c b/shell.c
index cfe372b..9826109 100644
--- a/shell.c
+++ b/shell.c
@@ -24,17 +24,11 @@ static int do_cvs_cmd(const char *me, char *arg)
 	const char *cvsserver_argv[3] = {
 		"cvsserver", "server", NULL
 	};
-	const char *oldpath = getenv("PATH");
-	struct strbuf newpath = STRBUF_INIT;
 
 	if (!arg || strcmp(arg, "server"))
 		die("git-cvsserver only handles server: %s", arg);
 
-	strbuf_addstr(&newpath, git_exec_path());
-	strbuf_addch(&newpath, ':');
-	strbuf_addstr(&newpath, oldpath);
-
-	setenv("PATH", strbuf_detach(&newpath, NULL), 1);
+	setup_path(NULL);
 
 	return execv_git_cmd(cvsserver_argv);
 }
-- 
gitgui.0.8.4.11178.g9a1bf-dirty

Re: [PATCH 6/7] walk $PATH to generate list of commands for "help -a"

[Junio C Hamano]

Scott R Parish <srp@srparish.net> writes:

> Git had previously been using the $PATH for scripts--a previous
> patch moved exec'ed commands to also use the $PATH. For consistancy
> "help -a" should also use the $PATH.

s/consistancy/consistency/

> We walk all the paths in $PATH collecting the names of "git-*"
> commands. To help distinguish between the main git commands
> and commands picked up elsewhere (probably extensions) we
> print them seperately. The main commands are the ones that
> are found in the first directory in $PATH that contains the
> "git" binary.

This is not right.  $(gitexecdir) in Makefile is designed to
allow distros to move git-* commands out of the primary user
$PATH directories and install only "git" wrapper in /usr/bin.
"Use the directory 'git' is in" rule breaks this.

The "main commands" should be the first of argv_exec_path,
EXEC_PATH_ENVIRONMENT or builtin_exec_path.

> diff --git a/help.c b/help.c
> index ce3d795..ee4fce0 100644
> --- a/help.c
> +++ b/help.c
> @@ -64,7 +69,42 @@ static int cmdname_compare(const void *a_, const void *b_)
> ...
> +static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
> +	int ai, aj, bi;
> +
> +	ai = aj = bi = 0;
> +	while (ai < a->cnt && bi < b->cnt) {
> +		if (0 > strcmp(a->names[ai]->name, b->names[bi]->name))
> +			a->names[aj++] = a->names[ai++];
> +		else if (0 > strcmp(a->names[ai]->name, b->names[bi]->name))
> +			bi++;
> +		else
> +			ai++, bi++;

In general, xxxcmp(a, b) is designed to return the same sign as
"a - b" (subtract b from a, using an appropriate definition of
"subtract" in the domain of a and b).  It is a good habit to
write:

	strcmp(a, b) < 0	strcmp(a, b) > 0

because these give the same sign as

	a < b			a > b

and makes your program easier to read.

> @@ -122,18 +168,66 @@ static void list_commands(const char *exec_path)
>  		if (has_extension(de->d_name, ".exe"))
>  			entlen -= 4;
>  
> +		if (has_extension(de->d_name, ".perl") ||
> +		    has_extension(de->d_name, ".sh"))
> +			continue;
> +

This needs a good justification.

If you have "." on PATH, and you run ./git in a freshly built
source directory, "git relink.perl" would try to run
./git-relink.perl.

I do not think excluding these is necessary nor is a good idea.

> +static void list_commands()
> +{

ANSI.  "static void list_commands(void)".

> +	path = paths = xstrdup(env_path);
> +	while ((char *)1 != path) {
> +		if ((colon = strchr(path, ':')))
> +			*colon = 0;
> +
> +		len = list_commands_in_dir(path);
> +		longest = MAX(longest, len);
> +
> +		path = colon + 1;
> +	}

I know that on modern architectures bit representation of
(char*) NULL is the same as integer 0 of the same size as a
pointer, and adding 1 to it would yield (char *)1, but the above
feels _dirty_.

	while (1) {
        	...
                if (!colon)
	                break;
		path = colon + 1;
	}

Re: [PATCH 6/7] walk $PATH to generate list of commands for "help -a"

[Scott Parish]

On Sat, Oct 27, 2007 at 11:18:02PM -0700, Junio C Hamano wrote:

> > We walk all the paths in $PATH collecting the names of "git-*"
> > commands. To help distinguish between the main git commands
> > and commands picked up elsewhere (probably extensions) we
> > print them seperately. The main commands are the ones that
> > are found in the first directory in $PATH that contains the
> > "git" binary.
> 
> This is not right.  $(gitexecdir) in Makefile is designed to
> allow distros to move git-* commands out of the primary user
> $PATH directories and install only "git" wrapper in /usr/bin.
> "Use the directory 'git' is in" rule breaks this.
> 
> The "main commands" should be the first of argv_exec_path,
> EXEC_PATH_ENVIRONMENT or builtin_exec_path.

This is after we've already prepended the above three paths (if
they're specified) to $PATH, so yes, generally they should be in
one of those directories, but more generally, it will be in one of
the directories in $PATH.

Its not clear to me what exactly you're looking for me to change,
just the wording i'm using in my comment? Or are you refering to
the approach?

When i email the changes, should i keep emailing the whole
patch series, or just the few patches that have changed?

Thanks
sRp

-- 
Scott Parish
http://srparish.net/

Re: [PATCH 6/7] walk $PATH to generate list of commands for "help -a"

[Junio C Hamano]

Scott Parish <sRp@srparish.net> writes:

> On Sat, Oct 27, 2007 at 11:18:02PM -0700, Junio C Hamano wrote:
>
>> > We walk all the paths in $PATH collecting the names of "git-*"
>> > commands. To help distinguish between the main git commands
>> > and commands picked up elsewhere (probably extensions) we
>> > print them seperately. The main commands are the ones that
>> > are found in the first directory in $PATH that contains the
>> > "git" binary.
>> ...
> Its not clear to me what exactly you're looking for me to change,
> just the wording i'm using in my comment? Or are you refering to
> the approach?

"git" binary will be found as /usr/bin/git while git-foo will be
found as /usr/libexec/git/git-foo in such an installation that
takes advantage of $(gitexecdir).  And /usr/libexec/git/git will
not exist.  Using existence of /usr/bin/git (I am referring to
your 'first directory on $PATH that contains the "git" binary'
above) as the cue for the location of "main commands" is wrong.

> When i email the changes, should i keep emailing the whole
> patch series, or just the few patches that have changed?

It's up to you.  Obviously, if you are replacing 2 patches out
of 100-patch series, you would not want to resend the whole
thing, though ;-)

Re: [PATCH 6/7] walk $PATH to generate list of commands for "help -a"

[Scott Parish]

On Sun, Oct 28, 2007 at 03:07:14AM -0700, Junio C Hamano wrote:

> Scott Parish <sRp@srparish.net> writes:
> 
> > On Sat, Oct 27, 2007 at 11:18:02PM -0700, Junio C Hamano wrote:
> >
> >> > We walk all the paths in $PATH collecting the names of "git-*"
> >> > commands. To help distinguish between the main git commands
> >> > and commands picked up elsewhere (probably extensions) we
> >> > print them seperately. The main commands are the ones that
> >> > are found in the first directory in $PATH that contains the
> >> > "git" binary.
> >> ...
> > Its not clear to me what exactly you're looking for me to change,
> > just the wording i'm using in my comment? Or are you refering to
> > the approach?
> 
> "git" binary will be found as /usr/bin/git while git-foo will be
> found as /usr/libexec/git/git-foo in such an installation that
> takes advantage of $(gitexecdir).  And /usr/libexec/git/git will
> not exist.  Using existence of /usr/bin/git (I am referring to
> your 'first directory on $PATH that contains the "git" binary'
> above) as the cue for the location of "main commands" is wrong.

Thanks for the clarification, that would be a problem. I've modified
the patch to list the main commands from git_exec_path(); i have
mixed feelings, but curious what you think.

sRp

-- 
Scott Parish
http://srparish.net/

[PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Scott R Parish]

Git had previously been using the $PATH for scripts--a previous
patch moved exec'ed commands to also use the $PATH. For consistency
"help -a" should also list commands in the $PATH.

The main commands are still listed from the git_exec_path(), but
the $PATH is walked and other git commands (probably extensions) are
listed.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |  162 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 124 insertions(+), 38 deletions(-)

diff --git a/help.c b/help.c
index 34ac5db..2534bf0 100644
--- a/help.c
+++ b/help.c
@@ -37,24 +37,28 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdname {
-	size_t len;
-	char name[1];
-} **cmdname;
-static int cmdname_alloc, cmdname_cnt;
-
-static void add_cmdname(const char *name, int len)
+static struct cmdnames {
+	int alloc;
+	int cnt;
+	struct cmdname {
+		size_t len;
+		char name[1];
+	} **names;
+} main_cmds, other_cmds;
+
+static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
 	struct cmdname *ent;
-	if (cmdname_alloc <= cmdname_cnt) {
-		cmdname_alloc = cmdname_alloc + 200;
-		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
+	if (cmds->alloc <= cmds->cnt) {
+		cmds->alloc = cmds->alloc + 200;
+		cmds->names = xrealloc(cmds->names,
+				       cmds->alloc * sizeof(*cmds->names));
 	}
 	ent = xmalloc(sizeof(*ent) + len);
 	ent->len = len;
 	memcpy(ent->name, name, len);
 	ent->name[len] = 0;
-	cmdname[cmdname_cnt++] = ent;
+	cmds->names[cmds->cnt++] = ent;
 }
 
 static int cmdname_compare(const void *a_, const void *b_)
@@ -64,7 +68,44 @@ static int cmdname_compare(const void *a_, const void *b_)
 	return strcmp(a->name, b->name);
 }
 
-static void pretty_print_string_list(struct cmdname **cmdname, int longest)
+static void uniq(struct cmdnames *cmds)
+{
+	int i, j;
+
+	if (!cmds->cnt)
+		return;
+
+	for (i = j = 1; i < cmds->cnt; i++) {
+		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
+			cmds->names[j++] = cmds->names[i];
+		}
+	}
+
+	cmds->cnt = j;
+}
+
+static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
+	int ai, aj, bi;
+	int cmp;
+
+	ai = aj = bi = 0;
+	while (ai < a->cnt && bi < b->cnt) {
+		cmp = strcmp(a->names[ai]->name, b->names[bi]->name);
+		if (cmp < 0)
+			a->names[aj++] = a->names[ai++];
+		else if (cmp == 0)
+			ai++, bi++;
+		else if (cmp > 0)
+			bi++;
+	}
+
+	while (ai < a->cnt)
+		a->names[aj++] = a->names[ai++];
+
+	a->cnt = aj;
+}
+
+static void pretty_print_string_list(struct cmdnames *cmds, int longest)
 {
 	int cols = 1, rows;
 	int space = longest + 1; /* min 1 SP between words */
@@ -73,9 +114,7 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 
 	if (space < max_cols)
 		cols = max_cols / space;
-	rows = (cmdname_cnt + cols - 1) / cols;
-
-	qsort(cmdname, cmdname_cnt, sizeof(*cmdname), cmdname_compare);
+	rows = (cmds->cnt + cols - 1) / cols;
 
 	for (i = 0; i < rows; i++) {
 		printf("  ");
@@ -83,31 +122,29 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 		for (j = 0; j < cols; j++) {
 			int n = j * rows + i;
 			int size = space;
-			if (n >= cmdname_cnt)
+			if (n >= cmds->cnt)
 				break;
-			if (j == cols-1 || n + rows >= cmdname_cnt)
+			if (j == cols-1 || n + rows >= cmds->cnt)
 				size = 1;
-			printf("%-*s", size, cmdname[n]->name);
+			printf("%-*s", size, cmds->names[n]->name);
 		}
 		putchar('\n');
 	}
 }
 
-static void list_commands(const char *exec_path)
+static unsigned int list_commands_in_dir(struct cmdnames *cmds, const char *dir)
 {
 	unsigned int longest = 0;
 	const char *prefix = "git-";
 	int prefix_len = strlen(prefix);
-	DIR *dir = opendir(exec_path);
+	DIR *dirp = opendir(dir);
 	struct dirent *de;
+	struct stat st;
 
-	if (!dir || chdir(exec_path)) {
-		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
-		exit(1);
-	}
+	if (!dirp || chdir(dir))
+		return 0;
 
-	while ((de = readdir(dir)) != NULL) {
-		struct stat st;
+	while ((de = readdir(dirp)) != NULL) {
 		int entlen;
 
 		if (prefixcmp(de->d_name, prefix))
@@ -125,16 +162,67 @@ static void list_commands(const char *exec_path)
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + prefix_len, entlen);
+		add_cmdname(cmds, de->d_name + prefix_len, entlen);
+	}
+	closedir(dirp);
+
+	return longest;
+}
+
+static void list_commands(void)
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *env_path = getenv("PATH");
+	char *paths, *path, *colon;
+	const char *exec_path = git_exec_path();
+
+	if (exec_path)
+		longest = list_commands_in_dir(&main_cmds, exec_path);
+
+	if (!env_path) {
+		fprintf(stderr, "PATH not set\n");
+		exit(1);
+	}
+
+	path = paths = xstrdup(env_path);
+	while (1) {
+		if ((colon = strchr(path, ':')))
+			*colon = 0;
+
+		len = list_commands_in_dir(&other_cmds, path);
+		longest = MAX(longest, len);
+
+		if (!colon)
+			break;
+		path = colon + 1;
+	}
+	free(paths);
+
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(*main_cmds.names), cmdname_compare);
+	uniq(&main_cmds);
+
+	qsort(other_cmds.names, other_cmds.cnt,
+	      sizeof(*other_cmds.names), cmdname_compare);
+	uniq(&other_cmds);
+	subtract_cmds(&other_cmds, &main_cmds);
+
+	if (main_cmds.cnt) {
+		printf("available git commands in '%s'\n", exec_path);
+		printf("----------------------------");
+		mput_char('-', strlen(exec_path));
+		putchar('\n');
+		pretty_print_string_list(&main_cmds, longest);
+		putchar('\n');
+	}
+
+	if (other_cmds.cnt) {
+		printf("git commands available from elsewhere on your $PATH\n");
+		printf("---------------------------------------------------\n");
+		pretty_print_string_list(&other_cmds, longest);
+		putchar('\n');
 	}
-	closedir(dir);
-
-	printf("git commands available in '%s'\n", exec_path);
-	printf("----------------------------");
-	mput_char('-', strlen(exec_path));
-	putchar('\n');
-	pretty_print_string_list(cmdname, longest);
-	putchar('\n');
 }
 
 void list_common_cmds_help(void)
@@ -188,7 +276,6 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	const char *help_cmd = argc > 1 ? argv[1] : NULL;
-	const char *exec_path = git_exec_path();
 
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
@@ -198,8 +285,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
-		if(exec_path)
-			list_commands(exec_path);
+		list_commands();
 		exit(0);
 	}
 
-- 
1.5.3.4.401.g19778-dirty

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Junio C Hamano]

Scott R Parish <srp@srparish.net> writes:

> +	while (1) {
> +		if ((colon = strchr(path, ':')))
> +			*colon = 0;
> +
> +		len = list_commands_in_dir(&other_cmds, path);
> +		longest = MAX(longest, len);

Where do we borrow this MAX() macro?

On Linux with glibc, /usr/include/sys/param.h which is included
by git-compat-util.h (meaning, for everybody) is where we find
it, but that somehow does not sound portable assumption.

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Scott Parish]

On Sun, Oct 28, 2007 at 04:32:12AM -0700, Junio C Hamano wrote:

> Scott R Parish <srp@srparish.net> writes:
> 
> > +	while (1) {
> > +		if ((colon = strchr(path, ':')))
> > +			*colon = 0;
> > +
> > +		len = list_commands_in_dir(&other_cmds, path);
> > +		longest = MAX(longest, len);
> 
> Where do we borrow this MAX() macro?
> 
> On Linux with glibc, /usr/include/sys/param.h which is included
> by git-compat-util.h (meaning, for everybody) is where we find
> it, but that somehow does not sound portable assumption.

Awesome catch

sRp

-- 
Scott Parish
http://srparish.net/

[PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Scott R Parish]

Git had previously been using the $PATH for scripts--a previous
patch moved exec'ed commands to also use the $PATH. For consistency
"help -a" should also list commands in the $PATH.

The main commands are still listed from the git_exec_path(), but
the $PATH is walked and other git commands (probably extensions) are
listed.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |  163 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 125 insertions(+), 38 deletions(-)

diff --git a/help.c b/help.c
index 34ac5db..07cf67a 100644
--- a/help.c
+++ b/help.c
@@ -37,24 +37,28 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdname {
-	size_t len;
-	char name[1];
-} **cmdname;
-static int cmdname_alloc, cmdname_cnt;
-
-static void add_cmdname(const char *name, int len)
+static struct cmdnames {
+	int alloc;
+	int cnt;
+	struct cmdname {
+		size_t len;
+		char name[1];
+	} **names;
+} main_cmds, other_cmds;
+
+static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
 	struct cmdname *ent;
-	if (cmdname_alloc <= cmdname_cnt) {
-		cmdname_alloc = cmdname_alloc + 200;
-		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
+	if (cmds->alloc <= cmds->cnt) {
+		cmds->alloc = cmds->alloc + 200;
+		cmds->names = xrealloc(cmds->names,
+				       cmds->alloc * sizeof(*cmds->names));
 	}
 	ent = xmalloc(sizeof(*ent) + len);
 	ent->len = len;
 	memcpy(ent->name, name, len);
 	ent->name[len] = 0;
-	cmdname[cmdname_cnt++] = ent;
+	cmds->names[cmds->cnt++] = ent;
 }
 
 static int cmdname_compare(const void *a_, const void *b_)
@@ -64,7 +68,44 @@ static int cmdname_compare(const void *a_, const void *b_)
 	return strcmp(a->name, b->name);
 }
 
-static void pretty_print_string_list(struct cmdname **cmdname, int longest)
+static void uniq(struct cmdnames *cmds)
+{
+	int i, j;
+
+	if (!cmds->cnt)
+		return;
+
+	for (i = j = 1; i < cmds->cnt; i++) {
+		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
+			cmds->names[j++] = cmds->names[i];
+		}
+	}
+
+	cmds->cnt = j;
+}
+
+static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
+	int ai, aj, bi;
+	int cmp;
+
+	ai = aj = bi = 0;
+	while (ai < a->cnt && bi < b->cnt) {
+		cmp = strcmp(a->names[ai]->name, b->names[bi]->name);
+		if (cmp < 0)
+			a->names[aj++] = a->names[ai++];
+		else if (cmp == 0)
+			ai++, bi++;
+		else if (cmp > 0)
+			bi++;
+	}
+
+	while (ai < a->cnt)
+		a->names[aj++] = a->names[ai++];
+
+	a->cnt = aj;
+}
+
+static void pretty_print_string_list(struct cmdnames *cmds, int longest)
 {
 	int cols = 1, rows;
 	int space = longest + 1; /* min 1 SP between words */
@@ -73,9 +114,7 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 
 	if (space < max_cols)
 		cols = max_cols / space;
-	rows = (cmdname_cnt + cols - 1) / cols;
-
-	qsort(cmdname, cmdname_cnt, sizeof(*cmdname), cmdname_compare);
+	rows = (cmds->cnt + cols - 1) / cols;
 
 	for (i = 0; i < rows; i++) {
 		printf("  ");
@@ -83,31 +122,29 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 		for (j = 0; j < cols; j++) {
 			int n = j * rows + i;
 			int size = space;
-			if (n >= cmdname_cnt)
+			if (n >= cmds->cnt)
 				break;
-			if (j == cols-1 || n + rows >= cmdname_cnt)
+			if (j == cols-1 || n + rows >= cmds->cnt)
 				size = 1;
-			printf("%-*s", size, cmdname[n]->name);
+			printf("%-*s", size, cmds->names[n]->name);
 		}
 		putchar('\n');
 	}
 }
 
-static void list_commands(const char *exec_path)
+static unsigned int list_commands_in_dir(struct cmdnames *cmds, const char *dir)
 {
 	unsigned int longest = 0;
 	const char *prefix = "git-";
 	int prefix_len = strlen(prefix);
-	DIR *dir = opendir(exec_path);
+	DIR *dirp = opendir(dir);
 	struct dirent *de;
+	struct stat st;
 
-	if (!dir || chdir(exec_path)) {
-		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
-		exit(1);
-	}
+	if (!dirp || chdir(dir))
+		return 0;
 
-	while ((de = readdir(dir)) != NULL) {
-		struct stat st;
+	while ((de = readdir(dirp)) != NULL) {
 		int entlen;
 
 		if (prefixcmp(de->d_name, prefix))
@@ -125,16 +162,68 @@ static void list_commands(const char *exec_path)
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + prefix_len, entlen);
+		add_cmdname(cmds, de->d_name + prefix_len, entlen);
+	}
+	closedir(dirp);
+
+	return longest;
+}
+
+static void list_commands(void)
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *env_path = getenv("PATH");
+	char *paths, *path, *colon;
+	const char *exec_path = git_exec_path();
+
+	if (exec_path)
+		longest = list_commands_in_dir(&main_cmds, exec_path);
+
+	if (!env_path) {
+		fprintf(stderr, "PATH not set\n");
+		exit(1);
+	}
+
+	path = paths = xstrdup(env_path);
+	while (1) {
+		if ((colon = strchr(path, ':')))
+			*colon = 0;
+
+		len = list_commands_in_dir(&other_cmds, path);
+		if (len > longest)
+			longest = len;
+
+		if (!colon)
+			break;
+		path = colon + 1;
+	}
+	free(paths);
+
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(*main_cmds.names), cmdname_compare);
+	uniq(&main_cmds);
+
+	qsort(other_cmds.names, other_cmds.cnt,
+	      sizeof(*other_cmds.names), cmdname_compare);
+	uniq(&other_cmds);
+	subtract_cmds(&other_cmds, &main_cmds);
+
+	if (main_cmds.cnt) {
+		printf("available git commands in '%s'\n", exec_path);
+		printf("----------------------------");
+		mput_char('-', strlen(exec_path));
+		putchar('\n');
+		pretty_print_string_list(&main_cmds, longest);
+		putchar('\n');
+	}
+
+	if (other_cmds.cnt) {
+		printf("git commands available from elsewhere on your $PATH\n");
+		printf("---------------------------------------------------\n");
+		pretty_print_string_list(&other_cmds, longest);
+		putchar('\n');
 	}
-	closedir(dir);
-
-	printf("git commands available in '%s'\n", exec_path);
-	printf("----------------------------");
-	mput_char('-', strlen(exec_path));
-	putchar('\n');
-	pretty_print_string_list(cmdname, longest);
-	putchar('\n');
 }
 
 void list_common_cmds_help(void)
@@ -188,7 +277,6 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	const char *help_cmd = argc > 1 ? argv[1] : NULL;
-	const char *exec_path = git_exec_path();
 
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
@@ -198,8 +286,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
-		if(exec_path)
-			list_commands(exec_path);
+		list_commands();
 		exit(0);
 	}
 
-- 
1.5.3.4.401.g19778-dirty

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Johannes Schindelin]

Hi,

On Sun, 28 Oct 2007, Scott R Parish wrote:

> diff --git a/help.c b/help.c
> index 34ac5db..07cf67a 100644
> --- a/help.c
> +++ b/help.c
> @@ -37,24 +37,28 @@ static inline void mput_char(char c, unsigned int num)
>  		putchar(c);
>  }
>  
> -static struct cmdname {
> -	size_t len;
> -	char name[1];
> -} **cmdname;
> -static int cmdname_alloc, cmdname_cnt;
> -
> -static void add_cmdname(const char *name, int len)
> +static struct cmdnames {
> +	int alloc;
> +	int cnt;
> +	struct cmdname {
> +		size_t len;
> +		char name[1];
> +	} **names;
> +} main_cmds, other_cmds;
> +
> +static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
>  {
>  	struct cmdname *ent;
> -	if (cmdname_alloc <= cmdname_cnt) {
> -		cmdname_alloc = cmdname_alloc + 200;
> -		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
> +	if (cmds->alloc <= cmds->cnt) {
> +		cmds->alloc = cmds->alloc + 200;
> +		cmds->names = xrealloc(cmds->names,
> +				       cmds->alloc * sizeof(*cmds->names));

Looks like a candidate for ALLOC_GROW() ...

> @@ -64,7 +68,44 @@ static int cmdname_compare(const void *a_, const void *b_)
>  	return strcmp(a->name, b->name);
>  }
>  
> -static void pretty_print_string_list(struct cmdname **cmdname, int longest)
> +static void uniq(struct cmdnames *cmds)
> +{
> +	int i, j;
> +
> +	if (!cmds->cnt)
> +		return;
> +
> +	for (i = j = 1; i < cmds->cnt; i++) {
> +		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name)) {
> +			cmds->names[j++] = cmds->names[i];
> +		}
> +	}

Losing the curly brackets would make this look much nicer.

> +
> +	cmds->cnt = j;
> +}
> +
> +static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {

Maybe "exclude_cmds()", and choose more suggestive names for the 
parameters?

> -	DIR *dir = opendir(exec_path);
> +	DIR *dirp = opendir(dir);

I am not sure that a rename from "dir" to "dirp" is needed here.  It 
distracts a little from the real content of your patch.

Thanks,
Dscho

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Scott Parish]

On Sun, Oct 28, 2007 at 04:51:00PM +0000, Johannes Schindelin wrote:

> > +static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
> 
> Maybe "exclude_cmds()", and choose more suggestive names for the 
> parameters?

I was thinking set operations when i named this (hense "a" and "b"),
but i'll try this out.

sRp

-- 
Scott Parish
http://srparish.net/

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Johannes Schindelin]

Hi,

On Sun, 28 Oct 2007, Scott Parish wrote:

> On Sun, Oct 28, 2007 at 04:51:00PM +0000, Johannes Schindelin wrote:
> 
> > > +static void subtract_cmds(struct cmdnames *a, struct cmdnames *b) {
> > 
> > Maybe "exclude_cmds()", and choose more suggestive names for the 
> > parameters?
> 
> I was thinking set operations when i named this (hense "a" and "b"),
> but i'll try this out.

Yes, I guessed that.  But in that case, "subtract" is actively wrong, 
since you cannot guarantee (and indeed do not want to assume) that one is 
the subset of the other.

Ciao,
Dscho

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[David Symonds]

On 10/29/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sun, 28 Oct 2007, Scott Parish wrote:
>
> > I was thinking set operations when i named this (hense "a" and "b"),
> > but i'll try this out.
>
> Yes, I guessed that.  But in that case, "subtract" is actively wrong,
> since you cannot guarantee (and indeed do not want to assume) that one is
> the subset of the other.

The nearest set theory operation would be "difference" (or
"complement"); that does not require that the subtrahend is a subset
of the minuend.


Dave.

[PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Scott R Parish]

Git had previously been using the $PATH for scripts--a previous
patch moved exec'ed commands to also use the $PATH. For consistency
"help -a" should also list commands in the $PATH.

The main commands are still listed from the git_exec_path(), but
the $PATH is walked and other git commands (probably extensions) are
listed.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |  157 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 120 insertions(+), 37 deletions(-)

diff --git a/help.c b/help.c
index 34ac5db..855aeef 100644
--- a/help.c
+++ b/help.c
@@ -37,24 +37,25 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdname {
-	size_t len;
-	char name[1];
-} **cmdname;
-static int cmdname_alloc, cmdname_cnt;
-
-static void add_cmdname(const char *name, int len)
+static struct cmdnames {
+	int alloc;
+	int cnt;
+	struct cmdname {
+		size_t len;
+		char name[1];
+	} **names;
+} main_cmds, other_cmds;
+
+static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
-	struct cmdname *ent;
-	if (cmdname_alloc <= cmdname_cnt) {
-		cmdname_alloc = cmdname_alloc + 200;
-		cmdname = xrealloc(cmdname, cmdname_alloc * sizeof(*cmdname));
-	}
-	ent = xmalloc(sizeof(*ent) + len);
+	struct cmdname *ent = xmalloc(sizeof(*ent) + len);
+
 	ent->len = len;
 	memcpy(ent->name, name, len);
 	ent->name[len] = 0;
-	cmdname[cmdname_cnt++] = ent;
+
+	ALLOC_GROW(cmds->names, cmds->cnt + 1, cmds->alloc);
+	cmds->names[cmds->cnt++] = ent;
 }
 
 static int cmdname_compare(const void *a_, const void *b_)
@@ -64,7 +65,42 @@ static int cmdname_compare(const void *a_, const void *b_)
 	return strcmp(a->name, b->name);
 }
 
-static void pretty_print_string_list(struct cmdname **cmdname, int longest)
+static void uniq(struct cmdnames *cmds)
+{
+	int i, j;
+
+	if (!cmds->cnt)
+		return;
+
+	for (i = j = 1; i < cmds->cnt; i++)
+		if (strcmp(cmds->names[i]->name, cmds->names[i-1]->name))
+			cmds->names[j++] = cmds->names[i];
+
+	cmds->cnt = j;
+}
+
+static void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes) {
+	int ci, cj, ei;
+	int cmp;
+
+	ci = cj = ei = 0;
+	while (ci < cmds->cnt && ei < excludes->cnt) {
+		cmp = strcmp(cmds->names[ci]->name, excludes->names[ei]->name);
+		if (cmp < 0)
+			cmds->names[cj++] = cmds->names[ci++];
+		else if (cmp == 0)
+			ci++, ei++;
+		else if (cmp > 0)
+			ei++;
+	}
+
+	while (ci < cmds->cnt)
+		cmds->names[cj++] = cmds->names[ci++];
+
+	cmds->cnt = cj;
+}
+
+static void pretty_print_string_list(struct cmdnames *cmds, int longest)
 {
 	int cols = 1, rows;
 	int space = longest + 1; /* min 1 SP between words */
@@ -73,9 +109,7 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 
 	if (space < max_cols)
 		cols = max_cols / space;
-	rows = (cmdname_cnt + cols - 1) / cols;
-
-	qsort(cmdname, cmdname_cnt, sizeof(*cmdname), cmdname_compare);
+	rows = (cmds->cnt + cols - 1) / cols;
 
 	for (i = 0; i < rows; i++) {
 		printf("  ");
@@ -83,28 +117,27 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 		for (j = 0; j < cols; j++) {
 			int n = j * rows + i;
 			int size = space;
-			if (n >= cmdname_cnt)
+			if (n >= cmds->cnt)
 				break;
-			if (j == cols-1 || n + rows >= cmdname_cnt)
+			if (j == cols-1 || n + rows >= cmds->cnt)
 				size = 1;
-			printf("%-*s", size, cmdname[n]->name);
+			printf("%-*s", size, cmds->names[n]->name);
 		}
 		putchar('\n');
 	}
 }
 
-static void list_commands(const char *exec_path)
+static unsigned int list_commands_in_dir(struct cmdnames *cmds,
+					 const char *path)
 {
 	unsigned int longest = 0;
 	const char *prefix = "git-";
 	int prefix_len = strlen(prefix);
-	DIR *dir = opendir(exec_path);
+	DIR *dir = opendir(path);
 	struct dirent *de;
 
-	if (!dir || chdir(exec_path)) {
-		fprintf(stderr, "git: '%s': %s\n", exec_path, strerror(errno));
-		exit(1);
-	}
+	if (!dir || chdir(path))
+		return 0;
 
 	while ((de = readdir(dir)) != NULL) {
 		struct stat st;
@@ -125,16 +158,68 @@ static void list_commands(const char *exec_path)
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + prefix_len, entlen);
+		add_cmdname(cmds, de->d_name + prefix_len, entlen);
 	}
 	closedir(dir);
 
-	printf("git commands available in '%s'\n", exec_path);
-	printf("----------------------------");
-	mput_char('-', strlen(exec_path));
-	putchar('\n');
-	pretty_print_string_list(cmdname, longest);
-	putchar('\n');
+	return longest;
+}
+
+static void list_commands(void)
+{
+	unsigned int longest = 0;
+	unsigned int len;
+	const char *env_path = getenv("PATH");
+	char *paths, *path, *colon;
+	const char *exec_path = git_exec_path();
+
+	if (exec_path)
+		longest = list_commands_in_dir(&main_cmds, exec_path);
+
+	if (!env_path) {
+		fprintf(stderr, "PATH not set\n");
+		exit(1);
+	}
+
+	path = paths = xstrdup(env_path);
+	while (1) {
+		if ((colon = strchr(path, ':')))
+			*colon = 0;
+
+		len = list_commands_in_dir(&other_cmds, path);
+		if (len > longest)
+			longest = len;
+
+		if (!colon)
+			break;
+		path = colon + 1;
+	}
+	free(paths);
+
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(*main_cmds.names), cmdname_compare);
+	uniq(&main_cmds);
+
+	qsort(other_cmds.names, other_cmds.cnt,
+	      sizeof(*other_cmds.names), cmdname_compare);
+	uniq(&other_cmds);
+	exclude_cmds(&other_cmds, &main_cmds);
+
+	if (main_cmds.cnt) {
+		printf("available git commands in '%s'\n", exec_path);
+		printf("----------------------------");
+		mput_char('-', strlen(exec_path));
+		putchar('\n');
+		pretty_print_string_list(&main_cmds, longest);
+		putchar('\n');
+	}
+
+	if (other_cmds.cnt) {
+		printf("git commands available from elsewhere on your $PATH\n");
+		printf("---------------------------------------------------\n");
+		pretty_print_string_list(&other_cmds, longest);
+		putchar('\n');
+	}
 }
 
 void list_common_cmds_help(void)
@@ -188,7 +273,6 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	const char *help_cmd = argc > 1 ? argv[1] : NULL;
-	const char *exec_path = git_exec_path();
 
 	if (!help_cmd) {
 		printf("usage: %s\n\n", git_usage_string);
@@ -198,8 +282,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
-		if(exec_path)
-			list_commands(exec_path);
+		list_commands();
 		exit(0);
 	}
 
-- 
1.5.3.4.401.g19778-dirty

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Junio C Hamano]

Scott R Parish <srp@srparish.net> writes:

> Git had previously been using the $PATH for scripts--a previous
> patch moved exec'ed commands to also use the $PATH. For consistency
> "help -a" should also list commands in the $PATH.
>
> The main commands are still listed from the git_exec_path(), but
> the $PATH is walked and other git commands (probably extensions) are
> listed.
>
> Signed-off-by: Scott R Parish <srp@srparish.net>
> ---
>  help.c |  157 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
>  1 files changed, 120 insertions(+), 37 deletions(-)

Thanks.

It's easier to read if you briefly describe the differences
between the replacement patch and the previous version of the
patch below the three-dash lines.  See for example Lars Knoll's
patch from today <200710290959.32538.lars@trolltech.com>.

Re: [PATCH 6/7] include $PATH in generating list of commands for "help -a"

[Scott Parish]

On Mon, Oct 29, 2007 at 02:17:26PM -0700, Junio C Hamano wrote:

> It's easier to read if you briefly describe the differences
> between the replacement patch and the previous version of the
> patch below the three-dash lines.  See for example Lars Knoll's
> patch from today <200710290959.32538.lars@trolltech.com>.

Oh, that's useful information!

For the above patch, were basically what Johannes Schindelin suggested:

 + add_cmdname() now uses ALLOC_GROW and has its lines reordered to be
   somewhat cleanre
 + uniq() has lost the curly brackets
 + s/subtract_cmds/exclude_cmds/
 + exclude_cmds() uses an arg name of "path" instead of "dir"
 + exclude_cmd() no longer renames "dir" to "dirp"
 + exclude_cmds() an earlier patch moved the "struct stat" declaration
   for no-longer relevant reasons. change removed.

sRp

-- 
Scott Parish
http://srparish.net/

Re: [PATCH 5/7] use only the $PATH for exec'ing git commands

[Junio C Hamano]

Scott R Parish <srp@srparish.net> writes:

> diff --git a/exec_cmd.c b/exec_cmd.c
> index 8b681d0..c228dbf 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -29,85 +29,68 @@ const char *git_exec_path(void)
>  	return builtin_exec_path;
>  }
>  
> +static void add_path(struct strbuf *out, const char *path)
> +{
> +	if (path && strlen(path)) {

I wonder if s/strlen(path)/*path/ micro-optimization is worth
doing.  Ideally, if built-in strlen() is used, the compiler
should be clever enough to notice it, though...

Re: [PATCH 5/7] use only the $PATH for exec'ing git commands

[Adam Roben]

Junio C Hamano wrote:
> Scott R Parish <srp@srparish.net> writes:
>
>   
>> diff --git a/exec_cmd.c b/exec_cmd.c
>> index 8b681d0..c228dbf 100644
>> --- a/exec_cmd.c
>> +++ b/exec_cmd.c
>> @@ -29,85 +29,68 @@ const char *git_exec_path(void)
>>  	return builtin_exec_path;
>>  }
>>  
>> +static void add_path(struct strbuf *out, const char *path)
>> +{
>> +	if (path && strlen(path)) {
>>     
>
> I wonder if s/strlen(path)/*path/ micro-optimization is worth
> doing.  Ideally, if built-in strlen() is used, the compiler
> should be clever enough to notice it, though...
>   

You could always just check path[0] instead of calling strlen.

-Adam

[PATCH 5/7] use only the $PATH for exec'ing git commands

[Scott R Parish]

We need to correctly set up $PATH for non-c based git commands.
Since we already do this, we can just use that $PATH and execvp,
instead of looping over the paths with execve.

This patch adds a setup_path() function to exec_cmd.c, which sets
the $PATH order correctly for our search order. execv_git_cmd() is
stripped down to setting up argv and calling execvp(). git.c's
main() only only needs to call setup_path().

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 exec_cmd.c |  121 ++++++++++++++++++++++++++----------------------------------
 exec_cmd.h |    1 +
 git.c      |   43 +++------------------
 3 files changed, 60 insertions(+), 105 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 8b681d0..53d0f3c 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -29,85 +29,68 @@ const char *git_exec_path(void)
 	return builtin_exec_path;
 }
 
+static void add_path(struct strbuf *out, const char *path)
+{
+	if (path && *path) {
+		if (is_absolute_path(path))
+			strbuf_addstr(out, path);
+		else
+			strbuf_addstr(out, make_absolute_path(path));
+
+		strbuf_addch(out, ':');
+	}
+}
+
+void setup_path(const char *cmd_path)
+{
+	const char *old_path = getenv("PATH");
+	struct strbuf new_path;
+
+	strbuf_init(&new_path, 0);
+
+	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);
+
+	if (old_path)
+		strbuf_addstr(&new_path, old_path);
+	else
+		strbuf_addstr(&new_path, "/usr/local/bin:/usr/bin:/bin");
+
+	setenv("PATH", new_path.buf, 1);
+
+	strbuf_release(&new_path);
+}
 
 int execv_git_cmd(const char **argv)
 {
-	char git_command[PATH_MAX + 1];
-	int i;
-	const char *paths[] = { argv_exec_path,
-				getenv(EXEC_PATH_ENVIRONMENT),
-				builtin_exec_path };
-
-	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
-		size_t len;
-		int rc;
-		const char *exec_dir = paths[i];
-		const char *tmp;
-
-		if (!exec_dir || !*exec_dir) continue;
-
-		if (*exec_dir != '/') {
-			if (!getcwd(git_command, sizeof(git_command))) {
-				fprintf(stderr, "git: cannot determine "
-					"current directory: %s\n",
-					strerror(errno));
-				break;
-			}
-			len = strlen(git_command);
-
-			/* Trivial cleanup */
-			while (!prefixcmp(exec_dir, "./")) {
-				exec_dir += 2;
-				while (*exec_dir == '/')
-					exec_dir++;
-			}
-
-			rc = snprintf(git_command + len,
-				      sizeof(git_command) - len, "/%s",
-				      exec_dir);
-			if (rc < 0 || rc >= sizeof(git_command) - len) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-		} else {
-			if (strlen(exec_dir) + 1 > sizeof(git_command)) {
-				fprintf(stderr, "git: command name given "
-					"is too long.\n");
-				break;
-			}
-			strcpy(git_command, exec_dir);
-		}
-
-		len = strlen(git_command);
-		rc = snprintf(git_command + len, sizeof(git_command) - len,
-			      "/git-%s", argv[0]);
-		if (rc < 0 || rc >= sizeof(git_command) - len) {
-			fprintf(stderr,
-				"git: command name given is too long.\n");
-			break;
-		}
+	struct strbuf cmd;
+	const char *tmp;
 
-		/* argv[0] must be the git command, but the argv array
-		 * belongs to the caller, and my be reused in
-		 * subsequent loop iterations. Save argv[0] and
-		 * restore it on error.
-		 */
+	strbuf_init(&cmd, 0);
+	strbuf_addf(&cmd, "git-%s", argv[0]);
 
-		tmp = argv[0];
-		argv[0] = git_command;
+	/* argv[0] must be the git command, but the argv array
+	 * belongs to the caller, and my be reused in
+	 * subsequent loop iterations. Save argv[0] and
+	 * restore it on error.
+	 */
+	tmp = argv[0];
+	argv[0] = cmd.buf;
 
-		trace_argv_printf(argv, -1, "trace: exec:");
+	trace_argv_printf(argv, -1, "trace: exec:");
 
-		/* execve() can only ever return if it fails */
-		execve(git_command, (char **)argv, environ);
+	/* execvp() can only ever return if it fails */
+	execvp(cmd.buf, (char **)argv);
 
-		trace_printf("trace: exec failed: %s\n", strerror(errno));
+	trace_printf("trace: exec failed: %s\n", strerror(errno));
 
-		argv[0] = tmp;
-	}
-	return -1;
+	argv[0] = tmp;
 
+	strbuf_release(&cmd);
+
+	return -1;
 }
 
 
diff --git a/exec_cmd.h b/exec_cmd.h
index da99287..a892355 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,6 +3,7 @@
 
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char* git_exec_path(void);
+extern void setup_path(const char *);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 extern int execl_git_cmd(const char *cmd, ...);
 
diff --git a/git.c b/git.c
index c7cabf5..4e10581 100644
--- a/git.c
+++ b/git.c
@@ -6,28 +6,6 @@
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
 
-static void prepend_to_path(const char *dir, int len)
-{
-	const char *old_path = getenv("PATH");
-	char *path;
-	int path_len = len;
-
-	if (!old_path)
-		old_path = "/usr/local/bin:/usr/bin:/bin";
-
-	path_len = len + strlen(old_path) + 1;
-
-	path = xmalloc(path_len + 1);
-
-	memcpy(path, dir, len);
-	path[len] = ':';
-	memcpy(path + len + 1, old_path, path_len - len);
-
-	setenv("PATH", path, 1);
-
-	free(path);
-}
-
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
@@ -408,7 +386,7 @@ int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] ? argv[0] : "git-help";
 	char *slash = strrchr(cmd, '/');
-	const char *exec_path = NULL;
+	const char *cmd_path = NULL;
 	int done_alias = 0;
 
 	/*
@@ -418,10 +396,7 @@ int main(int argc, const char **argv)
 	 */
 	if (slash) {
 		*slash++ = 0;
-		if (*cmd == '/')
-			exec_path = cmd;
-		else
-			exec_path = xstrdup(make_absolute_path(cmd));
+		cmd_path = cmd;
 		cmd = slash;
 	}
 
@@ -458,16 +433,12 @@ int main(int argc, const char **argv)
 	cmd = argv[0];
 
 	/*
-	 * We execute external git command via execv_git_cmd(),
-	 * which looks at "--exec-path" option, GIT_EXEC_PATH
-	 * environment, and $(gitexecdir) in Makefile while built,
-	 * in this order.  For scripted commands, we prepend
-	 * the value of the exec_path variable to the PATH.
+	 * We use PATH to find git commands, but we prepend some higher
+	 * precidence paths: the "--exec-path" option, the GIT_EXEC_PATH
+	 * environment, and the $(gitexecdir) from the Makefile at build
+	 * time.
 	 */
-	if (exec_path)
-		prepend_to_path(exec_path, strlen(exec_path));
-	exec_path = git_exec_path();
-	prepend_to_path(exec_path, strlen(exec_path));
+	setup_path(cmd_path);
 
 	while (1) {
 		/* See if it's an internal command */
-- 
1.5.3.4.401.g19778-dirty

Re: [PATCH 2/7] s/pattern/prefix/ in help's list_commands

[Junio C Hamano]

Scott R Parish <srp@srparish.net> writes:

> list_commands() currently accepts and ignores a "pattern" argument,
> and then hard codes a prefix as well as some magic numbers.

Correct observation.

Personally, I find this static function should not pretend to be
as flexible --- it is to list git subcommands anyway (and it
even knows about ".exe"), so rather than renaming the pattern
and using it, it might be simpler and cleaner to just drop the
parameter and be done with it.

[PATCH 2/7] remove unused/unneeded "pattern" argument of list_commands

[Scott R Parish]

list_commands() currently accepts and ignores a "pattern" argument,
and then hard codes a prefix as well as some magic numbers. This
hardcodes the prefix inside of the function and removes the magic
numbers.

Signed-off-by: Scott R Parish <srp@srparish.net>
---
 help.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/help.c b/help.c
index b0d2dd4..d6dfdff 100644
--- a/help.c
+++ b/help.c
@@ -93,10 +93,12 @@ static void pretty_print_string_list(struct cmdname **cmdname, int longest)
 	}
 }
 
-static void list_commands(const char *exec_path, const char *pattern)
+static void list_commands(const char *exec_path)
 {
 	unsigned int longest = 0;
 	char path[PATH_MAX];
+	const char *prefix = "git-";
+	int prefix_len = strlen(prefix);
 	int dirlen;
 	DIR *dir = opendir(exec_path);
 	struct dirent *de;
@@ -120,7 +122,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 		struct stat st;
 		int entlen;
 
-		if (prefixcmp(de->d_name, "git-"))
+		if (prefixcmp(de->d_name, prefix))
 			continue;
 		strcpy(path+dirlen, de->d_name);
 		if (stat(path, &st) || /* stat, not lstat */
@@ -128,14 +130,14 @@ static void list_commands(const char *exec_path, const char *pattern)
 		    !(st.st_mode & S_IXUSR))
 			continue;
 
-		entlen = strlen(de->d_name);
+		entlen = strlen(de->d_name) - prefix_len;
 		if (has_extension(de->d_name, ".exe"))
 			entlen -= 4;
 
 		if (longest < entlen)
 			longest = entlen;
 
-		add_cmdname(de->d_name + 4, entlen-4);
+		add_cmdname(de->d_name + prefix_len, entlen);
 	}
 	closedir(dir);
 
@@ -143,7 +145,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 	printf("----------------------------");
 	mput_char('-', strlen(exec_path));
 	putchar('\n');
-	pretty_print_string_list(cmdname, longest - 4);
+	pretty_print_string_list(cmdname, longest);
 	putchar('\n');
 }
 
@@ -210,7 +212,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 	else if (!strcmp(help_cmd, "--all") || !strcmp(help_cmd, "-a")) {
 		printf("usage: %s\n\n", git_usage_string);
 		if(exec_path)
-			list_commands(exec_path, "git-*");
+			list_commands(exec_path);
 		exit(0);
 	}
 
-- 
gitgui.0.8.4.11178.g9a1bf-dirty