public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail•com>
To: git@vger•kernel.org
Cc: gitster@pobox•com, phillip.wood123@gmail•com
Subject: Re: [PATCH 3/3] show-index: remove global state variables
Date: Wed, 21 Jan 2026 18:17:20 +0530	[thread overview]
Message-ID: <20260121124754.848110-1-shreyanshpaliwalcmsmn@gmail.com> (raw)
In-Reply-To: <7b5dd0c4-0ca0-458e-89db-621a70dac9ae@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 

That makes a lot of sense, especially for the commands
which are meant to run outside the repo as well.
In hindsight, the NULL repo issue and the segfault risk
should have been obvious to me, particularly given that I started
by creating the hash detection for no-repo cases :)

Anyways I will drop this patch in the next version.

> "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.

Yes sure, I will take a look and see where I can contribute in wt-status.c,
towards reducing global-state usage.

Best,
Shreyansh

  reply	other threads:[~2026-01-21 12:48 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20 14:05 [RFC][PATCH 0/3] show-index: modernize and implement auto-detection of hash algorithm Shreyansh Paliwal
2026-01-20 14:05 ` [PATCH 1/3] show-index: implement automatic hash detection Shreyansh Paliwal
2026-01-20 18:07   ` Junio C Hamano
2026-01-21  8:09     ` Patrick Steinhardt
2026-01-21 10:31       ` Shreyansh Paliwal
2026-01-23  7:22         ` Patrick Steinhardt
2026-01-23 16:08           ` Shreyansh Paliwal
2026-01-23 20:29       ` brian m. carlson
2026-01-21 10:28     ` Shreyansh Paliwal
2026-01-20 14:05 ` [PATCH 2/3] show-index: use gettext wrapping in error messages Shreyansh Paliwal
2026-01-20 14:05 ` [PATCH 3/3] show-index: remove global state variables Shreyansh Paliwal
2026-01-21 10:39   ` Phillip Wood
2026-01-21 12:47     ` Shreyansh Paliwal [this message]
2026-01-21 17:23     ` Junio C Hamano
2026-01-29 15:36 ` [PATCH] show-index: warn when falling back to SHA-1 outside a repository Shreyansh Paliwal
2026-01-29 23:03   ` Junio C Hamano
2026-01-30  8:59     ` Shreyansh Paliwal
2026-01-29 23:12   ` brian m. carlson
2026-01-30  9:04     ` Shreyansh Paliwal
2026-01-30 13:40       ` Patrick Steinhardt
2026-01-30 17:01         ` Junio C Hamano
2026-01-30 15:31   ` [PATCH V2 0/2] show-index: add warning and wrap error messages with gettext Shreyansh Paliwal
2026-01-30 15:31     ` [PATCH V2 1/2] show-index: warn when falling back to SHA-1 outside a repository Shreyansh Paliwal
2026-01-30 15:31     ` [PATCH V2 2/2] show-index: use gettext wrapping in user facing error messages Shreyansh Paliwal
2026-01-30 17:07     ` [PATCH V2 0/2] show-index: add warning and wrap error messages with gettext Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260121124754.848110-1-shreyanshpaliwalcmsmn@gmail.com \
    --to=shreyanshpaliwalcmsmn@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=phillip.wood123@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox