public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xdiff-interface: stop using the_repository
@ 2026-02-08 13:47 René Scharfe
  2026-02-09  9:48 ` Patrick Steinhardt
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: René Scharfe @ 2026-02-08 13:47 UTC (permalink / raw)
  To: Git List

Use the algorithm-agnostic is_null_oid() and push the dependency of
read_mmblob() on the_repository->objects to its callers.  This allows it
to be used with arbitrary object databases.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 apply.c              | 6 +++---
 builtin/checkout.c   | 6 +++---
 builtin/merge-file.c | 2 +-
 merge-ort.c          | 6 +++---
 notes-merge.c        | 6 +++---
 xdiff-interface.c    | 9 +++++----
 xdiff-interface.h    | 5 ++++-
 7 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/apply.c b/apply.c
index 3de4aa4d2e..ea90ed16be 100644
--- a/apply.c
+++ b/apply.c
@@ -3568,9 +3568,9 @@ static int three_way_merge(struct apply_state *state,
 	else if (oideq(base, theirs) || oideq(ours, theirs))
 		return resolve_to(image, ours);
 
-	read_mmblob(&base_file, base);
-	read_mmblob(&our_file, ours);
-	read_mmblob(&their_file, theirs);
+	read_mmblob(&base_file, the_repository->objects, base);
+	read_mmblob(&our_file, the_repository->objects, ours);
+	read_mmblob(&their_file, the_repository->objects, theirs);
 	merge_opts.variant = state->merge_variant;
 	status = ll_merge(&result, path,
 			  &base_file, "base",
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0ba4f03f2e..f7b313816e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -294,9 +294,9 @@ static int checkout_merged(int pos, const struct checkout *state,
 	if (is_null_oid(&threeway[1]) || is_null_oid(&threeway[2]))
 		return error(_("path '%s' does not have necessary versions"), path);
 
-	read_mmblob(&ancestor, &threeway[0]);
-	read_mmblob(&ours, &threeway[1]);
-	read_mmblob(&theirs, &threeway[2]);
+	read_mmblob(&ancestor, the_repository->objects, &threeway[0]);
+	read_mmblob(&ours, the_repository->objects, &threeway[1]);
+	read_mmblob(&theirs, the_repository->objects, &threeway[2]);
 
 	repo_config_get_bool(the_repository, "merge.renormalize", &renormalize);
 	ll_opts.renormalize = renormalize;
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 46775d0c79..c5dbd028fd 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -128,7 +128,7 @@ int cmd_merge_file(int argc,
 				ret = error(_("object '%s' does not exist"),
 					      argv[i]);
 			else if (!oideq(&oid, the_hash_algo->empty_blob))
-				read_mmblob(mmf, &oid);
+				read_mmblob(mmf, the_repository->objects, &oid);
 			else
 				read_mmfile(mmf, "/dev/null");
 		} else if (read_mmfile(mmf, fname)) {
diff --git a/merge-ort.c b/merge-ort.c
index e80e4f735a..a4103d56ed 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2136,9 +2136,9 @@ static int merge_3way(struct merge_options *opt,
 		name2 = mkpathdup("%s:%s", opt->branch2,  pathnames[2]);
 	}
 
-	read_mmblob(&orig, o);
-	read_mmblob(&src1, a);
-	read_mmblob(&src2, b);
+	read_mmblob(&orig, the_repository->objects, o);
+	read_mmblob(&src1, the_repository->objects, a);
+	read_mmblob(&src2, the_repository->objects, b);
 
 	merge_status = ll_merge(result_buf, path, &orig, base,
 				&src1, name1, &src2, name2,
diff --git a/notes-merge.c b/notes-merge.c
index 586939939f..47e9d7580a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -359,9 +359,9 @@ static int ll_merge_in_worktree(struct notes_merge_options *o,
 	mmfile_t base, local, remote;
 	enum ll_merge_result status;
 
-	read_mmblob(&base, &p->base);
-	read_mmblob(&local, &p->local);
-	read_mmblob(&remote, &p->remote);
+	read_mmblob(&base, the_repository->objects, &p->base);
+	read_mmblob(&local, the_repository->objects, &p->local);
+	read_mmblob(&remote, the_repository->objects, &p->remote);
 
 	status = ll_merge(&result_buf, oid_to_hex(&p->obj), &base, NULL,
 			  &local, o->local_ref, &remote, o->remote_ref,
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 1a35556380..cd7493730b 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -7,6 +6,7 @@
 #include "config.h"
 #include "hex.h"
 #include "odb.h"
+#include "repository.h"
 #include "strbuf.h"
 #include "xdiff-interface.h"
 #include "xdiff/xtypes.h"
@@ -177,18 +177,19 @@ int read_mmfile(mmfile_t *ptr, const char *filename)
 	return 0;
 }
 
-void read_mmblob(mmfile_t *ptr, const struct object_id *oid)
+void read_mmblob(mmfile_t *ptr, struct object_database *odb,
+		 const struct object_id *oid)
 {
 	unsigned long size;
 	enum object_type type;
 
-	if (oideq(oid, null_oid(the_hash_algo))) {
+	if (is_null_oid(oid)) {
 		ptr->ptr = xstrdup("");
 		ptr->size = 0;
 		return;
 	}
 
-	ptr->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
+	ptr->ptr = odb_read_object(odb, oid, &type, &size);
 	if (!ptr->ptr || type != OBJ_BLOB)
 		die("unable to read blob object %s", oid_to_hex(oid));
 	ptr->size = size;
diff --git a/xdiff-interface.h b/xdiff-interface.h
index dfc55daddf..fbc4ceec40 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -4,6 +4,8 @@
 #include "hash.h"
 #include "xdiff/xdiff.h"
 
+struct object_database;
+
 /*
  * xdiff isn't equipped to handle content over a gigabyte;
  * we make the cutoff 1GB - 1MB to give some breathing
@@ -45,7 +47,8 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
 		  void *consume_callback_data,
 		  xpparam_t const *xpp, xdemitconf_t const *xecfg);
 int read_mmfile(mmfile_t *ptr, const char *filename);
-void read_mmblob(mmfile_t *ptr, const struct object_id *oid);
+void read_mmblob(mmfile_t *ptr, struct object_database *odb,
+		 const struct object_id *oid);
 int buffer_is_binary(const char *ptr, unsigned long size);
 
 void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
-- 
2.52.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-08 13:47 [PATCH] xdiff-interface: stop using the_repository René Scharfe
@ 2026-02-09  9:48 ` Patrick Steinhardt
  2026-02-09 14:14   ` René Scharfe
  2026-02-09 11:15 ` Junio C Hamano
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Patrick Steinhardt @ 2026-02-09  9:48 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List

