[PATCH 0/5] refs: stop using `the_repository`

[PATCH 0/5] refs: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

Hi,

this patch series removes use of `the_repository` in the refs subsystem
and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those
files.

Patrick

Patrick Steinhardt (5):
  refs: stop using `the_repository`
  refs/files: stop using `the_repository` in
    `parse_loose_ref_contents()`
  refs/files: stop using `the_repository`
  refs/packed: stop using `the_repository`
  refs/reftable: stop using `the_repository`

 refs.c                  | 16 ++++++-------
 refs/files-backend.c    | 29 ++++++++++++-----------
 refs/packed-backend.c   | 14 +++++------
 refs/refs-internal.h    |  3 ++-
 refs/reftable-backend.c | 51 +++++++++++++++++++++--------------------
 5 files changed, 56 insertions(+), 57 deletions(-)

-- 
2.46.0.rc1.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/5] refs: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

Convert "refs.c" to stop using `the_repository` in favor of the repo
that gets passed in via `struct ref_store`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index 915aeb4d1d..94a41934d6 100644
--- a/refs.c
+++ b/refs.c
@@ -2,8 +2,6 @@
  * The backend-independent part of the reference module.
  */
 
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "git-compat-util.h"
 #include "advice.h"
 #include "config.h"
@@ -1838,7 +1836,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 			    failure_errno != ENOTDIR)
 				return NULL;
 
-			oidclr(oid, the_repository->hash_algo);
+			oidclr(oid, refs->repo->hash_algo);
 			if (*flags & REF_BAD_NAME)
 				*flags |= REF_ISBROKEN;
 			return refname;
@@ -1848,7 +1846,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 
 		if (!(read_flags & REF_ISSYMREF)) {
 			if (*flags & REF_BAD_NAME) {
-				oidclr(oid, the_repository->hash_algo);
+				oidclr(oid, refs->repo->hash_algo);
 				*flags |= REF_ISBROKEN;
 			}
 			return refname;
@@ -1856,7 +1854,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
 
 		refname = sb_refname.buf;
 		if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
-			oidclr(oid, the_repository->hash_algo);
+			oidclr(oid, refs->repo->hash_algo);
 			return refname;
 		}
 		if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
@@ -2011,7 +2009,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
 		free(subrepo);
 		goto done;
 	}
-	refs = ref_store_init(subrepo, the_repository->ref_storage_format,
+	refs = ref_store_init(subrepo, repo->ref_storage_format,
 			      submodule_sb.buf,
 			      REF_STORE_READ | REF_STORE_ODB);
 	register_ref_store_map(&repo->submodule_ref_stores, "submodule",
@@ -2045,7 +2043,7 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)
 				      common_path.buf, REF_STORE_ALL_CAPS);
 		strbuf_release(&common_path);
 	} else {
-		refs = ref_store_init(wt->repo, the_repository->ref_storage_format,
+		refs = ref_store_init(wt->repo, wt->repo->ref_storage_format,
 				      wt->repo->commondir, REF_STORE_ALL_CAPS);
 	}
 
-- 
2.46.0.rc1.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/5] refs/files: stop using `the_repository` in `parse_loose_ref_contents()`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 4853 bytes --]

We implicitly rely on `the_repository` in `parse_loose_ref_contents()`
by calling `parse_oid_hex()`. Convert the function to instead use
`parse_oid_hex_algop()` and have callers pass in the hash algorithm to
use.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs.c               |  4 ++--
 refs/files-backend.c | 21 ++++++++++++---------
 refs/refs-internal.h |  3 ++-
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index 94a41934d6..e082fc59b0 100644
--- a/refs.c
+++ b/refs.c
@@ -1752,8 +1752,8 @@ static int refs_read_special_head(struct ref_store *ref_store,
 		goto done;
 	}
 
