From: Junio C Hamano <gitster@pobox.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: [PATCH] make git start-up sequence a bit more robust
Date: Thu, 13 Dec 2007 21:03:55 -0800 [thread overview]
Message-ID: <7vsl25x3yc.fsf_-_@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <7vmysexdvw.fsf@gitster.siamese.dyndns.org> (Junio C. Hamano's message of "Thu, 13 Dec 2007 17:29:23 -0800")
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@pobox.com>
---
Junio C Hamano <gitster@pobox.com> 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);
next prev parent reply other threads:[~2007-12-14 5:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-29 12:21 [PATCH v2] Do check_repository_format() early Nguyễn Thái Ngọc Duy
2007-12-14 1:29 ` [Funky] "git -p cmd" inside a bare repository Junio C Hamano
2007-12-14 5:03 ` Junio C Hamano [this message]
2007-12-14 5:12 ` Jeff King
2007-12-14 5:14 ` Jeff King
2007-12-14 6:07 ` Junio C Hamano
2007-12-14 13:58 ` Nguyen Thai Ngoc Duy
2007-12-14 19:44 ` Johannes Schindelin
2007-12-14 20:18 ` Junio C Hamano
2007-12-14 20:37 ` Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vsl25x3yc.fsf_-_@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).