Re: [PATCH v2 5/6] bundle: get (mostly) rid of `the_repository`

On Thu, Aug 20, 2026 at 10:42:49PM -0700, Elijah Newren wrote:
> On Mon, Aug 17, 2026 at 10:26 PM Patrick Steinhardt <[email protected]> wrote:
> > On Mon, Aug 17, 2026 at 09:47:53AM -0700, Junio C Hamano wrote:
> > > Patrick Steinhardt <[email protected]> writes:
> > > > diff --git a/bundle.c b/bundle.c
> > > > index b64716f252..a9330bf0d3 100644
> > > > --- a/bundle.c
> > > > +++ b/bundle.c
> > > > @@ -1,4 +1,3 @@
> > > > -#define USE_THE_REPOSITORY_VARIABLE
> > > >  #define DISABLE_SIGN_COMPARE_WARNINGS
> > > >
> > > >  #include "git-compat-util.h"
> > > > @@ -21,6 +20,13 @@
> > > >  #include "connected.h"
> > > >  #include "write-or-die.h"
> > > >
> > > > +/*
> > > > + * NEEDSWORK: this function implicitly depends on `the_repository` and is not
> > > > + * available because we dropped USE_THE_REPOSITORY_VARIABLE. We can remove the
> > > > + * declaration once it's accessible via `repo_config_values`.
> > > > + */
> > > > +extern const char *get_log_output_encoding(void);
> > > > +
> > >
> > > Doesn't this defeat the whole "drop #define USE_THE_REPOSITORY_VARIABLE
> > > as a mark that we are done with this file and no longer need to
> > > worry about it going forward because we won't be able to compile if
> > > somebody adds a new use?" premise?
> >
> > Yes and no. By removing the define early it allows us to not reintroduce
> > new references to `the_repository` by accident, but carve out a single
> > exception for one of the functions that still depends on it. The
> > alternative would be to not do that, and if so there is no guarantee
> > whatsoever that we won't introduce more references to `the_repository`
> > in this file.
> >
> > So I'm still leaning towards keeping this as-is, but I don't feel very
> > strongly about this. Let me know in case that argument doesn't sway you
> > and I'll adapt.
> 
> Would it make more sense to do this the way replay.c does:
> 
> #define USE_THE_REPOSITORY_VARIABLE
> <a bunch of includes>
> /*
>  * We technically need USE_THE_REPOSITORY_VARIABLE for <X>, but
>  * do not want to use the_repository.
>  */
> #define the_repository DO_NOT_USE_THE_REPOSITORY
> 
> and remove the declaration of get_log_output_encoding() that you
> added?  Alternatively, should replay.c be adapted to the way you are
> doing it here?

The benefit of removing `USE_THE_REPOSITORY_VARIABLE` completely over
stubbing out `the_repository` is that it will also remove a couple of
function declarations that implicitly rely on `the_repository`, like for
example `get_log_output_encoding()`. So I think it's a slightly better
mechainsm over redefining `the_repository`.

Patrick