On Sun, Feb 08, 2026 at 02:47:40PM +0100, René Scharfe wrote:
> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index 1a35556380..cd7493730b 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -7,6 +6,7 @@
>  #include "config.h"
>  #include "hex.h"
>  #include "odb.h"
> +#include "repository.h"
>  #include "strbuf.h"
>  #include "xdiff-interface.h"
>  #include "xdiff/xtypes.h"

It's a bit surprising that we have to add this include, but I assume
that we use a function that's declared in this file?

> @@ -177,18 +177,19 @@ int read_mmfile(mmfile_t *ptr, const char *filename)
>  	return 0;
>  }
>  
> -void read_mmblob(mmfile_t *ptr, const struct object_id *oid)
> +void read_mmblob(mmfile_t *ptr, struct object_database *odb,
> +		 const struct object_id *oid)
>  {
>  	unsigned long size;
>  	enum object_type type;
>  
> -	if (oideq(oid, null_oid(the_hash_algo))) {
> +	if (is_null_oid(oid)) {
>  		ptr->ptr = xstrdup("");
>  		ptr->size = 0;
>  		return;
>  	}

Arguably the commit coudl've been split up into three:

  1. The change to `is_null_oid()`.

  2. Adding the ODB to the parameter.

  3. Removing the macro and adding the include.

So that each of those could have a bit more explanation. But I guess the
changes are smallish enough so that this borders on okay-ish, so I won't
insist on such a change.

Other than that this patch looks good to me, thanks!

Patrick

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-08 13:47 [PATCH] xdiff-interface: stop using the_repository René Scharfe
  2026-02-09  9:48 ` Patrick Steinhardt
@ 2026-02-09 11:15 ` Junio C Hamano
  2026-02-09 15:21   ` René Scharfe
  2026-02-09 18:57 ` Elijah Newren
  2026-02-09 19:24 ` [PATCH v2] " René Scharfe
  3 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2026-02-09 11:15 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List

René Scharfe <l.s.r@web.de> writes:

> Use the algorithm-agnostic is_null_oid() and push the dependency of
> read_mmblob() on the_repository->objects to its callers.  This allows it
> to be used with arbitrary object databases.

> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index 1a35556380..cd7493730b 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> ...
> -void read_mmblob(mmfile_t *ptr, const struct object_id *oid)
> +void read_mmblob(mmfile_t *ptr, struct object_database *odb,
> +		 const struct object_id *oid)

A possible alternative may be to pass "struct repository *" here,
but this passes the (current) smallest piece of data necessary to
drive the helper function odb_read_object(), so it would be fine.

>  {
>  	unsigned long size;
>  	enum object_type type;
>  
> -	if (oideq(oid, null_oid(the_hash_algo))) {
> +	if (is_null_oid(oid)) {
>  		ptr->ptr = xstrdup("");
>  		ptr->size = 0;
>  		return;
>  	}
>  
> -	ptr->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
> +	ptr->ptr = odb_read_object(odb, oid, &type, &size);
>  	if (!ptr->ptr || type != OBJ_BLOB)
>  		die("unable to read blob object %s", oid_to_hex(oid));
>  	ptr->size = size;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-09  9:48 ` Patrick Steinhardt
@ 2026-02-09 14:14   ` René Scharfe
  0 siblings, 0 replies; 14+ messages in thread
From: René Scharfe @ 2026-02-09 14:14 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Git List

On 2/9/26 10:48 AM, Patrick Steinhardt wrote:
> On Sun, Feb 08, 2026 at 02:47:40PM +0100, René Scharfe wrote:
>> diff --git a/xdiff-interface.c b/xdiff-interface.c
>> index 1a35556380..cd7493730b 100644
>> --- a/xdiff-interface.c
>> +++ b/xdiff-interface.c
>> @@ -7,6 +6,7 @@
>>  #include "config.h"
>>  #include "hex.h"
>>  #include "odb.h"
>> +#include "repository.h"
>>  #include "strbuf.h"
>>  #include "xdiff-interface.h"
>>  #include "xdiff/xtypes.h"
> 
> It's a bit surprising that we have to add this include, but I assume
> that we use a function that's declared in this file?

Good point, we don't actually need it.  It's left over from an earlier
version that had a struct repository pointer as new parameter. :-|

René


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-09 11:15 ` Junio C Hamano
@ 2026-02-09 15:21   ` René Scharfe
  2026-02-09 17:45     ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: René Scharfe @ 2026-02-09 15:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List

On 2/9/26 12:15 PM, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
> 
>> Use the algorithm-agnostic is_null_oid() and push the dependency of
>> read_mmblob() on the_repository->objects to its callers.  This allows it
>> to be used with arbitrary object databases.
> 
>> diff --git a/xdiff-interface.c b/xdiff-interface.c
>> index 1a35556380..cd7493730b 100644
>> --- a/xdiff-interface.c
>> +++ b/xdiff-interface.c
>> ...
>> -void read_mmblob(mmfile_t *ptr, const struct object_id *oid)
>> +void read_mmblob(mmfile_t *ptr, struct object_database *odb,
>> +		 const struct object_id *oid)
> 
> A possible alternative may be to pass "struct repository *" here,
> but this passes the (current) smallest piece of data necessary to
> drive the helper function odb_read_object(), so it would be fine.
> 
>>  {
>>  	unsigned long size;
>>  	enum object_type type;
>>  
>> -	if (oideq(oid, null_oid(the_hash_algo))) {
>> +	if (is_null_oid(oid)) {
>>  		ptr->ptr = xstrdup("");
>>  		ptr->size = 0;
>>  		return;
>>  	}
>>  
>> -	ptr->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
>> +	ptr->ptr = odb_read_object(odb, oid, &type, &size);
>>  	if (!ptr->ptr || type != OBJ_BLOB)
>>  		die("unable to read blob object %s", oid_to_hex(oid));
>>  	ptr->size = size;

My initial version did that.  Then I realized that read_mmblob() is just
a thin odb_read_object() wrapper that converts null_oid to
empty_blob_oid and dies on non-blobs, though, so requiring a full repo
pointer seemed excessive.  And all callers also use other odb_*
functions already.

René


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-09 15:21   ` René Scharfe
@ 2026-02-09 17:45     ` Junio C Hamano
  2026-02-09 19:24       ` René Scharfe
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2026-02-09 17:45 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List

René Scharfe <l.s.r@web.de> writes:

>>> -	ptr->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
>>> +	ptr->ptr = odb_read_object(odb, oid, &type, &size);
>>>  	if (!ptr->ptr || type != OBJ_BLOB)
>>>  		die("unable to read blob object %s", oid_to_hex(oid));
>>>  	ptr->size = size;
>
> My initial version did that.  Then I realized that read_mmblob() is just
> a thin odb_read_object() wrapper that converts null_oid to
> empty_blob_oid and dies on non-blobs, though, so requiring a full repo
> pointer seemed excessive.  And all callers also use other odb_*
> functions already.

Absolutely.  Passing the narrowest thing the callee needs is the
right approach and that is what is done in the version posted.

Thanks.  I presume that a small and final reroll is expected, if
only to remove the now unnecessary #include, if not splitting it
into three parts?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-08 13:47 [PATCH] xdiff-interface: stop using the_repository René Scharfe
  2026-02-09  9:48 ` Patrick Steinhardt
  2026-02-09 11:15 ` Junio C Hamano
@ 2026-02-09 18:57 ` Elijah Newren
  2026-02-09 20:01   ` Junio C Hamano
  2026-02-15 18:42   ` René Scharfe
  2026-02-09 19:24 ` [PATCH v2] " René Scharfe
  3 siblings, 2 replies; 14+ messages in thread
From: Elijah Newren @ 2026-02-09 18:57 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List

On Sun, Feb 8, 2026 at 5:47 AM René Scharfe <l.s.r@web.de> wrote:
>
...
> diff --git a/merge-ort.c b/merge-ort.c
> index e80e4f735a..a4103d56ed 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -2136,9 +2136,9 @@ static int merge_3way(struct merge_options *opt,
>                 name2 = mkpathdup("%s:%s", opt->branch2,  pathnames[2]);
>         }
>
> -       read_mmblob(&orig, o);
> -       read_mmblob(&src1, a);
> -       read_mmblob(&src2, b);
> +       read_mmblob(&orig, the_repository->objects, o);
> +       read_mmblob(&src1, the_repository->objects, a);
> +       read_mmblob(&src2, the_repository->objects, b);
>
>         merge_status = ll_merge(result_buf, path, &orig, base,
>                                 &src1, name1, &src2, name2,

A minor point, but could we use opt->repo instead of the_repository in
merge-ort?

I've cleaned out all the_repository references before, except one in
prefetch_for_content_merges(), and would prefer folks not add more.
However, that one in prefetch_for_content_merges() and the use of
DEFAULT_ABBREV prevent us from removing USE_THE_REPOSITORY, so it's
understandable that folks keep adding them back -- in fact, others
have added a few others to this file already since I cleaned them out.
So, if you want to go ahead with this and then I submit a later patch
that cleans them all up, that's fine too.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-09 17:45     ` Junio C Hamano
@ 2026-02-09 19:24       ` René Scharfe
  0 siblings, 0 replies; 14+ messages in thread
From: René Scharfe @ 2026-02-09 19:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List

On 2/9/26 6:45 PM, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
> 
>>>> -	ptr->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
>>>> +	ptr->ptr = odb_read_object(odb, oid, &type, &size);
>>>>  	if (!ptr->ptr || type != OBJ_BLOB)
>>>>  		die("unable to read blob object %s", oid_to_hex(oid));
>>>>  	ptr->size = size;
>>
>> My initial version did that.  Then I realized that read_mmblob() is just
>> a thin odb_read_object() wrapper that converts null_oid to
>> empty_blob_oid and dies on non-blobs, though, so requiring a full repo
>> pointer seemed excessive.  And all callers also use other odb_*
>> functions already.
> 
> Absolutely.  Passing the narrowest thing the callee needs is the
> right approach and that is what is done in the version posted.
> 
> Thanks.  I presume that a small and final reroll is expected, if
> only to remove the now unnecessary #include, if not splitting it
> into three parts?

Right, and still keeping it all in one patch, now that it has become
slightly shorter. :)

René


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] xdiff-interface: stop using the_repository
  2026-02-08 13:47 [PATCH] xdiff-interface: stop using the_repository René Scharfe
                   ` (2 preceding siblings ...)
  2026-02-09 18:57 ` Elijah Newren
@ 2026-02-09 19:24 ` René Scharfe
  2026-02-10 13:28   ` Patrick Steinhardt
  3 siblings, 1 reply; 14+ messages in thread
From: René Scharfe @ 2026-02-09 19:24 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano, Patrick Steinhardt

Use the algorithm-agnostic is_null_oid() and push the dependency of
read_mmblob() on the_repository->objects to its callers.  This allows it
to be used with arbitrary object databases.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
Change since v1: don't add unnecessary #include

 apply.c              | 6 +++---
 builtin/checkout.c   | 6 +++---
 builtin/merge-file.c | 2 +-
 merge-ort.c          | 6 +++---
 notes-merge.c        | 6 +++---
 xdiff-interface.c    | 8 ++++----
 xdiff-interface.h    | 5 ++++-
 7 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/apply.c b/apply.c
index 3de4aa4d2e..ea90ed16be 100644
--- a/apply.c
+++ b/apply.c
@@ -3568,9 +3568,9 @@ static int three_way_merge(struct apply_state *state,
 	else if (oideq(base, theirs) || oideq(ours, theirs))
 		return resolve_to(image, ours);
 
-	read_mmblob(&base_file, base);
-	read_mmblob(&our_file, ours);
-	read_mmblob(&their_file, theirs);
+	read_mmblob(&base_file, the_repository->objects, base);
+	read_mmblob(&our_file, the_repository->objects, ours);
+	read_mmblob(&their_file, the_repository->objects, theirs);
 	merge_opts.variant = state->merge_variant;
 	status = ll_merge(&result, path,
 			  &base_file, "base",
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0ba4f03f2e..f7b313816e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -294,9 +294,9 @@ static int checkout_merged(int pos, const struct checkout *state,
 	if (is_null_oid(&threeway[1]) || is_null_oid(&threeway[2]))
 		return error(_("path '%s' does not have necessary versions"), path);
 
-	read_mmblob(&ancestor, &threeway[0]);
-	read_mmblob(&ours, &threeway[1]);
-	read_mmblob(&theirs, &threeway[2]);
+	read_mmblob(&ancestor, the_repository->objects, &threeway[0]);
+	read_mmblob(&ours, the_repository->objects, &threeway[1]);
+	read_mmblob(&theirs, the_repository->objects, &threeway[2]);
 
 	repo_config_get_bool(the_repository, "merge.renormalize", &renormalize);
 	ll_opts.renormalize = renormalize;
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 46775d0c79..c5dbd028fd 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -128,7 +128,7 @@ int cmd_merge_file(int argc,
 				ret = error(_("object '%s' does not exist"),
 					      argv[i]);
 			else if (!oideq(&oid, the_hash_algo->empty_blob))
-				read_mmblob(mmf, &oid);
+				read_mmblob(mmf, the_repository->objects, &oid);
 			else
 				read_mmfile(mmf, "/dev/null");
 		} else if (read_mmfile(mmf, fname)) {
diff --git a/merge-ort.c b/merge-ort.c
index e80e4f735a..a4103d56ed 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -2136,9 +2136,9 @@ static int merge_3way(struct merge_options *opt,
 		name2 = mkpathdup("%s:%s", opt->branch2,  pathnames[2]);
 	}
 
-	read_mmblob(&orig, o);
-	read_mmblob(&src1, a);
-	read_mmblob(&src2, b);
+	read_mmblob(&orig, the_repository->objects, o);
+	read_mmblob(&src1, the_repository->objects, a);
+	read_mmblob(&src2, the_repository->objects, b);
 
 	merge_status = ll_merge(result_buf, path, &orig, base,
 				&src1, name1, &src2, name2,
diff --git a/notes-merge.c b/notes-merge.c
index 586939939f..47e9d7580a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -359,9 +359,9 @@ static int ll_merge_in_worktree(struct notes_merge_options *o,
 	mmfile_t base, local, remote;
 	enum ll_merge_result status;
 
-	read_mmblob(&base, &p->base);
-	read_mmblob(&local, &p->local);
-	read_mmblob(&remote, &p->remote);
+	read_mmblob(&base, the_repository->objects, &p->base);
+	read_mmblob(&local, the_repository->objects, &p->local);
+	read_mmblob(&remote, the_repository->objects, &p->remote);
 
 	status = ll_merge(&result_buf, oid_to_hex(&p->obj), &base, NULL,
 			  &local, o->local_ref, &remote, o->remote_ref,
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 1a35556380..f043330f2a 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -177,18 +176,19 @@ int read_mmfile(mmfile_t *ptr, const char *filename)
 	return 0;
 }
 
-void read_mmblob(mmfile_t *ptr, const struct object_id *oid)
+void read_mmblob(mmfile_t *ptr, struct object_database *odb,
+		 const struct object_id *oid)
 {
 	unsigned long size;
 	enum object_type type;
 
-	if (oideq(oid, null_oid(the_hash_algo))) {
+	if (is_null_oid(oid)) {
 		ptr->ptr = xstrdup("");
 		ptr->size = 0;
 		return;
 	}
 
-	ptr->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
+	ptr->ptr = odb_read_object(odb, oid, &type, &size);
 	if (!ptr->ptr || type != OBJ_BLOB)
 		die("unable to read blob object %s", oid_to_hex(oid));
 	ptr->size = size;
diff --git a/xdiff-interface.h b/xdiff-interface.h
index dfc55daddf..fbc4ceec40 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -4,6 +4,8 @@
 #include "hash.h"
 #include "xdiff/xdiff.h"
 
+struct object_database;
+
 /*
  * xdiff isn't equipped to handle content over a gigabyte;
  * we make the cutoff 1GB - 1MB to give some breathing
@@ -45,7 +47,8 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
 		  void *consume_callback_data,
 		  xpparam_t const *xpp, xdemitconf_t const *xecfg);
 int read_mmfile(mmfile_t *ptr, const char *filename);
-void read_mmblob(mmfile_t *ptr, const struct object_id *oid);
+void read_mmblob(mmfile_t *ptr, struct object_database *odb,
+		 const struct object_id *oid);
 int buffer_is_binary(const char *ptr, unsigned long size);
 
 void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
-- 
2.52.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-09 18:57 ` Elijah Newren
@ 2026-02-09 20:01   ` Junio C Hamano
  2026-02-15 18:42     ` René Scharfe
  2026-02-15 18:42   ` René Scharfe
  1 sibling, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2026-02-09 20:01 UTC (permalink / raw)
  To: Elijah Newren; +Cc: René Scharfe, Git List

Elijah Newren <newren@gmail.com> writes:

> On Sun, Feb 8, 2026 at 5:47 AM René Scharfe <l.s.r@web.de> wrote:
>>
> ...
>> diff --git a/merge-ort.c b/merge-ort.c
>> index e80e4f735a..a4103d56ed 100644
>> --- a/merge-ort.c
>> +++ b/merge-ort.c
>> @@ -2136,9 +2136,9 @@ static int merge_3way(struct merge_options *opt,
>>                 name2 = mkpathdup("%s:%s", opt->branch2,  pathnames[2]);
>>         }
>>
>> -       read_mmblob(&orig, o);
>> -       read_mmblob(&src1, a);
>> -       read_mmblob(&src2, b);
>> +       read_mmblob(&orig, the_repository->objects, o);
>> +       read_mmblob(&src1, the_repository->objects, a);
>> +       read_mmblob(&src2, the_repository->objects, b);
>>
>>         merge_status = ll_merge(result_buf, path, &orig, base,
>>                                 &src1, name1, &src2, name2,
>
> A minor point, but could we use opt->repo instead of the_repository in
> merge-ort?

Great.  If we have already an appropriate structure with the
relevant data, using it is the most welcome.

> So, if you want to go ahead with this and then I submit a later patch
> that cleans them all up, that's fine too.

True too, but as long as it is so obvious that "opt" here has .repo
member that we can use, I do not see a reason not to.

Thanks.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xdiff-interface: stop using the_repository
  2026-02-09 19:24 ` [PATCH v2] " René Scharfe
@ 2026-02-10 13:28   ` Patrick Steinhardt
  2026-02-10 21:31     ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Steinhardt @ 2026-02-10 13:28 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Junio C Hamano

On Mon, Feb 09, 2026 at 08:24:52PM +0100, René Scharfe wrote:
> Use the algorithm-agnostic is_null_oid() and push the dependency of
> read_mmblob() on the_repository->objects to its callers.  This allows it
> to be used with arbitrary object databases.
> 
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Change since v1: don't add unnecessary #include

Looks good to me, thanks!

Patrick

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] xdiff-interface: stop using the_repository
  2026-02-10 13:28   ` Patrick Steinhardt
@ 2026-02-10 21:31     ` Junio C Hamano
  0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2026-02-10 21:31 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: René Scharfe, Git List

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Feb 09, 2026 at 08:24:52PM +0100, René Scharfe wrote:
>> Use the algorithm-agnostic is_null_oid() and push the dependency of
>> read_mmblob() on the_repository->objects to its callers.  This allows it
>> to be used with arbitrary object databases.
>> 
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>> Change since v1: don't add unnecessary #include
>
> Looks good to me, thanks!
>
> Patrick

Thanks.  Replaced and marked for 'next'.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-09 18:57 ` Elijah Newren
  2026-02-09 20:01   ` Junio C Hamano
@ 2026-02-15 18:42   ` René Scharfe
  1 sibling, 0 replies; 14+ messages in thread
From: René Scharfe @ 2026-02-15 18:42 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Git List

On 2/9/26 7:57 PM, Elijah Newren wrote:
> On Sun, Feb 8, 2026 at 5:47 AM René Scharfe <l.s.r@web.de> wrote:
>>
> ...
>> diff --git a/merge-ort.c b/merge-ort.c
>> index e80e4f735a..a4103d56ed 100644
>> --- a/merge-ort.c
>> +++ b/merge-ort.c
>> @@ -2136,9 +2136,9 @@ static int merge_3way(struct merge_options *opt,
>>                 name2 = mkpathdup("%s:%s", opt->branch2,  pathnames[2]);
>>         }
>>
>> -       read_mmblob(&orig, o);
>> -       read_mmblob(&src1, a);
>> -       read_mmblob(&src2, b);
>> +       read_mmblob(&orig, the_repository->objects, o);
>> +       read_mmblob(&src1, the_repository->objects, a);
>> +       read_mmblob(&src2, the_repository->objects, b);
>>
>>         merge_status = ll_merge(result_buf, path, &orig, base,
>>                                 &src1, name1, &src2, name2,
> 
> A minor point, but could we use opt->repo instead of the_repository in
> merge-ort?
> 
> I've cleaned out all the_repository references before, except one in
> prefetch_for_content_merges(), and would prefer folks not add more.

I can imagine that this whack-a-mole game is annoying.  The patch above
at least didn't actually add them, it just made them explicit.  Indirect
references may look better on the surface, but the functions that
contain them still can only be used with the_repository.

The only way I can see to avoid that pain would be to convert leaf
functions, only, i.e. those that reference the_repository and friends,
but don't call any other functions or macros that do.  This way a
transition to the_repository-free would be meaningful and permanent for
each function.

Converting on all levels of the call chain in parallel requires less
coordination and is probably more realistic in our distributed
development model, though.

Do you (anyone) know nice tools for listing the full call chain graph
of C functions?  cscope can probably be made to do that with some
scripting, but seems inefficient for that purpose.  Such a tool could
be used to check for indirect references and tell us if functions are
safe for use with other repositories.

René


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] xdiff-interface: stop using the_repository
  2026-02-09 20:01   ` Junio C Hamano
@ 2026-02-15 18:42     ` René Scharfe
  0 siblings, 0 replies; 14+ messages in thread
From: René Scharfe @ 2026-02-15 18:42 UTC (permalink / raw)
  To: Junio C Hamano, Elijah Newren; +Cc: Git List

On 2/9/26 9:01 PM, Junio C Hamano wrote:
> Elijah Newren <newren@gmail.com> writes:
> 
>> On Sun, Feb 8, 2026 at 5:47 AM René Scharfe <l.s.r@web.de> wrote:
>>>
>> ...
>>> diff --git a/merge-ort.c b/merge-ort.c
>>> index e80e4f735a..a4103d56ed 100644
>>> --- a/merge-ort.c
>>> +++ b/merge-ort.c
>>> @@ -2136,9 +2136,9 @@ static int merge_3way(struct merge_options *opt,
>>>                 name2 = mkpathdup("%s:%s", opt->branch2,  pathnames[2]);
>>>         }
>>>
>>> -       read_mmblob(&orig, o);
>>> -       read_mmblob(&src1, a);
>>> -       read_mmblob(&src2, b);
>>> +       read_mmblob(&orig, the_repository->objects, o);
>>> +       read_mmblob(&src1, the_repository->objects, a);
>>> +       read_mmblob(&src2, the_repository->objects, b);
>>>
>>>         merge_status = ll_merge(result_buf, path, &orig, base,
>>>                                 &src1, name1, &src2, name2,
>>
>> A minor point, but could we use opt->repo instead of the_repository in
>> merge-ort?
> 
> Great.  If we have already an appropriate structure with the
> relevant data, using it is the most welcome.
> 
>> So, if you want to go ahead with this and then I submit a later patch
>> that cleans them all up, that's fine too.
> 
> True too, but as long as it is so obvious that "opt" here has .repo
> member that we can use, I do not see a reason not to.
Thanks for solving that issue!  I would have sent a separate patch if
I had been quicker, because using opt->repo would not have been
obvious to me.

René


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-02-15 18:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-08 13:47 [PATCH] xdiff-interface: stop using the_repository René Scharfe
2026-02-09  9:48 ` Patrick Steinhardt
2026-02-09 14:14   ` René Scharfe
2026-02-09 11:15 ` Junio C Hamano
2026-02-09 15:21   ` René Scharfe
2026-02-09 17:45     ` Junio C Hamano
2026-02-09 19:24       ` René Scharfe
2026-02-09 18:57 ` Elijah Newren
2026-02-09 20:01   ` Junio C Hamano
2026-02-15 18:42     ` René Scharfe
2026-02-15 18:42   ` René Scharfe
2026-02-09 19:24 ` [PATCH v2] " René Scharfe
2026-02-10 13:28   ` Patrick Steinhardt
2026-02-10 21:31     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox