Re: [PATCH v4 0/7] update-ref: add symref support for --stdin

> From: Karthik Nayak <karthik.188@gmail.com>
>
> 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@mail.gmail.com
>
> 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@pks.im>
>          Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>
>     @@ 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