Skip to contentSkip to conversation

refs: stop using `the_repository`

PATCH In review Patrick Steinhardt 3 participants 12 messages Jul 30, 2024 – Jul 31, 2024

lore
Share & tools
lore

Patrick Steinhardt

author
#0 lore raw

cover refs: stop using `the_repository`

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(-)
Signature · 1 line
2.46.0.rc1.dirty
author
#1 lore raw

PATCH 1/5 refs: stop using `the_repository`

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 <[email protected]>
---
 refs.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
refs.c +5 -7
index 915aeb4d1d..94a41934d6 100644
@@ -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);
 	}
Signature · 1 line
2.46.0.rc1.dirty
author
#2 lore raw

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

In reply to Patrick Steinhardt

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 <[email protected]>
---
 refs.c               |  4 ++--
 refs/files-backend.c | 21 ++++++++++++---------
 refs/refs-internal.h |  3 ++-
 3 files changed, 16 insertions(+), 12 deletions(-)
refs.c +2 -2
index 94a41934d6..e082fc59b0 100644
@@ -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);
refs/files-backend.c +12 -9
index aa52d9be7c..3437c79699 100644
@@ -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;
refs/refs-internal.h +2 -1
index fa975d69aa..309b382284 100644
@@ -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);
Signature · 1 line
2.46.0.rc1.dirty
author
#3 lore raw

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

In reply to Patrick Steinhardt

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 <[email protected]>
---
 refs/files-backend.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
refs/files-backend.c +3 -5
index 3437c79699..c73f95ecf2 100644
@@ -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:
Signature · 1 line
2.46.0.rc1.dirty
author
#4 lore raw

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

In reply to Patrick Steinhardt

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 <[email protected]>
---
 refs/packed-backend.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
refs/packed-backend.c +6 -8
index a0666407cd..89976aa359 100644
@@ -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;
Signature · 1 line
2.46.0.rc1.dirty
#5 lore raw

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

Quoted 3 lines · Patrick Steinhardt

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]

author
#6 lore raw

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

Quoted 8 lines · Karthik Nayak
Quoted 3 lines · Patrick Steinhardt

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

author
#7 lore raw

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

In reply to Patrick Steinhardt

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 <[email protected]>
---
 refs/reftable-backend.c | 51 +++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 25 deletions(-)
refs/reftable-backend.c +26 -25
index fbe74c239d..bf4446afd3 100644
@@ -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;
Signature · 1 line
2.46.0.rc1.dirty
#8 lore raw
In reply to Patrick Steinhardt
Quoted 6 lines · Patrick Steinhardt

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]

author
#9 lore raw
Quoted 9 lines · Karthik Nayak
Quoted 6 lines · Patrick Steinhardt

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.

Quoted 4 lines

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

#10 lore raw
In reply to Patrick Steinhardt
Quoted 7 lines · Patrick Steinhardt

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

author
#11 lore raw
Quoted 7 lines · James Liu
Quoted 6 lines · Patrick Steinhardt

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.

Quoted 6 lines
Quoted 1 line

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