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

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:
> >
> > > Refactor "bundle.c" so that we don't depend on `the_repository` anymore.
> > > This conversion is trivial for most of the part, as we already have a
> > > repository available in all calling conexts.
> > >
> > > The only exception is that we use `get_log_output_encoding()`, which
> > > implicitly depends on `the_repository`. Add an `extern` declaration for
> > > this function so that we can drop `USE_THE_REPOSITORY_VARIABLE` and not
> > > accidentally introduce more uses of `the_repository`.
> > >
> > > Signed-off-by: Patrick Steinhardt <[email protected]>
> > > ---
> > >  bundle.c | 32 +++++++++++++++++++++-----------
> > >  1 file changed, 21 insertions(+), 11 deletions(-)
> > >
> > > 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?