Re: [PATCH 3/3] show-index: remove global state variables

[Phillip Wood <phillip.wood123@gmail.com>]

On 20/01/2026 14:05, Shreyansh Paliwal wrote:
> As Git is in the process of removing global state,
> this function still relies on the global variables,
> the_repository and the_hash_algo.
> 
> Remove the associated macro and the UNUSED attribute from
> the repo parameter, and replace all uses of the_repository and
> the_hash_algo with repo and repo->hash_algo, respectively.

I don't think that is a good idea because repo will be NULL outside of a 
repository. For a lot of commands that does not matter because they 
require a repository to run but judging from the first patch in this 
series this command is supposed to be able to run outside a repository.

I'm increasingly of the opinion that adding a repository argument to the 
builtin commands was a mistake as they all just use a single repository 
so using "the_repository" seems perfectly reasonable. It leads to 
problems like the segfault in this patch and takes attention away from 
the much more useful task of moving our library code away from using 
"the_repository". If you're interested in contributing to that effort 
then there are a number of instances of "the_repository" in wt-status.c 
that can be trivially replaced by the repository instance in "struct 
wt_status" or the repository passed to the function. I'm not sure how 
easy it is to remove them all - you might need to change the code to 
pass a repository instance down the call chain in a few cases but there 
are certainly quite a few that can be easily and usefully cleaned up.

Thanks

Phillip

> This modernizes git show-index and makes it more compatible.
> 
> Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
> ---
>   builtin/show-index.c | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/builtin/show-index.c b/builtin/show-index.c
> index a9c2f18b73..96adae14c0 100644
> --- a/builtin/show-index.c
> +++ b/builtin/show-index.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
>   #define DISABLE_SIGN_COMPARE_WARNINGS
>   
>   #include "builtin.h"
> @@ -16,7 +15,7 @@ static const char *const show_index_usage[] = {
>   int cmd_show_index(int argc,
>   		   const char **argv,
>   		   const char *prefix,
> -		   struct repository *repo UNUSED)
> +		   struct repository *repo)
>   {
>   	int i;
>   	unsigned nr;
> @@ -37,7 +36,7 @@ int cmd_show_index(int argc,
>   		hash_algo = hash_algo_by_name(hash_name);
>   		if (hash_algo == GIT_HASH_UNKNOWN)
>   			die(_("Unknown hash algorithm"));
> -		repo_set_hash_algo(the_repository, hash_algo);
> +		repo_set_hash_algo(repo, hash_algo);
>   	}
>   
>   	if (fread(top_index, 2 * 4, 1, stdin) != 1)
> @@ -63,7 +62,7 @@ int cmd_show_index(int argc,
>   
>   	/* detection of hash algorithm
>   	Only works for small files, i.e without large offsets */
> -	if(!the_hash_algo && version == 2) {
> +	if(!repo->hash_algo && version == 2) {
>   		struct stat st;
>   		size_t file_base_size;
>   		size_t table_size;
> @@ -79,9 +78,9 @@ int cmd_show_index(int argc,
>   		hash_size = size_rem / (nr + 2);
>   
>   		if(hash_size == GIT_SHA1_RAWSZ) {
> -			repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> +			repo_set_hash_algo(repo, GIT_HASH_SHA1);
>   		} else if(hash_size == GIT_SHA256_RAWSZ) {
> -			repo_set_hash_algo(the_repository, GIT_HASH_SHA256);
> +			repo_set_hash_algo(repo, GIT_HASH_SHA256);
>   		} else {
>   			die(_("unable to detect hash algorithm, "
>   					"use --object-format option"));
> @@ -89,10 +88,10 @@ int cmd_show_index(int argc,
>   	}
>   
>   	/* Final fallback to SHA1 */
> -	if(!the_hash_algo)
> -		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> +	if(!repo->hash_algo)
> +		repo_set_hash_algo(repo, GIT_HASH_SHA1);
>   
> -	hashsz = the_hash_algo->rawsz;
> +	hashsz = repo->hash_algo->rawsz;
>   
>   	if (version == 1) {
>   		for (i = 0; i < nr; i++) {
> @@ -114,7 +113,7 @@ int cmd_show_index(int argc,
>   		for (i = 0; i < nr; i++) {
>   			if (fread(entries[i].oid.hash, hashsz, 1, stdin) != 1)
>   				die(_("unable to read sha1 %u/%u"), i, nr);
> -			entries[i].oid.algo = hash_algo_by_ptr(the_hash_algo);
> +			entries[i].oid.algo = hash_algo_by_ptr(repo->hash_algo);
>   		}
>   		for (i = 0; i < nr; i++)
>   			if (fread(&entries[i].crc, 4, 1, stdin) != 1)