From: Glen Choo <chooglen@google.com>
To: git@vger.kernel.org
Cc: Glen Choo <chooglen@google.com>
Subject: [RFC PATCH 1/2] refs: pass struct repository *r through to write_ref_to_lockfile()
Date: Tue, 21 Sep 2021 16:25:28 -0700 [thread overview]
Message-ID: <20210921232529.81811-2-chooglen@google.com> (raw)
In-Reply-To: <20210921232529.81811-1-chooglen@google.com>
In refs/files-backend.c, write_ref_to_lockfile() uses the_repository to
validate the oid of the ref being written. Thus, any function that
includes write_ref_to_lockfile() in its call chain has an implicit
dependence on the odb of the_repository. This is undesirable in the case
where we are using oids that are not in the_repository, e.g. when we are
updating refs in a submodule.
Let's fix this by passing struct repository * as a parameter through to
write_ref_to_lockfile().
To avoid breaking existing call sites, the existing function
signatures in refs.h are preserved. struct repository * is only passed
to new functions; these new functions have names prefixed with repo_.
Signed-off-by: Glen Choo <chooglen@google.com>
---
refs.c | 21 +++++++++++----------
refs.h | 18 ++++++++++++++----
refs/debug.c | 14 ++++++++------
refs/files-backend.c | 30 +++++++++++++++---------------
refs/packed-backend.c | 7 ++++---
refs/refs-internal.h | 7 ++++---
t/helper/test-ref-store.c | 2 +-
7 files changed, 57 insertions(+), 42 deletions(-)
diff --git a/refs.c b/refs.c
index 8b9f7c3a80..c217512cd3 100644
--- a/refs.c
+++ b/refs.c
@@ -2105,7 +2105,7 @@ static int run_transaction_hook(struct ref_transaction *transaction,
return ret;
}
-int ref_transaction_prepare(struct ref_transaction *transaction,
+int repo_ref_transaction_prepare(struct repository *r, struct ref_transaction *transaction,
struct strbuf *err)
{
struct ref_store *refs = transaction->ref_store;
@@ -2132,7 +2132,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
return -1;
}
- ret = refs->be->transaction_prepare(refs, transaction, err);
+ ret = refs->be->transaction_prepare(r, refs, transaction, err);
if (ret)
return ret;
@@ -2172,7 +2172,8 @@ int ref_transaction_abort(struct ref_transaction *transaction,
return ret;
}
-int ref_transaction_commit(struct ref_transaction *transaction,
+int repo_ref_transaction_commit(struct repository *r,
+ struct ref_transaction *transaction,
struct strbuf *err)
{
struct ref_store *refs = transaction->ref_store;
@@ -2181,7 +2182,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
switch (transaction->state) {
case REF_TRANSACTION_OPEN:
/* Need to prepare first. */
- ret = ref_transaction_prepare(transaction, err);
+ ret = repo_ref_transaction_prepare(r, transaction, err);
if (ret)
return ret;
break;
@@ -2421,36 +2422,36 @@ int delete_refs(const char *msg, struct string_list *refnames,
return refs_delete_refs(get_main_ref_store(the_repository), msg, refnames, flags);
}
-int refs_rename_ref(struct ref_store *refs, const char *oldref,
+int refs_rename_ref(struct repository *r, struct ref_store *refs, const char *oldref,
const char *newref, const char *logmsg)
{
char *msg;
int retval;
msg = normalize_reflog_message(logmsg);
- retval = refs->be->rename_ref(refs, oldref, newref, msg);
+ retval = refs->be->rename_ref(r, refs, oldref, newref, msg);
free(msg);
return retval;
}
int rename_ref(const char *oldref, const char *newref, const char *logmsg)
{
- return refs_rename_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
+ return refs_rename_ref(the_repository, get_main_ref_store(the_repository), oldref, newref, logmsg);
}
-int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+int refs_copy_existing_ref(struct repository *r, struct ref_store *refs, const char *oldref,
const char *newref, const char *logmsg)
{
char *msg;
int retval;
msg = normalize_reflog_message(logmsg);
- retval = refs->be->copy_ref(refs, oldref, newref, msg);
+ retval = refs->be->copy_ref(r, refs, oldref, newref, msg);
free(msg);
return retval;
}
int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
{
- return refs_copy_existing_ref(get_main_ref_store(the_repository), oldref, newref, logmsg);
+ return refs_copy_existing_ref(the_repository, get_main_ref_store(the_repository), oldref, newref, logmsg);
}
diff --git a/refs.h b/refs.h
index 48970dfc7e..157d14d7a3 100644
--- a/refs.h
+++ b/refs.h
@@ -524,13 +524,13 @@ char *refs_shorten_unambiguous_ref(struct ref_store *refs,
char *shorten_unambiguous_ref(const char *refname, int strict);
/** rename ref, return 0 on success **/
-int refs_rename_ref(struct ref_store *refs, const char *oldref,
+int refs_rename_ref(struct repository *r, struct ref_store *refs, const char *oldref,
const char *newref, const char *logmsg);
int rename_ref(const char *oldref, const char *newref,
const char *logmsg);
/** copy ref, return 0 on success **/
-int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+int refs_copy_existing_ref(struct repository *r, struct ref_store *refs, const char *oldref,
const char *newref, const char *logmsg);
int copy_existing_ref(const char *oldref, const char *newref,
const char *logmsg);
@@ -706,8 +706,13 @@ int ref_transaction_verify(struct ref_transaction *transaction,
* Callers who don't need such fine-grained control over committing
* reference transactions should just call `ref_transaction_commit()`.
*/
-int ref_transaction_prepare(struct ref_transaction *transaction,
+int repo_ref_transaction_prepare(struct repository *r, struct ref_transaction *transaction,
struct strbuf *err);
+static inline int ref_transaction_prepare(struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ return repo_ref_transaction_prepare(the_repository, transaction, err);
+}
/*
* Commit all of the changes that have been queued in transaction, as
@@ -716,8 +721,13 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
* transaction, write an error message to `err`, and return one of the
* `TRANSACTION_*` constants
*/
-int ref_transaction_commit(struct ref_transaction *transaction,
+int repo_ref_transaction_commit(struct repository *r, struct ref_transaction *transaction,
struct strbuf *err);
+static inline int ref_transaction_commit(struct ref_transaction *transaction,
+ struct strbuf *err)
+{
+ return repo_ref_transaction_commit(the_repository, transaction, err);
+}
/*
* Abort `transaction`, which has been begun and possibly prepared,
diff --git a/refs/debug.c b/refs/debug.c
index 1a7a9e11cf..224ee76563 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -38,14 +38,15 @@ static int debug_init_db(struct ref_store *refs, struct strbuf *err)
return res;
}
-static int debug_transaction_prepare(struct ref_store *refs,
+static int debug_transaction_prepare(struct repository *r,
+ struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
int res;
transaction->ref_store = drefs->refs;
- res = drefs->refs->be->transaction_prepare(drefs->refs, transaction,
+ res = drefs->refs->be->transaction_prepare(r, drefs->refs, transaction,
err);
trace_printf_key(&trace_refs, "transaction_prepare: %d\n", res);
return res;
@@ -153,23 +154,24 @@ static int debug_delete_refs(struct ref_store *ref_store, const char *msg,
return res;
}
-static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
+static int debug_rename_ref(struct repository *r,
+ struct ref_store *ref_store, const char *oldref,
const char *newref, const char *logmsg)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res = drefs->refs->be->rename_ref(drefs->refs, oldref, newref,
+ int res = drefs->refs->be->rename_ref(r, drefs->refs, oldref, newref,
logmsg);
trace_printf_key(&trace_refs, "rename_ref: %s -> %s \"%s\": %d\n", oldref, newref,
logmsg, res);
return res;
}
-static int debug_copy_ref(struct ref_store *ref_store, const char *oldref,
+static int debug_copy_ref(struct repository *r, struct ref_store *ref_store, const char *oldref,
const char *newref, const char *logmsg)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
int res =
- drefs->refs->be->copy_ref(drefs->refs, oldref, newref, logmsg);
+ drefs->refs->be->copy_ref(r, drefs->refs, oldref, newref, logmsg);
trace_printf_key(&trace_refs, "copy_ref: %s -> %s \"%s\": %d\n", oldref, newref,
logmsg, res);
return res;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 74c0385873..2bd6115484 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1308,14 +1308,14 @@ static int rename_tmp_log(struct files_ref_store *refs, const char *newrefname)
return ret;
}
-static int write_ref_to_lockfile(struct ref_lock *lock,
+static int write_ref_to_lockfile(struct repository *r, struct ref_lock *lock,
const struct object_id *oid, struct strbuf *err);
static int commit_ref_update(struct files_ref_store *refs,
struct ref_lock *lock,
const struct object_id *oid, const char *logmsg,
struct strbuf *err);
-static int files_copy_or_rename_ref(struct ref_store *ref_store,
+static int files_copy_or_rename_ref(struct repository *r, struct ref_store *ref_store,
const char *oldrefname, const char *newrefname,
const char *logmsg, int copy)
{
@@ -1427,7 +1427,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
}
oidcpy(&lock->old_oid, &orig_oid);
- if (write_ref_to_lockfile(lock, &orig_oid, &err) ||
+ if (write_ref_to_lockfile(r, lock, &orig_oid, &err) ||
commit_ref_update(refs, lock, &orig_oid, logmsg, &err)) {
error("unable to write current sha1 into %s: %s", newrefname, err.buf);
strbuf_release(&err);
@@ -1448,7 +1448,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
flag = log_all_ref_updates;
log_all_ref_updates = LOG_REFS_NONE;
- if (write_ref_to_lockfile(lock, &orig_oid, &err) ||
+ if (write_ref_to_lockfile(r, lock, &orig_oid, &err) ||
commit_ref_update(refs, lock, &orig_oid, NULL, &err)) {
error("unable to write current sha1 into %s: %s", oldrefname, err.buf);
strbuf_release(&err);
@@ -1472,19 +1472,19 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
return ret;
}
-static int files_rename_ref(struct ref_store *ref_store,
+static int files_rename_ref(struct repository *r, struct ref_store *ref_store,
const char *oldrefname, const char *newrefname,
const char *logmsg)
{
- return files_copy_or_rename_ref(ref_store, oldrefname,
+ return files_copy_or_rename_ref(r, ref_store, oldrefname,
newrefname, logmsg, 0);
}
-static int files_copy_ref(struct ref_store *ref_store,
+static int files_copy_ref(struct repository *r, struct ref_store *ref_store,
const char *oldrefname, const char *newrefname,
const char *logmsg)
{
- return files_copy_or_rename_ref(ref_store, oldrefname,
+ return files_copy_or_rename_ref(r, ref_store, oldrefname,
newrefname, logmsg, 1);
}
@@ -1683,14 +1683,14 @@ static int files_log_ref_write(struct files_ref_store *refs,
* Write oid into the open lockfile, then close the lockfile. On
* errors, rollback the lockfile, fill in *err and return -1.
*/
-static int write_ref_to_lockfile(struct ref_lock *lock,
+static int write_ref_to_lockfile(struct repository *r, struct ref_lock *lock,
const struct object_id *oid, struct strbuf *err)
{
static char term = '\n';
struct object *o;
int fd;
- o = parse_object(the_repository, oid);
+ o = parse_object(r, oid);
if (!o) {
strbuf_addf(err,
"trying to write ref '%s' with nonexistent object %s",
@@ -2390,7 +2390,7 @@ static int check_old_oid(struct ref_update *update, struct object_id *oid,
* - If it is an update of head_ref, add a corresponding REF_LOG_ONLY
* update of HEAD.
*/
-static int lock_ref_for_update(struct files_ref_store *refs,
+static int lock_ref_for_update(struct repository *r, struct files_ref_store *refs,
struct ref_update *update,
struct ref_transaction *transaction,
const char *head_ref,
@@ -2496,7 +2496,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
* The reference already has the desired
* value, so we don't need to write it.
*/
- } else if (write_ref_to_lockfile(lock, &update->new_oid,
+ } else if (write_ref_to_lockfile(r, lock, &update->new_oid,
err)) {
char *write_err = strbuf_detach(err, NULL);
@@ -2577,7 +2577,7 @@ static void files_transaction_cleanup(struct files_ref_store *refs,
transaction->state = REF_TRANSACTION_CLOSED;
}
-static int files_transaction_prepare(struct ref_store *ref_store,
+static int files_transaction_prepare(struct repository *r, struct ref_store *ref_store,
struct ref_transaction *transaction,
struct strbuf *err)
{
@@ -2667,7 +2667,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = transaction->updates[i];
- ret = lock_ref_for_update(refs, update, transaction,
+ ret = lock_ref_for_update(r, refs, update, transaction,
head_ref, &affected_refnames, err);
if (ret)
goto cleanup;
@@ -2708,7 +2708,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,
if (is_packed_transaction_needed(refs->packed_ref_store,
packed_transaction)) {
- ret = ref_transaction_prepare(packed_transaction, err);
+ ret = repo_ref_transaction_prepare(r, packed_transaction, err);
/*
* A failure during the prepare step will abort
* itself, but not free. Do that now, and disconnect
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index f8aa97d799..e6aa1841ba 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1403,7 +1403,8 @@ static void packed_transaction_cleanup(struct packed_ref_store *refs,
transaction->state = REF_TRANSACTION_CLOSED;
}
-static int packed_transaction_prepare(struct ref_store *ref_store,
+static int packed_transaction_prepare(struct repository *r,
+ struct ref_store *ref_store,
struct ref_transaction *transaction,
struct strbuf *err)
{
@@ -1577,14 +1578,14 @@ static int packed_create_symref(struct ref_store *ref_store,
BUG("packed reference store does not support symrefs");
}
-static int packed_rename_ref(struct ref_store *ref_store,
+static int packed_rename_ref(struct repository *r, struct ref_store *ref_store,
const char *oldrefname, const char *newrefname,
const char *logmsg)
{
BUG("packed reference store does not support renaming references");
}
-static int packed_copy_ref(struct ref_store *ref_store,
+static int packed_copy_ref(struct repository *r, struct ref_store *ref_store,
const char *oldrefname, const char *newrefname,
const char *logmsg)
{
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 3155708345..1b7a68db8c 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -530,7 +530,8 @@ typedef struct ref_store *ref_store_init_fn(const char *gitdir,
typedef int ref_init_db_fn(struct ref_store *refs, struct strbuf *err);
-typedef int ref_transaction_prepare_fn(struct ref_store *refs,
+typedef int ref_transaction_prepare_fn(struct repository *r,
+ struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
@@ -553,10 +554,10 @@ typedef int create_symref_fn(struct ref_store *ref_store,
const char *logmsg);
typedef int delete_refs_fn(struct ref_store *ref_store, const char *msg,
struct string_list *refnames, unsigned int flags);
-typedef int rename_ref_fn(struct ref_store *ref_store,
+typedef int rename_ref_fn(struct repository *r, struct ref_store *ref_store,
const char *oldref, const char *newref,
const char *logmsg);
-typedef int copy_ref_fn(struct ref_store *ref_store,
+typedef int copy_ref_fn(struct repository *r, struct ref_store *ref_store,
const char *oldref, const char *newref,
const char *logmsg);
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index b314b81a45..ad47488e5b 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -99,7 +99,7 @@ static int cmd_rename_ref(struct ref_store *refs, const char **argv)
const char *newref = notnull(*argv++, "newref");
const char *logmsg = *argv++;
- return refs_rename_ref(refs, oldref, newref, logmsg);
+ return refs_rename_ref(the_repository, refs, oldref, newref, logmsg);
}
static int each_ref(const char *refname, const struct object_id *oid,
--
2.33.0.464.g1972c5931b-goog
next prev parent reply other threads:[~2021-09-21 23:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-21 23:25 [RFC PATCH 0/2] branch: implement in-process --recurse-submodules Glen Choo
2021-09-21 23:25 ` Glen Choo [this message]
2021-09-21 23:25 ` [RFC PATCH 2/2] branch: add --recurse-submodules option for branch creation Glen Choo
2021-09-22 11:10 ` Ævar Arnfjörð Bjarmason
2021-09-22 16:55 ` Glen Choo
2021-09-22 12:28 ` Philippe Blain
2021-09-22 17:24 ` Glen Choo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210921232529.81811-2-chooglen@google.com \
--to=chooglen@google.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).