Subject: Re: [PATCH v5 1/7] git: esnure correct git directory setup with -h

From: Junio C Hamano <[email-masked]>
Date: Sat, 04 Dec 2021 11:58:19 -0800

"Lessley Dennington via GitGitGadget" <[email-masked]>
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()) {