From 8102d10ff8317c1e0ba5afb3a41b6a2bc523ff97 Mon Sep 17 00:00:00 2001 From: Bence Ferdinandy Date: Fri, 22 Nov 2024 13:28:44 +0100 Subject: refs: standardize output of refs_read_symbolic_ref When the symbolic reference we want to read with refs_read_symbolic_ref is actually not a symbolic reference, the files and the reftable backends return different values (1 and -1 respectively). Standardize the returned values so that 0 is success, -1 is a generic error and -2 is that the reference was actually non-symbolic. Signed-off-by: Bence Ferdinandy Signed-off-by: Junio C Hamano --- refs.h | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'refs.h') diff --git a/refs.h b/refs.h index 108dfc93b3..22c2b45b8b 100644 --- a/refs.h +++ b/refs.h @@ -83,6 +83,17 @@ int refs_read_ref_full(struct ref_store *refs, const char *refname, int refs_read_ref(struct ref_store *refs, const char *refname, struct object_id *oid); +#define NOT_A_SYMREF -2 + +/* + * Read the symbolic ref named "refname" and write its immediate referent into + * the provided buffer. Referent is left empty if "refname" is not a symbolic + * ref. It does not resolve the symbolic reference recursively in case the + * target is also a symbolic ref. + * + * Returns 0 on success, -2 if the "refname" is not a symbolic ref, + * -1 otherwise. + */ int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname, struct strbuf *referent); -- cgit v1.3 From d842cd130122c6f3d3019d467040570d7d001f7a Mon Sep 17 00:00:00 2001 From: Bence Ferdinandy Date: Fri, 22 Nov 2024 13:28:45 +0100 Subject: refs: atomically record overwritten ref in update_symref When updating a symref with update_symref it's currently not possible to know for sure what was the previous value that was overwritten. Extend refs_update_symref under a new function name, to record the value after the ref has been locked if the caller of refs_update_symref_extended requests it via a new variable in the function call. Make the return value of the function notify the caller, if the previous value was actually not a symbolic reference. Keep the original refs_update_symref function with the same signature, but now as a wrapper around refs_update_symref_extended. Signed-off-by: Bence Ferdinandy Signed-off-by: Junio C Hamano --- refs.c | 22 ++++++++++++++++++++-- refs.h | 4 ++++ 2 files changed, 24 insertions(+), 2 deletions(-) (limited to 'refs.h') diff --git a/refs.c b/refs.c index 5f729ed412..d80efd58f0 100644 --- a/refs.c +++ b/refs.c @@ -2115,6 +2115,13 @@ int peel_iterated_oid(struct repository *r, const struct object_id *base, struct int refs_update_symref(struct ref_store *refs, const char *ref, const char *target, const char *logmsg) +{ + return refs_update_symref_extended(refs, ref, target, logmsg, NULL); +} + +int refs_update_symref_extended(struct ref_store *refs, const char *ref, + const char *target, const char *logmsg, + struct strbuf *referent) { struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; @@ -2125,10 +2132,22 @@ int refs_update_symref(struct ref_store *refs, const char *ref, ref_transaction_update(transaction, ref, NULL, NULL, target, NULL, REF_NO_DEREF, logmsg, &err) || - ref_transaction_commit(transaction, &err)) { + ref_transaction_prepare(transaction, &err)) { ret = error("%s", err.buf); + goto cleanup; } + if (referent && refs_read_symbolic_ref(refs, ref, referent) == NOT_A_SYMREF) { + struct object_id oid; + if (!refs_read_ref(refs, ref, &oid)) { + strbuf_addstr(referent, oid_to_hex(&oid)); + ret = NOT_A_SYMREF; + } + } + + if (ref_transaction_commit(transaction, &err)) + ret = error("%s", err.buf); +cleanup: strbuf_release(&err); if (transaction) ref_transaction_free(transaction); @@ -2948,4 +2967,3 @@ int ref_update_expects_existing_old_ref(struct ref_update *update) return (update->flags & REF_HAVE_OLD) && (!is_null_oid(&update->old_oid) || update->old_target); } - diff --git a/refs.h b/refs.h index 22c2b45b8b..5c46ac9f34 100644 --- a/refs.h +++ b/refs.h @@ -584,6 +584,10 @@ int refs_copy_existing_ref(struct ref_store *refs, const char *oldref, int refs_update_symref(struct ref_store *refs, const char *refname, const char *target, const char *logmsg); +int refs_update_symref_extended(struct ref_store *refs, const char *refname, + const char *target, const char *logmsg, + struct strbuf *referent); + enum action_on_err { UPDATE_REFS_MSG_ON_ERR, UPDATE_REFS_DIE_ON_ERR, -- cgit v1.3 From ed2f6f8804cc142a02c701e808d110223b0256f8 Mon Sep 17 00:00:00 2001 From: Bence Ferdinandy Date: Fri, 22 Nov 2024 13:28:48 +0100 Subject: refs: add TRANSACTION_CREATE_EXISTS error Currently there is only one special error for transaction, for when there is a naming conflict, all other errors are dumped under a generic error. Add a new special error case for when the caller requests the reference to be updated only when it does not yet exist and the reference actually does exist. Signed-off-by: Bence Ferdinandy Signed-off-by: Junio C Hamano --- refs.h | 4 +++- refs/files-backend.c | 24 ++++++++++++++++-------- refs/reftable-backend.c | 6 ++++-- 3 files changed, 23 insertions(+), 11 deletions(-) (limited to 'refs.h') diff --git a/refs.h b/refs.h index 5c46ac9f34..b243739e4b 100644 --- a/refs.h +++ b/refs.h @@ -773,8 +773,10 @@ int ref_transaction_verify(struct ref_transaction *transaction, /* Naming conflict (for example, the ref names A and A/B conflict). */ #define TRANSACTION_NAME_CONFLICT -1 +/* When only creation was requested, but the ref already exists. */ +#define TRANSACTION_CREATE_EXISTS -2 /* All other errors. */ -#define TRANSACTION_GENERIC_ERROR -2 +#define TRANSACTION_GENERIC_ERROR -3 /* * Perform the preparatory stages of committing `transaction`. Acquire diff --git a/refs/files-backend.c b/refs/files-backend.c index 4cc43c32f2..23ae74089d 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2501,14 +2501,18 @@ static int split_symref_update(struct ref_update *update, static int check_old_oid(struct ref_update *update, struct object_id *oid, struct strbuf *err) { + int ret = TRANSACTION_GENERIC_ERROR; + if (!(update->flags & REF_HAVE_OLD) || oideq(oid, &update->old_oid)) return 0; - if (is_null_oid(&update->old_oid)) + if (is_null_oid(&update->old_oid)) { strbuf_addf(err, "cannot lock ref '%s': " "reference already exists", ref_update_original_update_refname(update)); + ret = TRANSACTION_CREATE_EXISTS; + } else if (is_null_oid(oid)) strbuf_addf(err, "cannot lock ref '%s': " "reference is missing but expected %s", @@ -2521,7 +2525,7 @@ static int check_old_oid(struct ref_update *update, struct object_id *oid, oid_to_hex(oid), oid_to_hex(&update->old_oid)); - return -1; + return ret; } /* @@ -2601,9 +2605,11 @@ static int lock_ref_for_update(struct files_ref_store *refs, ret = TRANSACTION_GENERIC_ERROR; goto out; } - } else if (check_old_oid(update, &lock->old_oid, err)) { - ret = TRANSACTION_GENERIC_ERROR; - goto out; + } else { + ret = check_old_oid(update, &lock->old_oid, err); + if (ret) { + goto out; + } } } else { /* @@ -2634,9 +2640,11 @@ static int lock_ref_for_update(struct files_ref_store *refs, update->old_target); ret = TRANSACTION_GENERIC_ERROR; goto out; - } else if (check_old_oid(update, &lock->old_oid, err)) { - ret = TRANSACTION_GENERIC_ERROR; - goto out; + } else { + ret = check_old_oid(update, &lock->old_oid, err); + if (ret) { + goto out; + } } /* diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index f74b8c4bb4..d764c2c3af 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1208,10 +1208,13 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, goto done; } } else if ((u->flags & REF_HAVE_OLD) && !oideq(¤t_oid, &u->old_oid)) { - if (is_null_oid(&u->old_oid)) + ret = TRANSACTION_NAME_CONFLICT; + if (is_null_oid(&u->old_oid)) { strbuf_addf(err, _("cannot lock ref '%s': " "reference already exists"), ref_update_original_update_refname(u)); + ret = TRANSACTION_CREATE_EXISTS; + } else if (is_null_oid(¤t_oid)) strbuf_addf(err, _("cannot lock ref '%s': " "reference is missing but expected %s"), @@ -1223,7 +1226,6 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, ref_update_original_update_refname(u), oid_to_hex(¤t_oid), oid_to_hex(&u->old_oid)); - ret = -1; goto done; } -- cgit v1.3 From 9963746c841dc786529827b7b6755d0a3e208ad4 Mon Sep 17 00:00:00 2001 From: Bence Ferdinandy Date: Fri, 22 Nov 2024 13:28:49 +0100 Subject: refs: add create_only option to refs_update_symref_extended Allow the caller to specify that it only wants to update the symref if it does not already exist. Silently ignore the error from the transaction API if the symref already exists. Signed-off-by: Bence Ferdinandy Signed-off-by: Junio C Hamano --- builtin/remote.c | 2 +- refs.c | 33 ++++++++++++++++++++++++--------- refs.h | 2 +- 3 files changed, 26 insertions(+), 11 deletions(-) (limited to 'refs.h') diff --git a/builtin/remote.c b/builtin/remote.c index 6d659d63ca..3dc1135b5c 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1476,7 +1476,7 @@ static int set_head(int argc, const char **argv, const char *prefix) goto cleanup; } was_detached = refs_update_symref_extended(refs, b_head.buf, b_remote_head.buf, - "remote set-head", &b_local_head); + "remote set-head", &b_local_head, 0); if (was_detached == -1) { result |= error(_("Could not set up %s"), b_head.buf); goto cleanup; diff --git a/refs.c b/refs.c index d80efd58f0..2efa6bcc5c 100644 --- a/refs.c +++ b/refs.c @@ -2116,26 +2116,38 @@ int peel_iterated_oid(struct repository *r, const struct object_id *base, struct int refs_update_symref(struct ref_store *refs, const char *ref, const char *target, const char *logmsg) { - return refs_update_symref_extended(refs, ref, target, logmsg, NULL); + return refs_update_symref_extended(refs, ref, target, logmsg, NULL, 0); } int refs_update_symref_extended(struct ref_store *refs, const char *ref, const char *target, const char *logmsg, - struct strbuf *referent) + struct strbuf *referent, int create_only) { struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - int ret = 0; + int ret = 0, prepret = 0; transaction = ref_store_transaction_begin(refs, &err); - if (!transaction || - ref_transaction_update(transaction, ref, NULL, NULL, - target, NULL, REF_NO_DEREF, - logmsg, &err) || - ref_transaction_prepare(transaction, &err)) { + if (!transaction) { + error_return: ret = error("%s", err.buf); goto cleanup; } + if (create_only) { + if (ref_transaction_create(transaction, ref, NULL, target, + REF_NO_DEREF, logmsg, &err)) + goto error_return; + prepret = ref_transaction_prepare(transaction, &err); + if (prepret && prepret != TRANSACTION_CREATE_EXISTS) + goto error_return; + } else { + if (ref_transaction_update(transaction, ref, NULL, NULL, + target, NULL, REF_NO_DEREF, + logmsg, &err) || + ref_transaction_prepare(transaction, &err)) + goto error_return; + } + if (referent && refs_read_symbolic_ref(refs, ref, referent) == NOT_A_SYMREF) { struct object_id oid; if (!refs_read_ref(refs, ref, &oid)) { @@ -2144,8 +2156,11 @@ int refs_update_symref_extended(struct ref_store *refs, const char *ref, } } + if (prepret == TRANSACTION_CREATE_EXISTS) + goto cleanup; + if (ref_transaction_commit(transaction, &err)) - ret = error("%s", err.buf); + goto error_return; cleanup: strbuf_release(&err); diff --git a/refs.h b/refs.h index b243739e4b..be38377b1f 100644 --- a/refs.h +++ b/refs.h @@ -586,7 +586,7 @@ int refs_update_symref(struct ref_store *refs, const char *refname, int refs_update_symref_extended(struct ref_store *refs, const char *refname, const char *target, const char *logmsg, - struct strbuf *referent); + struct strbuf *referent, int create_only); enum action_on_err { UPDATE_REFS_MSG_ON_ERR, -- cgit v1.3 From 5bcbde9e49c14f4e47a0ed5fc60470f3d4447efd Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 3 Dec 2024 11:32:37 +0900 Subject: refs: move ref name helpers around strbuf_branchname(), strbuf_check_{branch,tag}_ref() are helper functions to deal with branch and tag names, and the fact that they happen to use strbuf to hold the name of a branch or a tag is not essential. These functions fit better in the refs API than strbuf API, the latter of which is about string manipulations. Signed-off-by: Junio C Hamano --- builtin/tag.c | 11 ----------- object-name.c | 36 ------------------------------------ refs.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ refs.h | 29 +++++++++++++++++++++++++++++ strbuf.h | 22 ---------------------- 5 files changed, 76 insertions(+), 69 deletions(-) (limited to 'refs.h') diff --git a/builtin/tag.c b/builtin/tag.c index 93d10d5915..8279dccbe0 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -447,17 +447,6 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset) return 0; } -static int strbuf_check_tag_ref(struct strbuf *sb, const char *name) -{ - if (name[0] == '-') - return -1; - - strbuf_reset(sb); - strbuf_addf(sb, "refs/tags/%s", name); - - return check_refname_format(sb->buf, 0); -} - int cmd_tag(int argc, const char **argv, const char *prefix, diff --git a/object-name.c b/object-name.c index c892fbe80a..9f2ae164e4 100644 --- a/object-name.c +++ b/object-name.c @@ -1734,42 +1734,6 @@ int repo_interpret_branch_name(struct repository *r, return -1; } -void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed) -{ - int len = strlen(name); - struct interpret_branch_name_options options = { - .allowed = allowed - }; - int used = repo_interpret_branch_name(the_repository, name, len, sb, - &options); - - if (used < 0) - used = 0; - strbuf_add(sb, name + used, len - used); -} - -int strbuf_check_branch_ref(struct strbuf *sb, const char *name) -{ - if (startup_info->have_repository) - strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL); - else - strbuf_addstr(sb, name); - - /* - * This splice must be done even if we end up rejecting the - * name; builtin/branch.c::copy_or_rename_branch() still wants - * to see what the name expanded to so that "branch -m" can be - * used as a tool to correct earlier mistakes. - */ - strbuf_splice(sb, 0, 0, "refs/heads/", 11); - - if (*name == '-' || - !strcmp(sb->buf, "refs/heads/HEAD")) - return -1; - - return check_refname_format(sb->buf, 0); -} - void object_context_release(struct object_context *ctx) { free(ctx->path); diff --git a/refs.c b/refs.c index 5f729ed412..59a9223d4c 100644 --- a/refs.c +++ b/refs.c @@ -697,6 +697,53 @@ static char *substitute_branch_name(struct repository *r, return NULL; } +void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed) +{ + int len = strlen(name); + struct interpret_branch_name_options options = { + .allowed = allowed + }; + int used = repo_interpret_branch_name(the_repository, name, len, sb, + &options); + + if (used < 0) + used = 0; + strbuf_add(sb, name + used, len - used); +} + +int strbuf_check_branch_ref(struct strbuf *sb, const char *name) +{ + if (startup_info->have_repository) + strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL); + else + strbuf_addstr(sb, name); + + /* + * This splice must be done even if we end up rejecting the + * name; builtin/branch.c::copy_or_rename_branch() still wants + * to see what the name expanded to so that "branch -m" can be + * used as a tool to correct earlier mistakes. + */ + strbuf_splice(sb, 0, 0, "refs/heads/", 11); + + if (*name == '-' || + !strcmp(sb->buf, "refs/heads/HEAD")) + return -1; + + return check_refname_format(sb->buf, 0); +} + +int strbuf_check_tag_ref(struct strbuf *sb, const char *name) +{ + if (name[0] == '-') + return -1; + + strbuf_reset(sb); + strbuf_addf(sb, "refs/tags/%s", name); + + return check_refname_format(sb->buf, 0); +} + int repo_dwim_ref(struct repository *r, const char *str, int len, struct object_id *oid, char **ref, int nonfatal_dangling_mark) { diff --git a/refs.h b/refs.h index 108dfc93b3..f19b0ad92f 100644 --- a/refs.h +++ b/refs.h @@ -180,6 +180,35 @@ int repo_dwim_log(struct repository *r, const char *str, int len, struct object_ */ char *repo_default_branch_name(struct repository *r, int quiet); +/* + * Copy "name" to "sb", expanding any special @-marks as handled by + * repo_interpret_branch_name(). The result is a non-qualified branch name + * (so "foo" or "origin/master" instead of "refs/heads/foo" or + * "refs/remotes/origin/master"). + * + * Note that the resulting name may not be a syntactically valid refname. + * + * If "allowed" is non-zero, restrict the set of allowed expansions. See + * repo_interpret_branch_name() for details. + */ +void strbuf_branchname(struct strbuf *sb, const char *name, + unsigned allowed); + +/* + * Like strbuf_branchname() above, but confirm that the result is + * syntactically valid to be used as a local branch name in refs/heads/. + * + * The return value is "0" if the result is valid, and "-1" otherwise. + */ +int strbuf_check_branch_ref(struct strbuf *sb, const char *name); + +/* + * Similar for a tag name in refs/tags/. + * + * The return value is "0" if the result is valid, and "-1" otherwise. + */ +int strbuf_check_tag_ref(struct strbuf *sb, const char *name); + /* * A ref_transaction represents a collection of reference updates that * should succeed or fail together. diff --git a/strbuf.h b/strbuf.h index 003f880ff7..4dc05b4ba7 100644 --- a/strbuf.h +++ b/strbuf.h @@ -637,28 +637,6 @@ static inline void strbuf_complete_line(struct strbuf *sb) strbuf_complete(sb, '\n'); } -/* - * Copy "name" to "sb", expanding any special @-marks as handled by - * repo_interpret_branch_name(). The result is a non-qualified branch name - * (so "foo" or "origin/master" instead of "refs/heads/foo" or - * "refs/remotes/origin/master"). - * - * Note that the resulting name may not be a syntactically valid refname. - * - * If "allowed" is non-zero, restrict the set of allowed expansions. See - * repo_interpret_branch_name() for details. - */ -void strbuf_branchname(struct strbuf *sb, const char *name, - unsigned allowed); - -/* - * Like strbuf_branchname() above, but confirm that the result is - * syntactically valid to be used as a local branch name in refs/heads/. - * - * The return value is "0" if the result is valid, and "-1" otherwise. - */ -int strbuf_check_branch_ref(struct strbuf *sb, const char *name); - typedef int (*char_predicate)(char ch); void strbuf_addstr_urlencode(struct strbuf *sb, const char *name, -- cgit v1.3 From 93e5e048f84138a632b239632c9b45ae238cdf1c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 3 Dec 2024 11:32:38 +0900 Subject: refs: drop strbuf_ prefix from helpers The helper functions (strbuf_branchname, strbuf_check_branch_ref, and strbuf_check_tag_ref) are about handling branch and tag names, and it is a non-essential fact that these functions use strbuf to hold these names. Rename them to make it clarify that these are more about "ref". Signed-off-by: Junio C Hamano --- branch.c | 2 +- builtin/branch.c | 10 +++++----- builtin/check-ref-format.c | 2 +- builtin/checkout.c | 2 +- builtin/merge.c | 2 +- builtin/tag.c | 2 +- builtin/worktree.c | 8 ++++---- gitweb/gitweb.perl | 2 +- refs.c | 8 ++++---- refs.h | 8 ++++---- 10 files changed, 23 insertions(+), 23 deletions(-) (limited to 'refs.h') diff --git a/branch.c b/branch.c index 08fa4094d2..58b61831af 100644 --- a/branch.c +++ b/branch.c @@ -372,7 +372,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name) */ int validate_branchname(const char *name, struct strbuf *ref) { - if (strbuf_check_branch_ref(ref, name)) { + if (check_branch_ref(ref, name)) { int code = die_message(_("'%s' is not a valid branch name"), name); advise_if_enabled(ADVICE_REF_SYNTAX, _("See `man git check-ref-format`")); diff --git a/builtin/branch.c b/builtin/branch.c index fd1611ebf5..17acf598d2 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -257,7 +257,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, char *target = NULL; int flags = 0; - strbuf_branchname(&bname, argv[i], allowed_interpret); + copy_branchname(&bname, argv[i], allowed_interpret); free(name); name = mkpathdup(fmt, bname.buf); @@ -579,7 +579,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int int recovery = 0, oldref_usage = 0; struct worktree **worktrees = get_worktrees(); - if (strbuf_check_branch_ref(&oldref, oldname)) { + if (check_branch_ref(&oldref, oldname)) { /* * Bad name --- this could be an attempt to rename a * ref that we used to allow to be created by accident. @@ -894,7 +894,7 @@ int cmd_branch(int argc, die(_("cannot give description to detached HEAD")); branch_name = head; } else if (argc == 1) { - strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL); + copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL); branch_name = buf.buf; } else { die(_("cannot edit description of more than one branch")); @@ -933,7 +933,7 @@ int cmd_branch(int argc, if (!argc) branch = branch_get(NULL); else if (argc == 1) { - strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL); + copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL); branch = branch_get(buf.buf); } else die(_("too many arguments to set new upstream")); @@ -963,7 +963,7 @@ int cmd_branch(int argc, if (!argc) branch = branch_get(NULL); else if (argc == 1) { - strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL); + copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL); branch = branch_get(buf.buf); } else die(_("too many arguments to unset upstream")); diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c index e86d8ef980..cef1ffe3ce 100644 --- a/builtin/check-ref-format.c +++ b/builtin/check-ref-format.c @@ -42,7 +42,7 @@ static int check_ref_format_branch(const char *arg) int nongit; setup_git_directory_gently(&nongit); - if (strbuf_check_branch_ref(&sb, arg) || + if (check_branch_ref(&sb, arg) || !skip_prefix(sb.buf, "refs/heads/", &name)) die("'%s' is not a valid branch name", arg); printf("%s\n", name); diff --git a/builtin/checkout.c b/builtin/checkout.c index c449558e66..5e5afa0f26 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -742,7 +742,7 @@ static void setup_branch_path(struct branch_info *branch) &branch->oid, &branch->refname, 0)) repo_get_oid_committish(the_repository, branch->name, &branch->oid); - strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL); + copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL); if (strcmp(buf.buf, branch->name)) { free(branch->name); branch->name = xstrdup(buf.buf); diff --git a/builtin/merge.c b/builtin/merge.c index 84d0f3604b..d0c31d7714 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -498,7 +498,7 @@ static void merge_name(const char *remote, struct strbuf *msg) char *found_ref = NULL; int len, early; - strbuf_branchname(&bname, remote, 0); + copy_branchname(&bname, remote, 0); remote = bname.buf; oidclr(&branch_head, the_repository->hash_algo); diff --git a/builtin/tag.c b/builtin/tag.c index 8279dccbe0..670e564178 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -639,7 +639,7 @@ int cmd_tag(int argc, if (repo_get_oid(the_repository, object_ref, &object)) die(_("Failed to resolve '%s' as a valid ref."), object_ref); - if (strbuf_check_tag_ref(&ref, tag)) + if (check_tag_ref(&ref, tag)) die(_("'%s' is not a valid tag name."), tag); if (refs_read_ref(get_main_ref_store(the_repository), ref.buf, &prev)) diff --git a/builtin/worktree.c b/builtin/worktree.c index fc31d072a6..c68f601358 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -432,7 +432,7 @@ static int add_worktree(const char *path, const char *refname, worktrees = NULL; /* is 'refname' a branch or commit? */ - if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) && + if (!opts->detach && !check_branch_ref(&symref, refname) && refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) { is_branch = 1; if (!opts->force) @@ -604,7 +604,7 @@ static void print_preparing_worktree_line(int detach, fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch); } else { struct strbuf s = STRBUF_INIT; - if (!detach && !strbuf_check_branch_ref(&s, branch) && + if (!detach && !check_branch_ref(&s, branch) && refs_ref_exists(get_main_ref_store(the_repository), s.buf)) fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"), branch); @@ -745,7 +745,7 @@ static char *dwim_branch(const char *path, char **new_branch) char *branchname = xstrndup(s, n); struct strbuf ref = STRBUF_INIT; - branch_exists = !strbuf_check_branch_ref(&ref, branchname) && + branch_exists = !check_branch_ref(&ref, branchname) && refs_ref_exists(get_main_ref_store(the_repository), ref.buf); strbuf_release(&ref); @@ -838,7 +838,7 @@ static int add(int ac, const char **av, const char *prefix) new_branch = new_branch_force; if (!opts.force && - !strbuf_check_branch_ref(&symref, new_branch) && + !check_branch_ref(&symref, new_branch) && refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) die_if_checked_out(symref.buf, 0); strbuf_release(&symref); diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index b09a8d0523..8cdb0d9b9f 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2094,7 +2094,7 @@ sub format_log_line_html { ( # The output of "git describe", e.g. v2.10.0-297-gf6727b0 # or hadoop-20160921-113441-20-g094fb7d - (?have_repository) - strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL); + copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL); else strbuf_addstr(sb, name); @@ -733,7 +733,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name) return check_refname_format(sb->buf, 0); } -int strbuf_check_tag_ref(struct strbuf *sb, const char *name) +int check_tag_ref(struct strbuf *sb, const char *name) { if (name[0] == '-') return -1; diff --git a/refs.h b/refs.h index f19b0ad92f..c5280477f0 100644 --- a/refs.h +++ b/refs.h @@ -191,23 +191,23 @@ char *repo_default_branch_name(struct repository *r, int quiet); * If "allowed" is non-zero, restrict the set of allowed expansions. See * repo_interpret_branch_name() for details. */ -void strbuf_branchname(struct strbuf *sb, const char *name, +void copy_branchname(struct strbuf *sb, const char *name, unsigned allowed); /* - * Like strbuf_branchname() above, but confirm that the result is + * Like copy_branchname() above, but confirm that the result is * syntactically valid to be used as a local branch name in refs/heads/. * * The return value is "0" if the result is valid, and "-1" otherwise. */ -int strbuf_check_branch_ref(struct strbuf *sb, const char *name); +int check_branch_ref(struct strbuf *sb, const char *name); /* * Similar for a tag name in refs/tags/. * * The return value is "0" if the result is valid, and "-1" otherwise. */ -int strbuf_check_tag_ref(struct strbuf *sb, const char *name); +int check_tag_ref(struct strbuf *sb, const char *name); /* * A ref_transaction represents a collection of reference updates that -- cgit v1.3 From 84675fa2717e08b39bf810eb9a439068ac915dfb Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 16 Dec 2024 17:44:31 +0100 Subject: refs: introduce the `ref_transaction_update_reflog` function Introduce a new function `ref_transaction_update_reflog`, for clients to add a reflog update to a transaction. While the existing function `ref_transaction_update` also allows clients to add a reflog entry, this function does a few things more, It: - Enforces that only a reflog entry is added and does not update the ref itself. - Allows the users to also provide the committer information. This means clients can add reflog entries with custom committer information. The `transaction_refname_valid()` function also modifies the error message selectively based on the type of the update. This change also affects reflog updates which go through `ref_transaction_update()`. A follow up commit will utilize this function to add reflog support to `git refs migrate`. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs.c | 39 +++++++++++++++++++++++++++++++++++---- refs.h | 14 ++++++++++++++ refs/files-backend.c | 24 ++++++++++++++++-------- 3 files changed, 65 insertions(+), 12 deletions(-) (limited to 'refs.h') diff --git a/refs.c b/refs.c index 782bf1090a..8b3882cff1 100644 --- a/refs.c +++ b/refs.c @@ -1207,14 +1207,14 @@ static int transaction_refname_valid(const char *refname, return 1; if (is_pseudo_ref(refname)) { - strbuf_addf(err, _("refusing to update pseudoref '%s'"), - refname); + const char *what = flags & REF_LOG_ONLY ? "reflog for pseudoref" : "pseudoref"; + strbuf_addf(err, _("refusing to update %s '%s'"), what, refname); return 0; } else if ((new_oid && !is_null_oid(new_oid)) ? check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) : !refname_is_safe(refname)) { - strbuf_addf(err, _("refusing to update ref with bad name '%s'"), - refname); + const char *what = flags & REF_LOG_ONLY ? "reflog with bad name" : "ref with bad name"; + strbuf_addf(err, _("refusing to update %s '%s'"), what, refname); return 0; } @@ -1257,6 +1257,37 @@ int ref_transaction_update(struct ref_transaction *transaction, ref_transaction_add_update(transaction, refname, flags, new_oid, old_oid, new_target, old_target, NULL, msg); + + return 0; +} + +int ref_transaction_update_reflog(struct ref_transaction *transaction, + const char *refname, + const struct object_id *new_oid, + const struct object_id *old_oid, + const char *committer_info, unsigned int flags, + const char *msg, unsigned int index, + struct strbuf *err) +{ + struct ref_update *update; + + assert(err); + + flags |= REF_LOG_ONLY | REF_NO_DEREF; + + if (!transaction_refname_valid(refname, new_oid, flags, err)) + return -1; + + update = ref_transaction_add_update(transaction, refname, flags, + new_oid, old_oid, NULL, NULL, + committer_info, msg); + /* + * While we do set the old_oid value, we unset the flag to skip + * old_oid verification which only makes sense for refs. + */ + update->flags &= ~REF_HAVE_OLD; + update->index = index; + return 0; } diff --git a/refs.h b/refs.h index a5bedf48cf..b0dfc65ed2 100644 --- a/refs.h +++ b/refs.h @@ -727,6 +727,20 @@ int ref_transaction_update(struct ref_transaction *transaction, unsigned int flags, const char *msg, struct strbuf *err); +/* + * Similar to`ref_transaction_update`, but this function is only for adding + * a reflog update. Supports providing custom committer information. The index + * field can be utiltized to order updates as desired. When not used, the + * updates default to being ordered by refname. + */ +int ref_transaction_update_reflog(struct ref_transaction *transaction, + const char *refname, + const struct object_id *new_oid, + const struct object_id *old_oid, + const char *committer_info, unsigned int flags, + const char *msg, unsigned int index, + struct strbuf *err); + /* * Add a reference creation to transaction. new_oid is the value that * the reference should have after the update; it must not be diff --git a/refs/files-backend.c b/refs/files-backend.c index 255fed8354..c11213f520 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3080,10 +3080,12 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, } /* - * packed-refs don't support symbolic refs and root refs, so we - * have to queue these references via the loose transaction. + * packed-refs don't support symbolic refs, root refs and reflogs, + * so we have to queue these references via the loose transaction. */ - if (update->new_target || is_root_ref(update->refname)) { + if (update->new_target || + is_root_ref(update->refname) || + (update->flags & REF_LOG_ONLY)) { if (!loose_transaction) { loose_transaction = ref_store_transaction_begin(&refs->base, 0, err); if (!loose_transaction) { @@ -3092,11 +3094,17 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, } } - ref_transaction_add_update(loose_transaction, update->refname, - update->flags & ~REF_HAVE_OLD, - update->new_target ? NULL : &update->new_oid, NULL, - update->new_target, NULL, update->committer_info, - NULL); + if (update->flags & REF_LOG_ONLY) + ref_transaction_add_update(loose_transaction, update->refname, + update->flags, &update->new_oid, + &update->old_oid, NULL, NULL, + update->committer_info, update->msg); + else + ref_transaction_add_update(loose_transaction, update->refname, + update->flags & ~REF_HAVE_OLD, + update->new_target ? NULL : &update->new_oid, NULL, + update->new_target, NULL, update->committer_info, + NULL); } else { ref_transaction_add_update(packed_transaction, update->refname, update->flags & ~REF_HAVE_OLD, -- cgit v1.3