[PATCH] make git start-up sequence a bit more robust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The handle options loop called setup_git_dir() and setup_pager() that
would silently trigger repository discovery while it cycled.  This patch
changes it to collect the options from the user, and uses the
information in a saner, more controlled order, namely:

 - GIT_DIR, GIT_WORK_TREE and GIT_PAGER environments are set as needed;
 - setup_git_directory() is run if needed;
 - setup_work_tree(0 is run if needed;
 - setup_pager() is run if needed; 

The last two wants to read config, which means setup_git_directory()
should come before them, which in turn means GIT_DIR must be set up
before that.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 Junio C Hamano <gitster@xxxxxxxxx> writes:

 > If you have a bare repository and try this there:
 >
 > 	$ PAGER=head git show HEAD:gcc/ChangeLog
 >
 > it works as expected, but if you explicitly ask for pagination, like
 > this:
 >
 > 	$ PAGER=head git -p show HEAD:gcc/ChangeLog
 >
 > you would get a very funky error message:
 >
 > 	fatal: ambiguous argument 'HEAD:gcc/ChangeLog': unknown revision or path not in the working tree.
 > 	Use '--' to separate paths from revisions
 >
 > as if we are working with a repository with working tree.
 >
 > I originally noticed this with ls-tree.  The symptom is a bit different:
 >
 > 	$ git -p ls-tree HEAD
 > 	fatal: Not a valid object name HEAD
 >
 > I _think_ what is happening is that setup_pager() tries to run
 > git_config(), which runs setup(), and then RUN_SETUP set for "ls-tree"
 > (or "show") internal command runs setup again.  HEAD is given to
 > resolve_ref() and git_path("%s", ref) makes it to ".git/HEAD", even
 > though in a bare repository git_dir should be set to ".", and of course
 > we cannot find such a path in the git directory.

 git.c |   73 ++++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/git.c b/git.c
index 15fec89..fd756cd 100644
--- a/git.c
+++ b/git.c
@@ -6,7 +6,21 @@
 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 int handle_options(const char*** argv, int* argc, int* envchanged)
+static int want_pager = -1;
+static const char *want_git_dir;
+static int want_git_dir_badly = 1;
+static const char *want_work_tree;
+
+static void no_env_change_from_alias(const char *alias_command)
+{
+	if (!alias_command)
+		return;
+	die("alias '%s' changes environment variables\n"
+	    "You can use '!git' in the alias to do this.",
+	    alias_command);
+}
+
+static int handle_options(const char ***argv, int *argc, const char *alias_command)
 {
 	int handled = 0;
 
@@ -35,46 +49,42 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
 				exit(0);
 			}
 		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
-			setup_pager();
+			want_pager = 1;
 		} else if (!strcmp(cmd, "--no-pager")) {
-			setenv("GIT_PAGER", "cat", 1);
-			if (envchanged)
-				*envchanged = 1;
+			want_pager = 0;
 		} else if (!strcmp(cmd, "--git-dir")) {
+			no_env_change_from_alias(alias_command);
 			if (*argc < 2) {
 				fprintf(stderr, "No directory given for --git-dir.\n" );
 				usage(git_usage_string);
 			}
-			setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
-			if (envchanged)
-				*envchanged = 1;
+			want_git_dir = (*argv)[1];
 			(*argv)++;
 			(*argc)--;
 			handled++;
 		} else if (!prefixcmp(cmd, "--git-dir=")) {
-			setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
-			if (envchanged)
-				*envchanged = 1;
+			no_env_change_from_alias(alias_command);
+			want_git_dir = cmd + 10;
 		} else if (!strcmp(cmd, "--work-tree")) {
+			no_env_change_from_alias(alias_command);
 			if (*argc < 2) {
 				fprintf(stderr, "No directory given for --work-tree.\n" );
 				usage(git_usage_string);
 			}
-			setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1);
-			if (envchanged)
-				*envchanged = 1;
+			want_work_tree = (*argv)[1];
 			(*argv)++;
 			(*argc)--;
+			handled++;
 		} else if (!prefixcmp(cmd, "--work-tree=")) {
-			setenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
-			if (envchanged)
-				*envchanged = 1;
+			no_env_change_from_alias(alias_command);
+			want_work_tree = cmd + 12;
 		} else if (!strcmp(cmd, "--bare")) {
 			static char git_dir[PATH_MAX+1];
+			no_env_change_from_alias(alias_command);
 			is_bare_repository_cfg = 1;
-			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
-			if (envchanged)
-				*envchanged = 1;
+			getcwd(git_dir, sizeof(git_dir));
+			want_git_dir = git_dir;
+			want_git_dir_badly = 0;
 		} else {
 			fprintf(stderr, "Unknown option: %s\n", cmd);
 			usage(git_usage_string);
@@ -153,7 +163,7 @@ static int split_cmdline(char *cmdline, const char ***argv)
 
 static int handle_alias(int *argcp, const char ***argv)
 {
-	int nongit = 0, envchanged = 0, ret = 0, saved_errno = errno;
+	int nongit = 0, ret = 0, saved_errno = errno;
 	const char *subdir;
 	int count, option_count;
 	const char** new_argv;
@@ -183,11 +193,7 @@ static int handle_alias(int *argcp, const char ***argv)
 			    alias_string + 1, alias_command);
 		}
 		count = split_cmdline(alias_string, &new_argv);
-		option_count = handle_options(&new_argv, &count, &envchanged);
-		if (envchanged)
-			die("alias '%s' changes environment variables\n"
-				 "You can use '!git' in the alias to do this.",
-				 alias_command);
+		option_count = handle_options(&new_argv, &count, alias_command);
 		memmove(new_argv - option_count, new_argv,
 				count * sizeof(char *));
 		new_argv -= option_count;
@@ -247,11 +253,12 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
 	prefix = NULL;
 	if (p->option & RUN_SETUP)
 		prefix = setup_git_directory();
-	if (p->option & USE_PAGER)
-		setup_pager();
 	if (p->option & NEED_WORK_TREE)
 		setup_work_tree();
 
+	if (0 < want_pager || (p->option & USE_PAGER))
+		setup_pager();
+
 	trace_argv_printf(argv, "trace: built-in: git");
 
 	status = p->fn(argc, argv, prefix);
@@ -434,6 +441,13 @@ int main(int argc, const char **argv)
 	}
 	cmd = argv[0];
 
+	if (want_git_dir)
+		setenv(GIT_DIR_ENVIRONMENT, want_git_dir, want_git_dir_badly);
+	if (want_work_tree)
+		setenv(GIT_WORK_TREE_ENVIRONMENT, want_work_tree, 1);
+	if (!want_pager)
+		setenv("GIT_PAGER", "cat", 1);
+
 	/*
 	 * We use PATH to find git commands, but we prepend some higher
 	 * precidence paths: the "--exec-path" option, the GIT_EXEC_PATH
@@ -446,6 +460,9 @@ int main(int argc, const char **argv)
 		/* See if it's an internal command */
 		handle_internal_command(argc, argv);
 
+		if (!done_alias && 0 < want_pager)
+			setup_pager();
+
 		/* .. then try the external ones */
 		execv_git_cmd(argv);
 
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux