Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> From: Karthik Nayak <karthik.188@xxxxxxxxx>
>
> The 'update-ref' command is used to update refs using transactions. The
> command allows users to also utilize a '--stdin' mode to provide a
> batch of sub-commands which can be processed in a transaction.
>
> Currently, the sub-commands involve {verify, delete, create, update}
> and they allow users to work with regular refs in the repository. To
> work with symrefs, users only have the option of using
> 'git-symbolic-ref', which doesn't provide transaction support to the
> users eventhough it uses the same behind the hood.
>
> Recently, we modified the reference backend to add symref support,
> following which, 'git-symbolic-ref' also uses the transaction backend.
> But, it doesn't expose this to the user. To allow users to work with
> symrefs via transaction, this series adds support for new sub-commands
> {symrer-verify, symref-delete, symref-create, symref-update} to the
> '--stdin' mode of update-ref. These complement the existing
> sub-commands.
>
> The patches 1, 2, & 6 fix small issues in the reference backends. The other
> patches 3, 4, 5, & 7 each add one of the new sub-commands.
>
> The series is based off master, with 'kn/ref-transaction-symref' merged
> in.
>
> There was some discussion [1] also about adding `old_target` support to
> the existing `update` command. I think its worthwhile to do this with
> some tests cleanup, will follow that up as a separate series.
>
> Changes since v3:
> * Changed the position of `old_target` and `flags` in `ref_transaction_delete`
> to make it a coherent.
> * Added tests for deletion of regular refs using 'symref-delete', this lead to
> adding a new commit to have specific errors for when a regular update contains
> `old_target`.
>
> [1]: https://lore.kernel.org/r/CAOLa=ZQW-cCV5BP_fCvuZimfkjwAzjEiqXYRPft1Wf9kAX=_bw@xxxxxxxxxxxxxx
>
> Karthik Nayak (7):
> refs: create and use `ref_update_expects_existing_old_ref()`
> refs: specify error for regular refs with `old_target`
> update-ref: add support for 'symref-verify' command
> update-ref: add support for 'symref-delete' command
> update-ref: add support for 'symref-create' command
> reftable: pick either 'oid' or 'target' for new updates
> update-ref: add support for 'symref-update' command
>
> Documentation/git-update-ref.txt | 25 ++
> builtin/clone.c | 2 +-
> builtin/fetch.c | 4 +-
> builtin/receive-pack.c | 3 +-
> builtin/update-ref.c | 237 ++++++++++++++++-
> refs.c | 40 ++-
> refs.h | 6 +-
> refs/files-backend.c | 16 +-
> refs/refs-internal.h | 6 +
> refs/reftable-backend.c | 16 +-
> t/t0600-reffiles-backend.sh | 32 +++
> t/t1400-update-ref.sh | 430 ++++++++++++++++++++++++++++++-
> t/t1416-ref-transaction-hooks.sh | 54 ++++
> t/t5605-clone-local.sh | 2 +-
> 14 files changed, 832 insertions(+), 41 deletions(-)
>
> Range-diff against v3:
> -: ---------- > 1: cab5265c3c refs: create and use `ref_update_expects_existing_old_ref()`
> -: ---------- > 2: 57b5ff46c0 refs: specify error for regular refs with `old_target`
> 1: ed54b0dfb9 = 3: 5710fa81bf update-ref: add support for 'symref-verify' command
> 2: b82b86ff40 ! 4: 5f8fc4eb6e update-ref: add support for 'symref-delete' command
> @@ Commit message
> within a transaction, which promises atomicity of the operation and can
> be batched with other commands.
>
> + When no 'old_target' is provided it can also delete regular refs,
> + similar to how the 'delete' command can delete symrefs when no 'old_oid'
> + is provided.
> +
> Helped-by: Patrick Steinhardt <ps@xxxxxx>
> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
>
> @@ Documentation/git-update-ref.txt: verify::
>
> ## builtin/fetch.c ##
> @@ builtin/fetch.c: static int prune_refs(struct display_state *display_state,
> + if (!dry_run) {
> if (transaction) {
> for (ref = stale_refs; ref; ref = ref->next) {
> - result = ref_transaction_delete(transaction, ref->name, NULL, 0,
> +- result = ref_transaction_delete(transaction, ref->name, NULL, 0,
> - "fetch: prune", &err);
> -+ NULL, "fetch: prune", &err);
> ++ result = ref_transaction_delete(transaction, ref->name, NULL,
> ++ NULL, 0, "fetch: prune", &err);
> if (result)
> goto cleanup;
> }
> @@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh
> namespaced_name,
> old_oid,
> - 0, "push", &err)) {
> -+ 0, NULL,
> ++ NULL, 0,
> + "push", &err)) {
> rp_error("%s", err.buf);
> ret = "failed to delete";
> @@ builtin/update-ref.c: static void parse_cmd_delete(struct ref_transaction *trans
> if (ref_transaction_delete(transaction, refname,
> have_old ? &old_oid : NULL,
> - update_flags, msg, &err))
> -+ update_flags, NULL, msg, &err))
> ++ NULL, update_flags, msg, &err))
> die("%s", err.buf);
>
> update_flags = default_flags;
> @@ builtin/update-ref.c: static void parse_cmd_delete(struct ref_transaction *trans
> + die("symref-delete %s: extra input: %s", refname, next);
> +
> + if (ref_transaction_delete(transaction, refname, NULL,
> -+ update_flags, old_target, msg, &err))
> ++ old_target, update_flags, msg, &err))
> + die("%s", err.buf);
> +
> + update_flags = default_flags;
> @@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,
> if (!transaction ||
> ref_transaction_delete(transaction, refname, old_oid,
> - flags, msg, &err) ||
> -+ flags, NULL, msg, &err) ||
> ++ NULL, flags, msg, &err) ||
> ref_transaction_commit(transaction, &err)) {
> error("%s", err.buf);
> ref_transaction_free(transaction);
> @@ refs.c: int ref_transaction_create(struct ref_transaction *transaction,
> const char *refname,
> const struct object_id *old_oid,
> - unsigned int flags, const char *msg,
> -+ unsigned int flags,
> + const char *old_target,
> ++ unsigned int flags,
> + const char *msg,
> struct strbuf *err)
> {
> @@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,
> for_each_string_list_item(item, refnames) {
> ret = ref_transaction_delete(transaction, item->string,
> - NULL, flags, msg, &err);
> -+ NULL, flags, NULL, msg, &err);
> ++ NULL, NULL, flags, msg, &err);
> if (ret) {
> warning(_("could not delete reference %s: %s"),
> item->string, err.buf);
> @@ refs.h: int ref_transaction_create(struct ref_transaction *transaction,
> const char *refname,
> const struct object_id *old_oid,
> - unsigned int flags, const char *msg,
> -+ unsigned int flags,
> + const char *old_target,
> ++ unsigned int flags,
> + const char *msg,
> struct strbuf *err);
>
> @@ t/t1400-update-ref.sh: do
> + grep "fatal: symref-delete: missing <ref>" err
> + '
> +
> ++ test_expect_success "stdin $type symref-delete fails deleting regular ref" '
> ++ test_when_finished "git update-ref -d refs/heads/regularref" &&
> ++ git update-ref refs/heads/regularref $a &&
> ++ format_command $type "symref-delete refs/heads/regularref" "$a" >stdin &&
> ++ test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
> ++ grep "fatal: cannot update regular ref: ${SQ}refs/heads/regularref${SQ}: symref target ${SQ}$a${SQ} set" err
> ++ '
> ++
> + test_expect_success "stdin $type symref-delete fails with too many arguments" '
> + format_command $type "symref-delete refs/heads/symref" "$a" "$a" >stdin &&
> + test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
> @@ t/t1400-update-ref.sh: do
> + git update-ref --stdin $type --no-deref <stdin &&
> + test_must_fail git symbolic-ref -d refs/heads/symref2
> + '
> ++
> ++ test_expect_success "stdin $type symref-delete fails deleting regular ref without target" '
> ++ git update-ref refs/heads/regularref $a &&
> ++ format_command $type "symref-delete refs/heads/regularref" >stdin &&
> ++ git update-ref --stdin $type --no-deref <stdin
> ++ '
> +
> done
>
> 3: ae127f7d52 ! 5: 1542cfb806 update-ref: add support for 'symref-create' command
> @@ t/t0600-reffiles-backend.sh: test_expect_success POSIXPERM 'git reflog expire ho
>
> ## t/t1400-update-ref.sh ##
> @@ t/t1400-update-ref.sh: do
> - test_must_fail git symbolic-ref -d refs/heads/symref2
> + git update-ref --stdin $type --no-deref <stdin
> '
>
> + test_expect_success "stdin $type symref-create fails with too many arguments" '
> 4: 8889dcbf40 = 6: ec5380743d reftable: pick either 'oid' or 'target' for new updates
> 5: 19d85d56c4 ! 7: f8d91f7fc9 update-ref: add support for 'symref-update' command
> @@ Commit message
> we don't use a `(ref | oid)` prefix for the old_value, then there is
> ambiguity around if the value provided should be treated as an oid or a
> reference. This is more so the reason, because we allow anything
> - committish to be provided as an oid.
> + committish to be provided as an oid. While 'symref-verify' and
> + 'symref-delete' also take in `<old-target>` we do not have this
> + divergence there as those commands only work with symrefs. Whereas
> + 'symref-update' also works with regular refs and allows users to convert
> + regular refs to symrefs.
>
> The command allows users to perform symbolic ref updates within a
> transaction. This provides atomicity and allows users to perform a set
> --
> 2.43.GIT
I also noticed that the range-diff seems a bit weird. I used
git format-patch -v 4 --cover-letter --range-diff=symref-cmd-v3 @~7..@
to generate the patch series, but seems like when doing this it
internally computes the merge-base, which happens to be the first commit
in this series, skewing the range-diff by one commit. I guess its always
better to provide the commit range to `--range-diff`. Adding the range
diff for the same, when run using
git format-patch -v 4 --cover-letter
--range-diff=6ab505f161..symref-cmd-v3 @~7..@
where `6ab505f161` is the common base for both v3 and v4, and is from
merging in `ps/leakfixes`.
Range-diff against v3:
1: cab5265c3c = 1: cab5265c3c refs: create and use
`ref_update_expects_existing_old_ref()`
-: ---------- > 2: 57b5ff46c0 refs: specify error for regular refs
with `old_target`
2: ed54b0dfb9 = 3: 5710fa81bf update-ref: add support for
'symref-verify' command
3: b82b86ff40 ! 4: 5f8fc4eb6e update-ref: add support for
'symref-delete' command
@@ Commit message
within a transaction, which promises atomicity of the operation and can
be batched with other commands.
+ When no 'old_target' is provided it can also delete regular refs,
+ similar to how the 'delete' command can delete symrefs when
no 'old_oid'
+ is provided.
+
Helped-by: Patrick Steinhardt <ps@xxxxxx>
Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
@@ Documentation/git-update-ref.txt: verify::
## builtin/fetch.c ##
@@ builtin/fetch.c: static int prune_refs(struct display_state
*display_state,
+ if (!dry_run) {
if (transaction) {
for (ref = stale_refs; ref; ref = ref->next) {
- result = ref_transaction_delete(transaction, ref->name, NULL, 0,
+- result = ref_transaction_delete(transaction, ref->name, NULL, 0,
- "fetch: prune", &err);
-+ NULL, "fetch: prune", &err);
++ result = ref_transaction_delete(transaction, ref->name, NULL,
++ NULL, 0, "fetch: prune", &err);
if (result)
goto cleanup;
}
@@ builtin/receive-pack.c: static const char *update(struct
command *cmd, struct sh
namespaced_name,
old_oid,
- 0, "push", &err)) {
-+ 0, NULL,
++ NULL, 0,
+ "push", &err)) {
rp_error("%s", err.buf);
ret = "failed to delete";
@@ builtin/update-ref.c: static void parse_cmd_delete(struct
ref_transaction *trans
if (ref_transaction_delete(transaction, refname,
have_old ? &old_oid : NULL,
- update_flags, msg, &err))
-+ update_flags, NULL, msg, &err))
++ NULL, update_flags, msg, &err))
die("%s", err.buf);
update_flags = default_flags;
@@ builtin/update-ref.c: static void parse_cmd_delete(struct
ref_transaction *trans
+ die("symref-delete %s: extra input: %s", refname, next);
+
+ if (ref_transaction_delete(transaction, refname, NULL,
-+ update_flags, old_target, msg, &err))
++ old_target, update_flags, msg, &err))
+ die("%s", err.buf);
+
+ update_flags = default_flags;
@@ refs.c: int refs_delete_ref(struct ref_store *refs, const char *msg,
if (!transaction ||
ref_transaction_delete(transaction, refname, old_oid,
- flags, msg, &err) ||
-+ flags, NULL, msg, &err) ||
++ NULL, flags, msg, &err) ||
ref_transaction_commit(transaction, &err)) {
error("%s", err.buf);
ref_transaction_free(transaction);
@@ refs.c: int ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
const struct object_id *old_oid,
- unsigned int flags, const char *msg,
-+ unsigned int flags,
+ const char *old_target,
++ unsigned int flags,
+ const char *msg,
struct strbuf *err)
{
@@ refs.c: int refs_delete_refs(struct ref_store *refs, const char *logmsg,
for_each_string_list_item(item, refnames) {
ret = ref_transaction_delete(transaction, item->string,
- NULL, flags, msg, &err);
-+ NULL, flags, NULL, msg, &err);
++ NULL, NULL, flags, msg, &err);
if (ret) {
warning(_("could not delete reference %s: %s"),
item->string, err.buf);
@@ refs.h: int ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
const struct object_id *old_oid,
- unsigned int flags, const char *msg,
-+ unsigned int flags,
+ const char *old_target,
++ unsigned int flags,
+ const char *msg,
struct strbuf *err);
@@ t/t1400-update-ref.sh: do
+ grep "fatal: symref-delete: missing <ref>" err
+ '
+
++ test_expect_success "stdin $type symref-delete fails deleting
regular ref" '
++ test_when_finished "git update-ref -d refs/heads/regularref" &&
++ git update-ref refs/heads/regularref $a &&
++ format_command $type "symref-delete refs/heads/regularref"
"$a" >stdin &&
++ test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
++ grep "fatal: cannot update regular ref:
${SQ}refs/heads/regularref${SQ}: symref target ${SQ}$a${SQ} set" err
++ '
++
+ test_expect_success "stdin $type symref-delete fails with too
many arguments" '
+ format_command $type "symref-delete refs/heads/symref" "$a"
"$a" >stdin &&
+ test_must_fail git update-ref --stdin $type --no-deref <stdin 2>err &&
@@ t/t1400-update-ref.sh: do
+ git update-ref --stdin $type --no-deref <stdin &&
+ test_must_fail git symbolic-ref -d refs/heads/symref2
+ '
++
++ test_expect_success "stdin $type symref-delete fails deleting
regular ref without target" '
++ git update-ref refs/heads/regularref $a &&
++ format_command $type "symref-delete refs/heads/regularref" >stdin &&
++ git update-ref --stdin $type --no-deref <stdin
++ '
+
done
4: ae127f7d52 ! 5: 1542cfb806 update-ref: add support for
'symref-create' command
@@ t/t0600-reffiles-backend.sh: test_expect_success POSIXPERM 'git
reflog expire ho
## t/t1400-update-ref.sh ##
@@ t/t1400-update-ref.sh: do
- test_must_fail git symbolic-ref -d refs/heads/symref2
+ git update-ref --stdin $type --no-deref <stdin
'
+ test_expect_success "stdin $type symref-create fails with too
many arguments" '
5: 8889dcbf40 = 6: ec5380743d reftable: pick either 'oid' or
'target' for new updates
6: 19d85d56c4 ! 7: f8d91f7fc9 update-ref: add support for
'symref-update' command
@@ Commit message
we don't use a `(ref | oid)` prefix for the old_value, then there is
ambiguity around if the value provided should be treated as an oid or a
reference. This is more so the reason, because we allow anything
- committish to be provided as an oid.
+ committish to be provided as an oid. While 'symref-verify' and
+ 'symref-delete' also take in `<old-target>` we do not have this
+ divergence there as those commands only work with symrefs. Whereas
+ 'symref-update' also works with regular refs and allows users
to convert
+ regular refs to symrefs.
The command allows users to perform symbolic ref updates within a
transaction. This provides atomicity and allows users to perform a set
--
2.43.GIT
Attachment:
signature.asc
Description: PGP signature
