diff mbox series

[v5,1/7] git: esnure correct git directory setup with -h

Message ID 09c2ff9f89833b3ac918a399e10d1b6abe71b339.1638566165.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Sparse Index: diff and blame builtins | expand

Commit Message

Lessley Dennington Dec. 3, 2021, 9:15 p.m. UTC 
From: Lessley Dennington <lessleydennington@gmail.com>

Ensure correct git directory setup when -h is passed with commands. This
specifically applies to repos with special help text configuration
variables and to commands run with -h outside a repository. This
will also protect against test failures in the upcoming change to BUG in
prepare_repo_settings if no git directory exists.

Note: this diff is better seen when ignoring whitespace changes.

Co-authored-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
---
 git.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

Comments

Elijah Newren Dec. 4, 2021, 6:41 p.m. UTC | #1 
On Fri, Dec 3, 2021 at 1:16 PM Lessley Dennington via GitGitGadget
<gitgitgadget@gmail.com> wrote:

Simple typo in the subject of the commit message: "esnure" -> "ensure"

> From: Lessley Dennington <lessleydennington@gmail.com>
>
> Ensure correct git directory setup when -h is passed with commands. This
> specifically applies to repos with special help text configuration
> variables and to commands run with -h outside a repository. This
> will also protect against test failures in the upcoming change to BUG in
> prepare_repo_settings if no git directory exists.
>
> Note: this diff is better seen when ignoring whitespace changes.
>
> Co-authored-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>
> ---
>  git.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/git.c b/git.c
> index 60c2784be45..03a2dfa5174 100644
> --- a/git.c
> +++ b/git.c
> @@ -421,27 +421,28 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>         int status, help;
>         struct stat st;
>         const char *prefix;
> -
> +       int run_setup = (p->option & (RUN_SETUP | RUN_SETUP_GENTLY));
>         prefix = NULL;
>         help = argc == 2 && !strcmp(argv[1], "-h");
> -       if (!help) {
> -               if (p->option & RUN_SETUP)
> -                       prefix = setup_git_directory();
> -               else if (p->option & RUN_SETUP_GENTLY) {
> -                       int nongit_ok;
> -                       prefix = setup_git_directory_gently(&nongit_ok);
> -               }
> -               precompose_argv_prefix(argc, argv, NULL);
> -               if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY) &&
> -                   !(p->option & DELAY_PAGER_CONFIG))
> -                       use_pager = check_pager_config(p->cmd);
> -               if (use_pager == -1 && p->option & USE_PAGER)
> -                       use_pager = 1;
> -
> -               if ((p->option & (RUN_SETUP | RUN_SETUP_GENTLY)) &&
> -                   startup_info->have_repository) /* get_git_dir() may set up repo, avoid that */
> -                       trace_repo_setup(prefix);
> +       if (help && (run_setup & RUN_SETUP))
> +               /* demote to GENTLY to allow 'git cmd -h' outside repo */
> +               run_setup = RUN_SETUP_GENTLY;
> +
> +       if (run_setup & RUN_SETUP)
> +               prefix = setup_git_directory();
> +       else if (run_setup & RUN_SETUP_GENTLY) {
> +               int nongit_ok;
> +               prefix = setup_git_directory_gently(&nongit_ok);
>         }
> +       precompose_argv_prefix(argc, argv, NULL);
> +       if (use_pager == -1 && run_setup &&
> +               !(p->option & DELAY_PAGER_CONFIG))
> +               use_pager = check_pager_config(p->cmd);
> +       if (use_pager == -1 && p->option & USE_PAGER)
> +               use_pager = 1;
> +       if (run_setup && startup_info->have_repository)
> +               /* get_git_dir() may set up repo, avoid that */
> +               trace_repo_setup(prefix);
>         commit_pager_choice();
>
>         if (!help && get_super_prefix()) {
> --
> gitgitgadget
Junio C Hamano Dec. 4, 2021, 7:58 p.m. UTC | #2 
"Lessley Dennington via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> @@ -421,27 +421,28 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>  	int status, help;
>  	struct stat st;
>  	const char *prefix;
> -
> +	int run_setup = (p->option & (RUN_SETUP | RUN_SETUP_GENTLY));
>  	prefix = NULL;

Don't lose the blank line that separates the variable declarations
and the first statement.  Also, it probably makes sense to remove
the assignment of NULL to prefix from here (see below).

>  	help = argc == 2 && !strcmp(argv[1], "-h");
> -...
> +	if (help && (run_setup & RUN_SETUP))
> +		/* demote to GENTLY to allow 'git cmd -h' outside repo */
> +		run_setup = RUN_SETUP_GENTLY;

OK.

> +	if (run_setup & RUN_SETUP)
> +		prefix = setup_git_directory();
> +	else if (run_setup & RUN_SETUP_GENTLY) {
> +		int nongit_ok;
> +		prefix = setup_git_directory_gently(&nongit_ok);
>  	}

Here, we say "depending on how run_setup is specified, we compute
the prefix in different ways".  So the "we do not run setup, so set
prefix to NULL" naturally belongs here.  Perhaps something like...

	if (run_setup & RUN_SETUP) {
		prefix = setup_git_directory();
	} else if (run_setup & RUN_SETUP_GENTLY) {
		int nongit_ok;
		prefix = setup_git_directory_gently(&nongit_ok);
	} else {
		prefix = NULL;
	}

> +	precompose_argv_prefix(argc, argv, NULL);
> +	if (use_pager == -1 && run_setup &&
> +		!(p->option & DELAY_PAGER_CONFIG))
> +		use_pager = check_pager_config(p->cmd);
> +	if (use_pager == -1 && p->option & USE_PAGER)
> +		use_pager = 1;
> +	if (run_setup && startup_info->have_repository)
> +		/* get_git_dir() may set up repo, avoid that */
> +		trace_repo_setup(prefix);
>  	commit_pager_choice();
>  
>  	if (!help && get_super_prefix()) {
diff mbox series

Patch

diff --git a/git.c b/git.c
index 60c2784be45..03a2dfa5174 100644
--- a/git.c
+++ b/git.c
@@ -421,27 +421,28 @@  static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	int status, help;
 	struct stat st;
 	const char *prefix;
-
+	int run_setup = (p->option & (RUN_SETUP | RUN_SETUP_GENTLY));
 	prefix = NULL;
 	help = argc == 2 && !strcmp(argv[1], "-h");
-	if (!help) {
-		if (p->option & RUN_SETUP)
-			prefix = setup_git_directory();
-		else if (p->option & RUN_SETUP_GENTLY) {
-			int nongit_ok;
-			prefix = setup_git_directory_gently(&nongit_ok);
-		}
-		precompose_argv_prefix(argc, argv, NULL);
-		if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY) &&
-		    !(p->option & DELAY_PAGER_CONFIG))
-			use_pager = check_pager_config(p->cmd);
-		if (use_pager == -1 && p->option & USE_PAGER)
-			use_pager = 1;
-
-		if ((p->option & (RUN_SETUP | RUN_SETUP_GENTLY)) &&
-		    startup_info->have_repository) /* get_git_dir() may set up repo, avoid that */
-			trace_repo_setup(prefix);
+	if (help && (run_setup & RUN_SETUP))
+		/* demote to GENTLY to allow 'git cmd -h' outside repo */
+		run_setup = RUN_SETUP_GENTLY;
+
+	if (run_setup & RUN_SETUP)
+		prefix = setup_git_directory();
+	else if (run_setup & RUN_SETUP_GENTLY) {
+		int nongit_ok;
+		prefix = setup_git_directory_gently(&nongit_ok);
 	}
+	precompose_argv_prefix(argc, argv, NULL);
+	if (use_pager == -1 && run_setup &&
+		!(p->option & DELAY_PAGER_CONFIG))
+		use_pager = check_pager_config(p->cmd);
+	if (use_pager == -1 && p->option & USE_PAGER)
+		use_pager = 1;
+	if (run_setup && startup_info->have_repository)
+		/* get_git_dir() may set up repo, avoid that */
+		trace_repo_setup(prefix);
 	commit_pager_choice();
 
 	if (!help && get_super_prefix()) {