Re: [PATCH 00/20] packfile: avoid using the 'the_repository' global variable

[Taylor Blau <me@ttaylorr.com>]

On Mon, Oct 21, 2024 at 11:57:43AM +0200, Karthik Nayak wrote:
> The `packfile.c` file uses the global variable 'the_repository' extensively
> throughout the code. Let's remove all usecases of this, by modifying the
> required functions to accept a 'struct repository' instead. This is to clean up
> usage of global state.
>
> The first 18 patches are mostly passing a `struct repository` to each of the
> functions within `packfile.c` from other files. The last two patches move some
> global config variables and make them local. I'm not too well versed with this
> section of the code, so would be nice to get some eyes here.

I agree with the goal of this series, but I worry that as written it
will be quite disruptive to other topics on the list.

The standard way to avoid this disruption is to, for e.g. the first
change, do the following:

  - Introduce a new function repo_odb_pack_name() that takes in a
    'struct repository *', and rewrite odb_pack_name() in terms of it
    (passing 'the_repository' in as the argument).

  - Write a Coccinelle rule to replace all calls to odb_pack_name()
    with calls to repo_odb_pack_name().

  - Submit those patches without adjusting any non-obvious callers or
    ones that are not contained to a single compilation unit that you
    are already touching.

  - Wait until a new development cycle has begun, run spatch on the new
    rule to replace all other calls. Then optionally rename
    repo_odb_pack_name() to odb_pack_name().

I think Patrick (CC'd) has done one of these transitions recently, so
I'll defer to him in case I got any of the details wrong.

In the meantime, I'm going to hold this one out of seen as it may be
disruptive in the current state.

Thanks,
Taylor