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

On Mon, Aug 17, 2026 at 10:26 PM Patrick Steinhardt [off-list ref] wrote:

On Mon, Aug 17, 2026 at 09:47:53AM -0700, Junio C Hamano wrote:

quoted

Patrick Steinhardt [off-list ref] writes:

quoted

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 <redacted>
---
 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?