When run_builtin() sees "-h" as the first argument, it assumes:
- this is the call for help usage
- the real git command will only print help usage then exit
So it skips all setup in this case. Unfortunately, some commands do
other things before calling parse_options(), which is often where the
help usage is printed. Some of those things may try to access the
repository unnecessarily. If a repository is broken, the command may
die() before it prints help usage, not really helpful.
Make real commands aware of this fast path so that they can handle it
properly (i.e., print help usage then exit immediately) if they were
going to do more initialization than git_config().
Demonstrating "git foo -h" fails depends on individual commands and
is generally difficult to do. Instead GIT_TRACE is used to check
if a command does set repo. If it does, it is supposed to fail if
repo setup code chokes.
"git upload-archive" fails for another reason, but will be fixed too
when "git upload-archive -h" is converted to use startup_info->help
Signed-off-by: Nguyán ThÃi Ngác Duy <pclouds@xxxxxxxxx>
---
cache.h | 1 +
git.c | 18 ++++++++++++++----
t/t3905-help.sh | 24 ++++++++++++++++++++++++
3 files changed, 39 insertions(+), 4 deletions(-)
create mode 100755 t/t3905-help.sh
diff --git a/cache.h b/cache.h
index 33decd9..bb57e34 100644
--- a/cache.h
+++ b/cache.h
@@ -1117,6 +1117,7 @@ const char *split_cmdline_strerror(int cmdline_errno);
/* git.c */
struct startup_info {
int have_repository;
+ int help; /* git foo -h */
};
extern struct startup_info *startup_info;
diff --git a/git.c b/git.c
index 50a1401..bb67540 100644
--- a/git.c
+++ b/git.c
@@ -246,13 +246,23 @@ struct cmd_struct {
static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
{
- int status, help;
+ int status;
struct stat st;
const char *prefix;
prefix = NULL;
- help = argc == 2 && !strcmp(argv[1], "-h");
- if (!help) {
+ startup_info->help = argc == 2 && !strcmp(argv[1], "-h");
+ if (startup_info->help) {
+ /*
+ * Fast path for "git foo -h", no setup is done.
+ * Other functions might set .git up automatically
+ * and potentially die() along the way. It's best
+ * to check this flag from the beginning, print its
+ * help usage and exit, nothing more.
+ */
+ ;
+ }
+ else {
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
if (p->option & RUN_SETUP_GENTLY) {
@@ -267,7 +277,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
}
commit_pager_choice();
- if (!help && p->option & NEED_WORK_TREE)
+ if (!startup_info->help && p->option & NEED_WORK_TREE)
setup_work_tree();
trace_argv_printf(argv, "trace: built-in: git");
diff --git a/t/t3905-help.sh b/t/t3905-help.sh
new file mode 100755
index 0000000..0dcbedf
--- /dev/null
+++ b/t/t3905-help.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='tests that git foo -h should work even in potentially broken repos'
+
+. ./test-lib.sh
+
+test_help() {
+ test_expect_"$1" "$2 -h" "
+ GIT_TRACE=\"`pwd`\"/$2.log test_must_fail git $2 -h &&
+ test \$exit_code = 129 &&
+ ! grep 'defaults to' $2.log
+ "
+}
+
+test_help failure branch
+test_help failure checkout-index
+test_help failure commit
+test_help failure gc
+test_help failure ls-files
+test_help failure merge
+test_help failure update-index
+test_help failure upload-archive
+
+test_done
--
1.7.0.2.445.gcbdb3
--
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