Re: [PATCH v2 1/4] git: pass in repo for RUN_SETUP_GENTLY
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 30, 2024, 19:40 UTC
- Message-ID
- <xmqqldz953qt.fsf@gitster.g>
- In-Reply-To
- <5d72c31c6f3b97b7f5f7d3b4fa9a8b1587597670.1727718030.git.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 35 quoted lines
> From: John Cai <johncai86@gmail.com>
>
> commands that have RUN_SETUP_GENTLY potentially need a repository.
> Modify the logic in run_builtin() to pass the repository to the builtin
> if a builtin has the RUN_SETUP_GENTLY property.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> git.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/git.c b/git.c
> index 2fbea24ec92..f58f169f3c7 100644
> --- a/git.c
> +++ b/git.c
> @@ -443,7 +443,7 @@ static int handle_alias(int *argcp, const char ***argv)
>
> static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct repository *repo)
> {
> - int status, help;
> + int status, help, repo_exists;
> struct stat st;
> const char *prefix;
> int run_setup = (p->option & (RUN_SETUP | RUN_SETUP_GENTLY));
> @@ -455,9 +455,13 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
>
> if (run_setup & RUN_SETUP) {
> prefix = setup_git_directory();
> + repo_exists = 1;
> } else if (run_setup & RUN_SETUP_GENTLY) {
> int nongit_ok;
> prefix = setup_git_directory_gently(&nongit_ok);
> +
> + if (!nongit_ok)
> + repo_exists = 1;Why not use the new variable and pass it directly to nongit_ok? The polarity of the new variable needs to be swapped, of course, but I think it makes reading the code to call p->fn() easier to grok
i.e.,
- rename repo_exists to "no_repo", and initialize it to non-zero. - remove "int nongit_ok"; pass &no_repo instead. - update the calling of p->fn() to
p->fn(argc, argv, prefix, no_repo ? NULL : repo);
Show 12 quoted lines
> } else {
> prefix = NULL;
> }
> @@ -480,7 +484,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
> trace2_cmd_name(p->cmd);
>
> validate_cache_entries(repo->index);
> - status = p->fn(argc, argv, prefix, (p->option & RUN_SETUP)? repo : NULL);
> + status = p->fn(argc,
> + argv,
> + prefix,
> + repo_exists ? repo : NULL);Keeping the call to a single line would be much better than spreding it across four lines.
Thanks
> validate_cache_entries(repo->index); > > if (status)