Re: [Outreachy PATCH] merge: stop using the_repository

[Patrick Steinhardt <ps@pks.im>]

On Fri, Nov 06, 2026 at 05:07:40PM -0800, Junio C Hamano wrote:
> Usman Akinyemi <usmanakinyemi202@gmail.com> writes:
> 
> >>  merge.c | 16 +++++++---------
> >>  1 file changed, 7 insertions(+), 9 deletions(-)
> >
> >> +               strvec_push(&cmd.args, merge_argument(r, j->item));
> >>         strvec_push(&cmd.args, "--");
> >>         strvec_push(&cmd.args, head_arg);
> >>         for (j = remotes; j; j = j->next)
> >> -               strvec_push(&cmd.args, merge_argument(j->item));
> >> +               strvec_push(&cmd.args, merge_argument(r, j->item));
> >>
> >>         cmd.git_cmd = 1;
> >>         ret = run_command(&cmd);
> >> @@ -68,18 +66,18 @@ int checkout_fast_forward(struct repository *r,
> >>         memset(&trees, 0, sizeof(trees));
> >>         memset(&t, 0, sizeof(t));
> >>         }
> >>         for (i = 0; i < nr_trees; i++) {
> >> -               if (repo_parse_tree(the_repository, trees[i]) < 0) {
> >> +               if (repo_parse_tree(r, trees[i]) < 0) {
> >>                         rollback_lock_file(&lock_file);
> >>                         return -1;
> >>                 }
> 
> This shows one thing that I really hate about the current
> USE_THE_REPOSITORY model.
> 
> This function still calls unpack_trees(), which explicitly refers to
> and depends on the_repository.  It is somewhat misleading to claim
> that merge.c no longer depends on the_repository because of that.
> Even if you pass an arbitrary repository to checkout_fast_forward(),
> the call would not work as intended because unpack_trees() called by
> it will only work with the_repository under the hood.
> 
> Yet, this file now compiles without USE_THE_REPOSITORY defined,
> giving a false sense of achievement.

It _is_ an achievement. That achievement isn't that this file is now
properly libified and that it can be used with an arbitrary repo.
Because even if the subsystem didn't reference `the_repository` at all
anymore, "libification" to me isn't only about that but also about
having a sane API.

But it is an achievement because we have one less file to worry about
with regards to our end goal of getting rid of `the_repository`, and
this file cannot grow new explicit references to it.

[snip]
> Again, that is why I really hate the USE_THE_REPOSITORY macro.  It
> does not help us measure our progress incrementally.  Perhaps we
> could hide certain symbols from their header files (like the
> functions I converted in this demonstration patch to rid them of
> the_repository dependency) for those who do not define the macro,
> so we can get a more accurate picture of our progress.  However,
> an earlier attempt to do exactly that on notes-utils.h in
> https://lore.kernel.org/git/xmqqjymt94l3.fsf@gitster.g/ was shot
> down by Patrick, so I am not sure what a good direction would be
> to provide a real incremental progress indicator.

The thing is that no matter what, unless we now start to hide every
single symbol that implicitly references `the_repository`, it will be
an imperfect measure anyway. And sure, in a perfect world we'd do
exactly that. But it would result in a ton of churn, and I don't think
it's easy to get right.

So from my perspective, the best direction here is "forward", because
the problem will solve itself automatically once libgit.a does not use
`the_repository` at all anymore.

Patrick