Re: [PATCH 3/3] show-index: remove global state variables
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jan 21, 2026, 10:39 UTC
- Message-ID
- <7b5dd0c4-0ca0-458e-89db-621a70dac9ae@gmail.com>
- In-Reply-To
- <20260120140901.517928-4-shreyanshpaliwalcmsmn@gmail.com>
On 20/01/2026 14:05, Shreyansh Paliwal wrote:
Show 7 quoted lines
> 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
Show 78 quoted lines
> 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)