* [Outreachy PATCH] merge: stop using the_repository
@ 2026-10-15 14:25 Okhuomon Ajayi
2026-10-15 16:59 ` Usman Akinyemi
2026-10-15 19:20 ` Junio C Hamano
0 siblings, 2 replies; 7+ messages in thread
From: Okhuomon Ajayi @ 2026-10-15 14:25 UTC (permalink / raw)
To: gitster
Cc: git, christian.couder, usmanakinyemi202, siddharthasthana31,
kaartic.sivaraam, Okhuomon Ajayi
try_merge_command() and checkout_fast_forward() already receive a
repository, so use it instead of relying on global variables.
Add a repository parameter to merge_argument() and pass it
from try_merge_command(). Use the repository in
checkout_fast_forward() when parsing trees.
Remove the now-unnecessary USE_THE_REPOSITORY_VARIABLE
definition.
Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
---
merge.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/merge.c b/merge.c
index 0f5e823e63..85dad63a94 100644
--- a/merge.c
+++ b/merge.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "gettext.h"
#include "hash.h"
@@ -14,9 +12,9 @@
#include "tree-walk.h"
#include "unpack-trees.h"
-static const char *merge_argument(struct commit *commit)
+static const char *merge_argument(struct repository *r, struct commit *commit)
{
- return oid_to_hex(commit ? &commit->object.oid : the_hash_algo->empty_tree);
+ return oid_to_hex(commit ? &commit->object.oid : r->hash_algo->empty_tree);
}
int try_merge_command(struct repository *r,
@@ -32,11 +30,11 @@ int try_merge_command(struct repository *r,
for (size_t i = 0; i < xopts_nr; i++)
strvec_pushf(&cmd.args, "--%s", xopts[i]);
for (j = common; j; j = j->next)
- strvec_push(&cmd.args, merge_argument(j->item));
+ 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));
- trees[nr_trees] = repo_parse_tree_indirect(the_repository, head);
+ trees[nr_trees] = repo_parse_tree_indirect(r, head);
if (!trees[nr_trees++]) {
rollback_lock_file(&lock_file);
return -1;
}
- trees[nr_trees] = repo_parse_tree_indirect(the_repository, remote);
+ trees[nr_trees] = repo_parse_tree_indirect(r, remote);
if (!trees[nr_trees++]) {
rollback_lock_file(&lock_file);
return -1;
}
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;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Outreachy PATCH] merge: stop using the_repository
2026-10-15 14:25 [Outreachy PATCH] merge: stop using the_repository Okhuomon Ajayi
@ 2026-10-15 16:59 ` Usman Akinyemi
2026-10-15 17:20 ` Okhuomon Ajayi
2026-11-07 1:07 ` Junio C Hamano
2026-10-15 19:20 ` Junio C Hamano
1 sibling, 2 replies; 7+ messages in thread
From: Usman Akinyemi @ 2026-10-15 16:59 UTC (permalink / raw)
To: Okhuomon Ajayi
Cc: gitster, git, christian.couder, siddharthasthana31,
kaartic.sivaraam
>
> try_merge_command() and checkout_fast_forward() already receive a
> repository, so use it instead of relying on global variables.
>
> Add a repository parameter to merge_argument() and pass it
> from try_merge_command(). Use the repository in
> checkout_fast_forward() when parsing trees.
>
> Remove the now-unnecessary USE_THE_REPOSITORY_VARIABLE
> definition.
>
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
Good catch.
> ---
> 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;
> }
> --
> 2.54.0
>
looks good to me.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy PATCH] merge: stop using the_repository
2026-10-15 16:59 ` Usman Akinyemi
@ 2026-10-15 17:20 ` Okhuomon Ajayi
2026-11-07 1:07 ` Junio C Hamano
1 sibling, 0 replies; 7+ messages in thread
From: Okhuomon Ajayi @ 2026-10-15 17:20 UTC (permalink / raw)
To: Usman Akinyemi
Cc: gitster, git, christian.couder, siddharthasthana31,
kaartic.sivaraam
On Thu, Oct 15, 2026 at 5:59 PM Usman Akinyemi
<usmanakinyemi202@gmail.com> wrote:
> looks good to me.
Thank you
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy PATCH] merge: stop using the_repository
2026-10-15 14:25 [Outreachy PATCH] merge: stop using the_repository Okhuomon Ajayi
2026-10-15 16:59 ` Usman Akinyemi
@ 2026-10-15 19:20 ` Junio C Hamano
2026-10-15 19:56 ` Okhuomon Ajayi
1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2026-10-15 19:20 UTC (permalink / raw)
To: Okhuomon Ajayi
Cc: git, christian.couder, usmanakinyemi202, siddharthasthana31,
kaartic.sivaraam
Okhuomon Ajayi <okhuomonajayi54@gmail.com> writes:
> try_merge_command() and checkout_fast_forward() already receive a
> repository, so use it instead of relying on global variables.
>
> Add a repository parameter to merge_argument() and pass it
> from try_merge_command(). Use the repository in
> checkout_fast_forward() when parsing trees.
>
> Remove the now-unnecessary USE_THE_REPOSITORY_VARIABLE
> definition.
>
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
> ---
> merge.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
To: gitster@pobox.com
Cc: git@vger.kernel.org, christian.couder@gmail.com, usmanakinyemi202@gmail.com,
siddharthasthana31@gmail.com, kaartic.sivaraam@gmail.com, Okhuomon Ajayi
<okhuomonajayi54@gmail.com>
Please do not put me on "To:" alone unless the patch has been fully
reviewed and reviewered acked it. Placing reviewers on Cc: line (or
To: line) as "area experts" asking for review is fine, and if the
patch is in the area where I happen to be an area expert, my address
may appear there together with other reviewers, but the default
"To:" address on this list for a brand new patch should be the list
address itself.
> diff --git a/merge.c b/merge.c
> index 0f5e823e63..85dad63a94 100644
> --- a/merge.c
> +++ b/merge.c
> @@ -1,5 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> -
> #include "git-compat-util.h"
> #include "gettext.h"
> #include "hash.h"
> @@ -14,9 +12,9 @@
> #include "tree-walk.h"
> #include "unpack-trees.h"
>
> -static const char *merge_argument(struct commit *commit)
> +static const char *merge_argument(struct repository *r, struct commit *commit)
> {
> - return oid_to_hex(commit ? &commit->object.oid : the_hash_algo->empty_tree);
> + return oid_to_hex(commit ? &commit->object.oid : r->hash_algo->empty_tree);
> }
>
> int try_merge_command(struct repository *r,
> @@ -32,11 +30,11 @@ int try_merge_command(struct repository *r,
> for (size_t i = 0; i < xopts_nr; i++)
> strvec_pushf(&cmd.args, "--%s", xopts[i]);
> for (j = common; j; j = j->next)
> - strvec_push(&cmd.args, merge_argument(j->item));
> + 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));
>
> - trees[nr_trees] = repo_parse_tree_indirect(the_repository, head);
> + trees[nr_trees] = repo_parse_tree_indirect(r, head);
> if (!trees[nr_trees++]) {
> rollback_lock_file(&lock_file);
> return -1;
> }
> - trees[nr_trees] = repo_parse_tree_indirect(the_repository, remote);
> + trees[nr_trees] = repo_parse_tree_indirect(r, remote);
> if (!trees[nr_trees++]) {
> rollback_lock_file(&lock_file);
> return -1;
> }
> 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;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy PATCH] merge: stop using the_repository
2026-10-15 19:20 ` Junio C Hamano
@ 2026-10-15 19:56 ` Okhuomon Ajayi
0 siblings, 0 replies; 7+ messages in thread
From: Okhuomon Ajayi @ 2026-10-15 19:56 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, christian.couder, usmanakinyemi202, siddharthasthana31,
kaartic.sivaraam
On Thu, Oct 15, 2026 at 8:20 PM Junio C Hamano <gitster@pobox.com> wrote:
> Please do not put me on "To:" alone unless the patch has been fully
> reviewed and reviewered acked it. Placing reviewers on Cc: line (or
> To: line) as "area experts" asking for review is fine, and if the
> patch is in the area where I happen to be an area expert, my address
> may appear there together with other reviewers, but the default
> "To:" address on this list for a brand new patch should be the list
> address itself.
Thanks for pointing that out, i'll address new patches to the list
from now on , with reviewers on the Cc
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Outreachy PATCH] merge: stop using the_repository
2026-10-15 16:59 ` Usman Akinyemi
2026-10-15 17:20 ` Okhuomon Ajayi
@ 2026-11-07 1:07 ` Junio C Hamano
2026-11-09 8:17 ` Patrick Steinhardt
1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2026-11-07 1:07 UTC (permalink / raw)
To: Usman Akinyemi, Patrick Steinhardt
Cc: Okhuomon Ajayi, git, christian.couder, siddharthasthana31,
kaartic.sivaraam
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.
The attached patch is what I did while reviewing your patch, trying
to get unpack_trees.c to compile without USE_THE_REPOSITORY. This
was done on top of 'seen', so it is more of an RFC or technology
demonstration than a serious patch to be applied (yet).
Note that I did not say that the patch allows unpack_trees() to work
on a repository other than the_repository. That is because many
code paths called by unpack_trees() may still have hidden, hardcoded
dependencies on the_repository.
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.
Anyway, provided that we will someday make the functions called by
the functions your patch touched to remove the references to
the_repository also work on arbitrary repositories (and the attached
patch could be one of the changes necessary to go there), I think
your patch is taking us in a better direction. Let's mark the topic
as ready for 'next'.
Thanks.
unpack-trees.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 76 insertions(+), 18 deletions(-)
diff --git c/unpack-trees.c w/unpack-trees.c
index 2e40537dd4..7419793cda 100644
--- c/unpack-trees.c
+++ w/unpack-trees.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -82,6 +81,56 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = {
? ((o)->internal.msgs[(type)]) \
: (unpack_plumbing_errors[(type)]) )
+
+/*
+ * Some functions in these code paths get unpack_trees_options, which
+ * has up to two istates, and some get an istate. These istates can
+ * be used to find the repository to be used for this operation.
+ *
+ * As these are all working on the same repository, they must match.
+ * For now, with a super paranoid check, make sure all potential
+ * sources of the information match before using it.
+ *
+ * Once this proves working well, we could just use the first one we
+ * find in a production setting.
+ */
+static struct repository *unpack_repo(struct index_state *istate,
+ struct unpack_trees_options *o)
+{
+ struct repository *repo = NULL;
+ const char *camefrom = NULL;
+
+ if (istate) {
+ if (!istate->repo)
+ BUG("istate->repo == NULL");
+ repo = istate->repo;
+ camefrom = "istate->repo";
+ }
+ if (o) {
+ if (o->src_index) {
+ if (!o->src_index->repo)
+ BUG("o->src_index->repo == NULL");
+ if (repo && repo != o->src_index->repo)
+ BUG("%s != o->src_index->repo", camefrom);
+ repo = o->src_index->repo;
+ camefrom = "o->src_index->repo";
+ }
+
+ if (o->dst_index) {
+ if (!o->dst_index->repo)
+ BUG("o->dst_index->repo == NULL");
+ if (repo && repo != o->dst_index->repo)
+ BUG("%s != o->dst_index->repo", camefrom);
+ repo = o->dst_index->repo;
+ }
+ }
+ if (!repo)
+ BUG("no repository for unpack_trees()?");
+ return repo;
+
+}
+
+
static const char *super_prefixed(const char *path, const char *super_prefix)
{
/*
@@ -345,15 +394,16 @@ static void load_gitmodules_file(struct index_state *index,
struct checkout *state)
{
int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
+ struct repository *repo = unpack_repo(index, NULL);
if (pos >= 0) {
struct cache_entry *ce = index->cache[pos];
if (!state && ce->ce_flags & CE_WT_REMOVE) {
- repo_read_gitmodules(the_repository, 0);
+ repo_read_gitmodules(repo, 0);
} else if (state && (ce->ce_flags & CE_UPDATE)) {
- submodule_free(the_repository);
+ submodule_free(repo);
checkout_entry(ce, state, NULL, NULL);
- repo_read_gitmodules(the_repository, 0);
+ repo_read_gitmodules(repo, 0);
}
}
}
@@ -362,6 +412,7 @@ static struct progress *get_progress(struct unpack_trees_options *o,
struct index_state *index)
{
unsigned cnt = 0, total = 0;
+ struct repository *repo = unpack_repo(index, o);
if (!o->update || !o->verbose_update)
return NULL;
@@ -372,7 +423,7 @@ static struct progress *get_progress(struct unpack_trees_options *o,
total++;
}
- return start_delayed_progress(the_repository,
+ return start_delayed_progress(repo,
_("Updating files"), total);
}
@@ -430,6 +481,7 @@ static int check_updates(struct unpack_trees_options *o,
struct progress *progress;
struct checkout state = CHECKOUT_INIT;
int i, pc_workers, pc_threshold;
+ struct repository *repo = unpack_repo(index, o);
trace_performance_enter();
state.super_prefix = o->super_prefix;
@@ -473,7 +525,7 @@ static int check_updates(struct unpack_trees_options *o,
if (should_update_submodules())
load_gitmodules_file(index, &state);
- if (repo_has_promisor_remote(the_repository))
+ if (repo_has_promisor_remote(repo))
/*
* Prefetch the objects that are to be checked out in the loop
* below.
@@ -879,6 +931,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
struct traverse_info newinfo;
struct name_entry *p;
int nr_entries;
+ struct repository *repo = unpack_repo(NULL, o);
nr_entries = all_trees_same_as_cache_tree(n, dirmask, names, info);
if (nr_entries > 0) {
@@ -940,7 +993,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
const struct object_id *oid = NULL;
if (dirmask & 1)
oid = &names[i].oid;
- buf[nr_buf++] = fill_tree_descriptor(the_repository, t + i, oid);
+ buf[nr_buf++] = fill_tree_descriptor(repo, t + i, oid);
}
}
@@ -1773,24 +1826,25 @@ static int clear_ce_flags(struct index_state *istate,
static struct strbuf prefix = STRBUF_INIT;
char label[100];
int rval;
+ struct repository *repo = unpack_repo(istate, NULL);
strbuf_reset(&prefix);
if (show_progress)
istate->progress = start_delayed_progress(
- the_repository,
+ repo,
_("Updating index flags"),
istate->cache_nr);
xsnprintf(label, sizeof(label), "clear_ce_flags(0x%08lx,0x%08lx)",
(unsigned long)select_mask, (unsigned long)clear_mask);
- trace2_region_enter("unpack_trees", label, istate->repo);
+ trace2_region_enter("unpack_trees", label, repo);
rval = clear_ce_flags_1(istate,
istate->cache,
istate->cache_nr,
&prefix,
select_mask, clear_mask,
pl, 0, 0);
- trace2_region_leave("unpack_trees", label, istate->repo);
+ trace2_region_leave("unpack_trees", label, repo);
stop_progress(&istate->progress);
return rval;
@@ -1885,13 +1939,13 @@ static int verify_absent(const struct cache_entry *,
*/
int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
{
- struct repository *repo = o->src_index->repo;
+ struct repository *repo = unpack_repo(NULL, o);
int i, ret;
static struct cache_entry *dfc;
struct pattern_list pl;
int free_pattern_list = 0;
struct dir_struct dir = DIR_INIT;
- struct repo_config_values *cfg = repo_config_values(the_repository);
+ struct repo_config_values *cfg = repo_config_values(repo);
if (o->reset == UNPACK_RESET_INVALID)
BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
@@ -1936,7 +1990,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
populate_from_existing_patterns(o, &pl);
}
- index_state_init(&o->internal.result, o->src_index->repo);
+ index_state_init(&o->internal.result, repo);
o->internal.result.initialized = 1;
o->internal.result.timestamp.sec = o->src_index->timestamp.sec;
o->internal.result.timestamp.nsec = o->src_index->timestamp.nsec;
@@ -2081,7 +2135,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
move_index_extensions(&o->internal.result, o->src_index);
if (!ret) {
if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0) &&
- cache_tree_verify(the_repository,
+ cache_tree_verify(repo,
&o->internal.result) < 0) {
ret = -1;
goto done;
@@ -2137,10 +2191,11 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
int i;
unsigned old_show_all_errors;
int free_pattern_list = 0;
+ struct repository *repo = unpack_repo(NULL, o);
old_show_all_errors = o->internal.show_all_errors;
o->internal.show_all_errors = 1;
- index_state_init(&o->internal.result, o->src_index->repo);
+ index_state_init(&o->internal.result, repo);
/* Sanity checks */
if (!o->update || o->index_only || o->skip_sparse_checkout)
@@ -2333,10 +2388,11 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
struct dir_struct d;
char *pathbuf;
int cnt = 0;
+ struct repository *repo = unpack_repo(NULL, o);
if (S_ISGITLINK(ce->ce_mode)) {
struct object_id oid;
- int sub_head = repo_resolve_gitlink_ref(the_repository, ce->name,
+ int sub_head = repo_resolve_gitlink_ref(repo, ce->name,
"HEAD", &oid);
/*
* If we are not going to update the submodule, then
@@ -2424,6 +2480,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
struct unpack_trees_options *o)
{
const struct cache_entry *result;
+ struct repository *repo = unpack_repo(NULL, o);
/*
* It may be that the 'lstat()' succeeded even though
@@ -2432,7 +2489,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
*
* Ignore that lstat() if it matches.
*/
- if (repo_ignore_case(the_repository) && icase_exists(o, name, len, st))
+ if (repo_ignore_case(repo) && icase_exists(o, name, len, st))
return 0;
if (o->internal.dir &&
@@ -2652,6 +2709,7 @@ static int merged_sparse_dir(const struct cache_entry * const *src, int n,
void * tree_bufs[MAX_UNPACK_TREES + 1];
struct traverse_info info;
int i, ret;
+ struct repository *repo = unpack_repo(NULL, o);
/*
* Create the tree traversal information for traversing into *only* the
@@ -2665,7 +2723,7 @@ static int merged_sparse_dir(const struct cache_entry * const *src, int n,
/* Get the tree descriptors of the sparse directory in each of the merging trees */
for (i = 0; i < n; i++)
- tree_bufs[i] = fill_tree_descriptor(o->src_index->repo, &t[i],
+ tree_bufs[i] = fill_tree_descriptor(repo, &t[i],
src[i] && !is_null_oid(&src[i]->oid) ? &src[i]->oid : NULL);
ret = traverse_trees(o->src_index, n, t, &info);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Outreachy PATCH] merge: stop using the_repository
2026-11-07 1:07 ` Junio C Hamano
@ 2026-11-09 8:17 ` Patrick Steinhardt
0 siblings, 0 replies; 7+ messages in thread
From: Patrick Steinhardt @ 2026-11-09 8:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Usman Akinyemi, Okhuomon Ajayi, git, christian.couder,
siddharthasthana31, kaartic.sivaraam
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-11-09 8:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 14:25 [Outreachy PATCH] merge: stop using the_repository Okhuomon Ajayi
2026-10-15 16:59 ` Usman Akinyemi
2026-10-15 17:20 ` Okhuomon Ajayi
2026-11-07 1:07 ` Junio C Hamano
2026-11-09 8:17 ` Patrick Steinhardt
2026-10-15 19:20 ` Junio C Hamano
2026-10-15 19:56 ` Okhuomon Ajayi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox