mbox series

[v5,0/7] update-ref: add symref support for --stdin

Message ID 20240607133304.2333280-1-knayak@gitlab.com (mailing list archive)
Headers show
Series update-ref: add symref support for --stdin | expand

Message

karthik nayak June 7, 2024, 1:32 p.m. UTC
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 v4:
* Modified the commit message for the second patch and also the error to be
less technical and more user friendly. 
* Renamed an incorrectly named test.

Version 1-3 can be found here:
https://lore.kernel.org/all/20240530120940.456817-1-knayak@gitlab.com/

[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             |  17 +-
 refs/refs-internal.h             |   6 +
 refs/reftable-backend.c          |  17 +-
 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, 834 insertions(+), 41 deletions(-)

Range-diff against v4:
1:  cab5265c3c = 1:  cab5265c3c refs: create and use `ref_update_expects_existing_old_ref()`
2:  57b5ff46c0 ! 2:  94077e69cc refs: specify error for regular refs with `old_target`
    @@ Metadata
      ## Commit message ##
         refs: specify error for regular refs with `old_target`
     
    -    When a regular reference update contains `old_target` set, we call the
    -    `ref_update_check_old_target` function to check the referent value. But
    -    for regular refs we know that the referent value is not set and this
    -    simply raises a generic error which says nothing about this being a
    -    regular ref. Instead let's raise a more specific error when a regular
    -    ref update contains `old_target`.
    +    When a reference update tries to update a symref, but the ref in
    +    question is actually a regular ref, we raise an error. However the error
    +    raised in this situation is:
    +
    +      verifying symref target: '<ref>': reference is missing but expected <old-target>
    +
    +    which is very generic and doesn't indicate the mismatch of types. Let's
    +    make this error more specific:
    +
    +      cannot lock ref '<ref>': expected symref with target '<old-target>': but is a regular ref
    +
    +    so that users have a clearer understanding.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    @@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *ref
     -				ret = TRANSACTION_GENERIC_ERROR;
     -				goto out;
     -			}
    -+			strbuf_addf(err, _("cannot update regular ref: '%s': "
    -+					   "symref target '%s' set"),
    ++			strbuf_addf(err, _("cannot lock ref '%s': "
    ++					   "expected symref with target '%s': "
    ++					   "but is a regular ref"),
     +				    ref_update_original_update_refname(update),
     +				    update->old_target);
     +			ret = TRANSACTION_GENERIC_ERROR;
    @@ refs/reftable-backend.c: static int reftable_be_transaction_prepare(struct ref_s
      		 */
      		if (u->old_target) {
     +			if (!(u->type & REF_ISSYMREF)) {
    -+				strbuf_addf(err, _("cannot update regular ref: '%s': "
    -+					    "symref target '%s' set"),
    ++				strbuf_addf(err, _("cannot lock ref '%s': "
    ++					   "expected symref with target '%s': "
    ++					   "but is a regular ref"),
     +					    ref_update_original_update_refname(u),
     +					    u->old_target);
     +				ret = -1;
3:  5710fa81bf = 3:  153d8cce75 update-ref: add support for 'symref-verify' command
4:  5f8fc4eb6e ! 4:  07382bfa09 update-ref: add support for 'symref-delete' command
    @@ t/t1400-update-ref.sh: do
     +		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
    ++		grep "fatal: cannot lock ref ${SQ}refs/heads/regularref${SQ}: expected symref with target ${SQ}$a${SQ}: but is a regular ref" err
     +	'
     +
     +	test_expect_success "stdin $type symref-delete fails with too many arguments" '
    @@ t/t1400-update-ref.sh: do
     +		test_must_fail git symbolic-ref -d refs/heads/symref2
     +	'
     +
    -+	test_expect_success "stdin $type symref-delete fails deleting regular ref without target" '
    ++	test_expect_success "stdin $type symref-delete deletes 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
5:  1542cfb806 = 5:  d5f92f610d update-ref: add support for 'symref-create' command
6:  ec5380743d = 6:  117b9e49cd reftable: pick either 'oid' or 'target' for new updates
7:  f8d91f7fc9 = 7:  cc140fedbd update-ref: add support for 'symref-update' command