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

From: Karthik Nayak <[email-masked]>
Date: Wed, 5 Jun 2024 12:29:51 +0200

From: Karthik Nayak <[email-masked]>

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=[email-masked]

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 <[email-masked]>
         Signed-off-by: Karthik Nayak <[email-masked]>
     
    @@ 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