-	result = parse_loose_ref_contents(content.buf, oid, referent, type,
-					  failure_errno);
+	result = parse_loose_ref_contents(ref_store->repo->hash_algo, content.buf,
+					  oid, referent, type, failure_errno);
 
 done:
 	strbuf_release(&full_path);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index aa52d9be7c..3437c79699 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -552,7 +552,8 @@ static int read_ref_internal(struct ref_store *ref_store, const char *refname,
 	strbuf_rtrim(&sb_contents);
 	buf = sb_contents.buf;
 
-	ret = parse_loose_ref_contents(buf, oid, referent, type, &myerr);
+	ret = parse_loose_ref_contents(ref_store->repo->hash_algo, buf,
+				       oid, referent, type, &myerr);
 
 out:
 	if (ret && !myerr)
@@ -586,7 +587,8 @@ static int files_read_symbolic_ref(struct ref_store *ref_store, const char *refn
 	return !(type & REF_ISSYMREF);
 }
 
-int parse_loose_ref_contents(const char *buf, struct object_id *oid,
+int parse_loose_ref_contents(const struct git_hash_algo *algop,
+			     const char *buf, struct object_id *oid,
 			     struct strbuf *referent, unsigned int *type,
 			     int *failure_errno)
 {
@@ -604,7 +606,7 @@ int parse_loose_ref_contents(const char *buf, struct object_id *oid,
 	/*
 	 * FETCH_HEAD has additional data after the sha.
 	 */
-	if (parse_oid_hex(buf, oid, &p) ||
+	if (parse_oid_hex_algop(buf, oid, &p, algop) ||
 	    (*p != '\0' && !isspace(*p))) {
 		*type |= REF_ISBROKEN;
 		*failure_errno = EINVAL;
@@ -1998,7 +2000,8 @@ static int files_delete_reflog(struct ref_store *ref_store,
 	return ret;
 }
 
-static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
+static int show_one_reflog_ent(struct files_ref_store *refs, struct strbuf *sb,
+			       each_reflog_ent_fn fn, void *cb_data)
 {
 	struct object_id ooid, noid;
 	char *email_end, *message;
@@ -2008,8 +2011,8 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 
 	/* old SP new SP name <email> SP time TAB msg LF */
 	if (!sb->len || sb->buf[sb->len - 1] != '\n' ||
-	    parse_oid_hex(p, &ooid, &p) || *p++ != ' ' ||
-	    parse_oid_hex(p, &noid, &p) || *p++ != ' ' ||
+	    parse_oid_hex_algop(p, &ooid, &p, refs->base.repo->hash_algo) || *p++ != ' ' ||
+	    parse_oid_hex_algop(p, &noid, &p, refs->base.repo->hash_algo) || *p++ != ' ' ||
 	    !(email_end = strchr(p, '>')) ||
 	    email_end[1] != ' ' ||
 	    !(timestamp = parse_timestamp(email_end + 2, &message, 10)) ||
@@ -2108,7 +2111,7 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 				strbuf_splice(&sb, 0, 0, bp + 1, endp - (bp + 1));
 				scanp = bp;
 				endp = bp + 1;
-				ret = show_one_reflog_ent(&sb, fn, cb_data);
+				ret = show_one_reflog_ent(refs, &sb, fn, cb_data);
 				strbuf_reset(&sb);
 				if (ret)
 					break;
@@ -2120,7 +2123,7 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 				 * Process it, and we can end the loop.
 				 */
 				strbuf_splice(&sb, 0, 0, buf, endp - buf);
-				ret = show_one_reflog_ent(&sb, fn, cb_data);
+				ret = show_one_reflog_ent(refs, &sb, fn, cb_data);
 				strbuf_reset(&sb);
 				break;
 			}
@@ -2170,7 +2173,7 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
 		return -1;
 
 	while (!ret && !strbuf_getwholeline(&sb, logfp, '\n'))
-		ret = show_one_reflog_ent(&sb, fn, cb_data);
+		ret = show_one_reflog_ent(refs, &sb, fn, cb_data);
 	fclose(logfp);
 	strbuf_release(&sb);
 	return ret;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index fa975d69aa..309b382284 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -705,7 +705,8 @@ struct ref_store {
  * Parse contents of a loose ref file. *failure_errno maybe be set to EINVAL for
  * invalid contents.
  */
-int parse_loose_ref_contents(const char *buf, struct object_id *oid,
+int parse_loose_ref_contents(const struct git_hash_algo *algop,
+			     const char *buf, struct object_id *oid,
 			     struct strbuf *referent, unsigned int *type,
 			     int *failure_errno);
 
-- 
2.46.0.rc1.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/5] refs/files: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

Convert the files ref backend to stop using `the_repository` in favor of
the repo that gets passed in via `struct ref_store`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/files-backend.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 3437c79699..c73f95ecf2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "../git-compat-util.h"
 #include "../copy.h"
 #include "../environment.h"
@@ -248,7 +246,7 @@ static void loose_fill_ref_dir_regular_file(struct files_ref_store *refs,
 
 	if (!refs_resolve_ref_unsafe(&refs->base, refname, RESOLVE_REF_READING,
 				     &oid, &flag)) {
-		oidclr(&oid, the_repository->hash_algo);
+		oidclr(&oid, refs->base.repo->hash_algo);
 		flag |= REF_ISBROKEN;
 	} else if (is_null_oid(&oid)) {
 		/*
@@ -265,7 +263,7 @@ static void loose_fill_ref_dir_regular_file(struct files_ref_store *refs,
 	if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
 		if (!refname_is_safe(refname))
 			die("loose refname is dangerous: %s", refname);
-		oidclr(&oid, the_repository->hash_algo);
+		oidclr(&oid, refs->base.repo->hash_algo);
 		flag |= REF_BAD_NAME | REF_ISBROKEN;
 	}
 	add_entry_to_dir(dir, create_ref_entry(refname, &oid, flag));
@@ -1154,7 +1152,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
 
 	if (!refs_resolve_ref_unsafe(&refs->base, lock->ref_name, 0,
 				     &lock->old_oid, NULL))
-		oidclr(&lock->old_oid, the_repository->hash_algo);
+		oidclr(&lock->old_oid, refs->base.repo->hash_algo);
 	goto out;
 
  error_return:
-- 
2.46.0.rc1.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/5] refs/packed: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

Convert the packed ref backend to stop using `the_repository` in favor
of the repo that gets passed in via `struct ref_store`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/packed-backend.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index a0666407cd..89976aa359 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "../git-compat-util.h"
 #include "../config.h"
 #include "../dir.h"
@@ -794,7 +792,7 @@ static int packed_read_raw_ref(struct ref_store *ref_store, const char *refname,
 		return -1;
 	}
 
-	if (get_oid_hex(rec, oid))
+	if (get_oid_hex_algop(rec, oid, ref_store->repo->hash_algo))
 		die_invalid_line(refs->path, rec, snapshot->eof - rec);
 
 	*type = REF_ISPACKED;
@@ -879,7 +877,7 @@ static int next_record(struct packed_ref_iterator *iter)
 	p = iter->pos;
 
 	if (iter->eof - p < snapshot_hexsz(iter->snapshot) + 2 ||
-	    parse_oid_hex(p, &iter->oid, &p) ||
+	    parse_oid_hex_algop(p, &iter->oid, &p, iter->repo->hash_algo) ||
 	    !isspace(*p++))
 		die_invalid_line(iter->snapshot->refs->path,
 				 iter->pos, iter->eof - iter->pos);
@@ -896,7 +894,7 @@ static int next_record(struct packed_ref_iterator *iter)
 		if (!refname_is_safe(iter->base.refname))
 			die("packed refname is dangerous: %s",
 			    iter->base.refname);
-		oidclr(&iter->oid, the_repository->hash_algo);
+		oidclr(&iter->oid, iter->repo->hash_algo);
 		iter->base.flags |= REF_BAD_NAME | REF_ISBROKEN;
 	}
 	if (iter->snapshot->peeled == PEELED_FULLY ||
@@ -909,7 +907,7 @@ static int next_record(struct packed_ref_iterator *iter)
 	if (iter->pos < iter->eof && *iter->pos == '^') {
 		p = iter->pos + 1;
 		if (iter->eof - p < snapshot_hexsz(iter->snapshot) + 1 ||
-		    parse_oid_hex(p, &iter->peeled, &p) ||
+		    parse_oid_hex_algop(p, &iter->peeled, &p, iter->repo->hash_algo) ||
 		    *p++ != '\n')
 			die_invalid_line(iter->snapshot->refs->path,
 					 iter->pos, iter->eof - iter->pos);
@@ -921,13 +919,13 @@ static int next_record(struct packed_ref_iterator *iter)
 		 * we suppress it if the reference is broken:
 		 */
 		if ((iter->base.flags & REF_ISBROKEN)) {
-			oidclr(&iter->peeled, the_repository->hash_algo);
+			oidclr(&iter->peeled, iter->repo->hash_algo);
 			iter->base.flags &= ~REF_KNOWS_PEELED;
 		} else {
 			iter->base.flags |= REF_KNOWS_PEELED;
 		}
 	} else {
-		oidclr(&iter->peeled, the_repository->hash_algo);
+		oidclr(&iter->peeled, iter->repo->hash_algo);
 	}
 
 	return ITER_OK;
-- 
2.46.0.rc1.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/5] refs/packed: stop using `the_repository`

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 311 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> Convert the packed ref backend to stop using `the_repository` in favor
> of the repo that gets passed in via `struct ref_store`.
>

Nit: Might be nice to add that we move to using `get_oid_hex_algop` in
this commit. Since it was also mentioned in a prev commit

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 4/5] refs/packed: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On Tue, Jul 30, 2024 at 07:23:55AM -0400, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Convert the packed ref backend to stop using `the_repository` in favor
> > of the repo that gets passed in via `struct ref_store`.
> >
> 
> Nit: Might be nice to add that we move to using `get_oid_hex_algop` in
> this commit. Since it was also mentioned in a prev commit

Yeah, that'd make sense indeed. I'll refrain from sending out a new
version of this patch series just to amend the commit message, but have
queued the change locally and will send it out in case any other changes
are required for this patch series.

Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 5/5] refs/reftable: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 7641 bytes --]

Convert the reftable ref backend to stop using `the_repository` in favor
of the repo that gets passed in via `struct ref_store`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/reftable-backend.c | 51 +++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index fbe74c239d..bf4446afd3 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "../git-compat-util.h"
 #include "../abspath.h"
 #include "../chdir-notify.h"
@@ -201,7 +199,8 @@ static void fill_reftable_log_record(struct reftable_log_record *log, const stru
 	log->value.update.tz_offset = sign * atoi(tz_begin);
 }
 
-static int read_ref_without_reload(struct reftable_stack *stack,
+static int read_ref_without_reload(struct reftable_ref_store *refs,
+				   struct reftable_stack *stack,
 				   const char *refname,
 				   struct object_id *oid,
 				   struct strbuf *referent,
@@ -220,7 +219,7 @@ static int read_ref_without_reload(struct reftable_stack *stack,
 		*type |= REF_ISSYMREF;
 	} else if (reftable_ref_record_val1(&ref)) {
 		oidread(oid, reftable_ref_record_val1(&ref),
-			the_repository->hash_algo);
+			refs->base.repo->hash_algo);
 	} else {
 		/* We got a tombstone, which should not happen. */
 		BUG("unhandled reference value type %d", ref.value_type);
@@ -487,16 +486,16 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
 		switch (iter->ref.value_type) {
 		case REFTABLE_REF_VAL1:
 			oidread(&iter->oid, iter->ref.value.val1,
-				the_repository->hash_algo);
+				refs->base.repo->hash_algo);
 			break;
 		case REFTABLE_REF_VAL2:
 			oidread(&iter->oid, iter->ref.value.val2.value,
-				the_repository->hash_algo);
+				refs->base.repo->hash_algo);
 			break;
 		case REFTABLE_REF_SYMREF:
 			if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->ref.refname,
 						     RESOLVE_REF_READING, &iter->oid, &flags))
-				oidclr(&iter->oid, the_repository->hash_algo);
+				oidclr(&iter->oid, refs->base.repo->hash_algo);
 			break;
 		default:
 			BUG("unhandled reference value type %d", iter->ref.value_type);
@@ -508,7 +507,7 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
 		if (check_refname_format(iter->ref.refname, REFNAME_ALLOW_ONELEVEL)) {
 			if (!refname_is_safe(iter->ref.refname))
 				die(_("refname is dangerous: %s"), iter->ref.refname);
-			oidclr(&iter->oid, the_repository->hash_algo);
+			oidclr(&iter->oid, refs->base.repo->hash_algo);
 			flags |= REF_BAD_NAME | REF_ISBROKEN;
 		}
 
@@ -551,7 +550,7 @@ static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator,
 
 	if (iter->ref.value_type == REFTABLE_REF_VAL2) {
 		oidread(peeled, iter->ref.value.val2.target_value,
-			the_repository->hash_algo);
+			iter->refs->base.repo->hash_algo);
 		return 0;
 	}
 
@@ -659,7 +658,7 @@ static int reftable_be_read_raw_ref(struct ref_store *ref_store,
 	if (ret)
 		return ret;
 
-	ret = read_ref_without_reload(stack, refname, oid, referent, type);
+	ret = read_ref_without_reload(refs, stack, refname, oid, referent, type);
 	if (ret < 0)
 		return ret;
 	if (ret > 0) {
@@ -868,8 +867,8 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 		goto done;
 	}
 
-	ret = read_ref_without_reload(stack_for(refs, "HEAD", NULL), "HEAD", &head_oid,
-				      &head_referent, &head_type);
+	ret = read_ref_without_reload(refs, stack_for(refs, "HEAD", NULL), "HEAD",
+				      &head_oid, &head_referent, &head_type);
 	if (ret < 0)
 		goto done;
 	ret = 0;
@@ -936,7 +935,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
 			string_list_insert(&affected_refnames, new_update->refname);
 		}
 
-		ret = read_ref_without_reload(stack, rewritten_ref,
+		ret = read_ref_without_reload(refs, stack, rewritten_ref,
 					      &current_oid, &referent, &u->type);
 		if (ret < 0)
 			goto done;
@@ -1500,7 +1499,8 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
 		memcpy(logs[logs_nr].value.update.old_hash, old_ref.value.val1, GIT_MAX_RAWSZ);
 		logs_nr++;
 
-		ret = read_ref_without_reload(arg->stack, "HEAD", &head_oid, &head_referent, &head_type);
+		ret = read_ref_without_reload(arg->refs, arg->stack, "HEAD", &head_oid,
+					      &head_referent, &head_type);
 		if (ret < 0)
 			goto done;
 		append_head_reflog = (head_type & REF_ISSYMREF) && !strcmp(head_referent.buf, arg->oldname);
@@ -1790,15 +1790,16 @@ static struct ref_iterator *reftable_be_reflog_iterator_begin(struct ref_store *
 					ref_iterator_select, NULL);
 }
 
-static int yield_log_record(struct reftable_log_record *log,
+static int yield_log_record(struct reftable_ref_store *refs,
+			    struct reftable_log_record *log,
 			    each_reflog_ent_fn fn,
 			    void *cb_data)
 {
 	struct object_id old_oid, new_oid;
 	const char *full_committer;
 
-	oidread(&old_oid, log->value.update.old_hash, the_repository->hash_algo);
-	oidread(&new_oid, log->value.update.new_hash, the_repository->hash_algo);
+	oidread(&old_oid, log->value.update.old_hash, refs->base.repo->hash_algo);
+	oidread(&new_oid, log->value.update.new_hash, refs->base.repo->hash_algo);
 
 	/*
 	 * When both the old object ID and the new object ID are null
@@ -1841,7 +1842,7 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 			break;
 		}
 
-		ret = yield_log_record(&log, fn, cb_data);
+		ret = yield_log_record(refs, &log, fn, cb_data);
 		if (ret)
 			break;
 	}
@@ -1886,7 +1887,7 @@ static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,
 	}
 
 	for (i = logs_nr; i--;) {
-		ret = yield_log_record(&logs[i], fn, cb_data);
+		ret = yield_log_record(refs, &logs[i], fn, cb_data);
 		if (ret)
 			goto done;
 	}
@@ -2200,7 +2201,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
 		goto done;
 	if (reftable_ref_record_val1(&ref_record))
 		oidread(&oid, reftable_ref_record_val1(&ref_record),
-			the_repository->hash_algo);
+			ref_store->repo->hash_algo);
 	prepare_fn(refname, &oid, policy_cb_data);
 
 	while (1) {
@@ -2216,9 +2217,9 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
 		}
 
 		oidread(&old_oid, log.value.update.old_hash,
-			the_repository->hash_algo);
+			ref_store->repo->hash_algo);
 		oidread(&new_oid, log.value.update.new_hash,
-			the_repository->hash_algo);
+			ref_store->repo->hash_algo);
 
 		/*
 		 * Skip over the reflog existence marker. We will add it back
@@ -2250,9 +2251,9 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
 
 		*dest = logs[i];
 		oidread(&old_oid, logs[i].value.update.old_hash,
-			the_repository->hash_algo);
+			ref_store->repo->hash_algo);
 		oidread(&new_oid, logs[i].value.update.new_hash,
-			the_repository->hash_algo);
+			ref_store->repo->hash_algo);
 
 		if (should_prune_fn(&old_oid, &new_oid, logs[i].value.update.email,
 				    (timestamp_t)logs[i].value.update.time,
@@ -2269,7 +2270,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
 
 	if (flags & EXPIRE_REFLOGS_UPDATE_REF && last_hash &&
 	    reftable_ref_record_val1(&ref_record))
-		oidread(&arg.update_oid, last_hash, the_repository->hash_algo);
+		oidread(&arg.update_oid, last_hash, ref_store->repo->hash_algo);
 
 	arg.refs = refs;
 	arg.records = rewritten;
-- 
2.46.0.rc1.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/5] refs: stop using `the_repository`

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> this patch series removes use of `the_repository` in the refs subsystem
> and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those
> files.
>

So the idea is to slowly cleanup usages of `the_repository` and the
`USE_THE_REPOSITORY_VARIABLE` macro acts as a check for this. I think
the changes look great. I even ran clang-format on this series and apart
from line wrap suggestions, there were no issues.

Thanks for this.

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 0/5] refs: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 959 bytes --]

On Tue, Jul 30, 2024 at 07:26:50AM -0400, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Hi,
> >
> > this patch series removes use of `the_repository` in the refs subsystem
> > and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those
> > files.
> >
> 
> So the idea is to slowly cleanup usages of `the_repository` and the
> `USE_THE_REPOSITORY_VARIABLE` macro acts as a check for this. I think
> the changes look great. I even ran clang-format on this series and apart
> from line wrap suggestions, there were no issues.

Exactly. One intent is to demonstrate to reviewers that a file actually
got rid of `the_repository`. The second intent is to hide away APIs that
have an implicit dependency on `the_repository`, which would be easy to
miss otherwise.

The second part is not perfect because we only hide away a subset of
interfaces. That part can be extended over time as required though.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/5] refs: stop using `the_repository`

[James Liu]

On Tue Jul 30, 2024 at 3:22 PM AEST, Patrick Steinhardt wrote:
> Hi,
>
> this patch series removes use of `the_repository` in the refs subsystem
> and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those
> files.
>
> Patrick

Thanks Patrick, these improvements make sense to me.

Is there a priority order on removing `the_repository` from other parts
of the codebase?

Cheers,
James

Re: [PATCH 0/5] refs: stop using `the_repository`

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

On Wed, Jul 31, 2024 at 10:55:09AM +1000, James Liu wrote:
> On Tue Jul 30, 2024 at 3:22 PM AEST, Patrick Steinhardt wrote:
> > Hi,
> >
> > this patch series removes use of `the_repository` in the refs subsystem
> > and drops the `USE_THE_REPOSITORY_VARIABLE` macro define from those
> > files.
> >
> > Patrick
> 
> Thanks Patrick, these improvements make sense to me.
> 
> Is there a priority order on removing `the_repository` from other parts
> of the codebase?

I was tackling the refs API because I knew that it was a "leaf" package
that doesn't have a ton of dependencies on subsystems that may be using
`the_repository` itself. So I guess that the order that makes most sense
is from leaf subsystems up to the root such that we can adjust layer by
layer.

Figuring out what those leaf subsystems are is a different story though.
I typically tend to brute force it, see how far I get and if I succeed
then I don't mention all the failed tries that led to the patch series
;) Over time you then get some intuition for which parts to handle next,
even though I realize that this is not particularly useful as advice to
somebody not that familiar with the codebase.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]