Subject: Re: [PATCH 3/4] apply: remove the_repository global variable

From: Junio C Hamano <[email-masked]>
Date: Tue, 24 Sep 2024 11:50:02 -0700

Junio C Hamano <[email-masked]> writes:

>> -	if (init_apply_state(&state, the_repository, prefix))
>> +	if (init_apply_state(&state, repo, prefix))
>>  		exit(128);
>
> Hmph, the reason why we do not segfault with this patch is because
> repo will _always_ be the_repository due to the previous change.
>
> I am not sure if [1/4] is an improvement, though.  We used to be
> able to tell if we were running in a repository, or we were running
> in "nongit" mode, by looking at the NULL-ness of repo (which was
> UNUSED because we weren't taking advantage of that).  
>
> With [1/4], it no longer is possible.  From the point of view of API
> to call into builtin implementations, it smells like a regression.

We can avoid the regression by passing the discovered "nongit" (aka
"are we outside of a repository?") bit separately, perhaps like
this.  With such a change, I do not mind this change too much, but
pretending that we do not depend on the_repository (by removing the
textual mention of the_repository), but still depending on
the_repository (which points at the_repo) may be losing a bigger
picture, the true reason why we want to reduce the dependence on
the_repository.  We still need "if the hash-algo is not initialized
fall back to SHA-1" code here, but that is an overly broad fallback
that we would rather want to tighten to something like "we know we
have no reasonable value to initialize hash-algo in the_repository
if we are outside a repository, so initialize hash-algo if we are
outside any repository" (leaving it an error not have hash-algo in
"repo" if we _are_ in a repository).
   
diff --git c/git.c w/git.c
index 2fbea24ec9..579c6fa36d 100644
--- c/git.c
+++ w/git.c
@@ -447,6 +447,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
 	struct stat st;
 	const char *prefix;
 	int run_setup = (p->option & (RUN_SETUP | RUN_SETUP_GENTLY));
+	int nongit = 0;
 
 	help = argc == 2 && !strcmp(argv[1], "-h");
 	if (help && (run_setup & RUN_SETUP))
@@ -456,8 +457,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
 	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);
+		prefix = setup_git_directory_gently(&nongit);
 	} else {
 		prefix = NULL;
 	}
@@ -480,7 +480,7 @@ 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, run_setup ? repo : NULL, nongit);
 	validate_cache_entries(repo->index);
 
 	if (status)