Git development
 help / color / mirror / Atom feed
* [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