[PATCH v2 2/4] run_builtin(): save "-h" detection result for later use
From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:49:49
Subsystem:
the rest · Maintainer:
Linus Torvalds
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 <redacted> --- 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