Re: [PATCH 00/18] setup: drop uses of `the_repository`

[Patrick Steinhardt <ps@pks.im>]

On Mon, Apr 20, 2026 at 08:50:45AM -0700, Elijah Newren wrote:
> Thanks, I would have preferred this series to have used the "PATCH v2"
> prefix and for its cover letter to be sent in-response-to the cover
> letter of v1, but those are nitpicks that don't affect the overall
> series.

Yeah, true. I noticed that b4 didn't quite do its thing, but too late. I
think this happens when b4 is still busy sending out mails from the
preceding version, but I already switch branches.

> I thought to check one additional thing I didn't check when reviewing
> v1.  I looked for all cases where this series added "the_repository"
> to some function between the beginning and end of the series, and for
> which the function where the code was found already had a
> repo/repository parameter.  There were several of these in builtins,
> but those are already littered with cases where cmd_foo() has a
> repository parameter declared UNUSED and where we use the_repository
> anyway.  Those should be cleaned up, but that's outside the scope of
> this series.  However, I did spot one non-builtin case: in patch 16,
> repo_migrate_ref_storage_format() already has a repo parameter, so
> your call to initialize_repository_version() can use it instead of
> the_repository.

This is something I'm always torn on. The problem here is that more
often than not, series like this end up touching lots and lots of places
throughout the code base.

On the one hand this means that I'd have to reason through every
callsite to figure out whether any repo that is available in the
caller's context is the correct repository to use, or if it's even set
at all (e.g. in the case of `cmd_foo()`). On the other hand, even if I
do so, I cannot trivially prove that the refactoring is equivalent, and
the refactoring may introduce bugs at the callsites I'm adapting because
of it.

So that's why I typically lean towards fixing the subsystem itself, but
doing the trivial swap for callers so that we only have to worry about
one part. The only exception is callers where we don't have
USE_THE_REPOSITORY_VARIABLE defined anymore.

Patrick