git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] refs: add symref support to 'git-update-ref'
@ 2024-04-23 21:28 Karthik Nayak
  2024-04-26 15:24 ` [PATCH v4 0/7] add symref-* commands to 'git-update-ref --stdin' Karthik Nayak
  0 siblings, 1 reply; 23+ messages in thread
From: Karthik Nayak @ 2024-04-23 21:28 UTC (permalink / raw)
  To: karthik.188; +Cc: chris.torek, git, gitster, ps

From: Karthik Nayak <karthik.188@gmail.com>

The 'git-update-ref(1)' command allows transactional reference updates.
But currently only supports regular reference updates. Meaning, if one
wants to update HEAD (symbolic ref) in a transaction, there is no tool
to do so.

One option to obtain transactional updates for the HEAD ref is to
manually create the HEAD.lock file and commit. This is intrusive, where
the user needs to mimic internal git behavior. Also, this only works
when using the files backend.

At GitLab, we've been using the manual process till date, to allow users
to set and change their default branch. But with the introduction of
reftables as a reference backend, this becomes a necessity to be solved
within git.

The patch series adds symref support to the existing commands {verify,
create, delete, update} within 'git-update-ref'. This is done by parsing
inputs with the 'ref:' prefix as symref targets. This updates our current
commands to:

update SP <ref> SP (<new-oid> | ref:<new-target>) [SP (<old-oid> | ref:<old-target>)] LF
create SP <ref> SP (<new-oid> | ref:<new-target>) LF
delete SP <ref> [SP (<old-oid> | ref:<old-target>)] LF
verify SP <ref> [SP (<old-oid> | ref:<old-target>)] LF

Wherein, when ref:<new-target> is provided, the update ensures that
the <ref> is a symbolic ref which targets <new-target>. When
ref:<old-target> is provided, we ensure that <ref> is a symbolic ref
which targets <old-target> before the update.

With this it is possible to:
1. Create, verify, delete, and update symrefs
2. Create dangling symrefs
3. Update regular refs to become symrefs
4. Update symrefs to become regular refs

V1 of the patch series can be found here:
https://lore.kernel.org/git/20240330224623.579457-1-knayak@gitlab.com/
V2 of the patch series can be found here:
https://lore.kernel.org/git/20240412095908.1134387-1-knayak@gitlab.com/

Changes from v2:
1. We no longer have separate commands for symrefs, instead the regular
commands learn to parse 'ref:<target>' as symref targets. This reduces
the code in this series. Thanks Patrick for the suggestion.
2. Apart from supporting regular refs => symrefs. We also support the
inverse now.
3. Also allow creation of dangling refs.
4. Bunch of cleanups
   - whitespace issues in the previous patch series
   - uneeded header includes
   - uneeded tests
5. Added more documentation and better error messages, especially in reftables.
6. Better assertions around the input data.

Thanks all for the reviews on the previous iteration. Appreciate the support!

Range diff against v2:

1:  3269d0e91e ! 1:  4e49d54dcc refs: accept symref values in `ref_transaction[_add]_update`
    @@ Commit message
         flags to create a `ref_update` and add it to the transaction at hand.
     
         To extend symref support in transactions, we need to also accept the
    -    old and new ref values and process it. In this commit, let's add the
    -    required paramaters to the function and modify all call sites.
    +    old and new ref targets and process it. In this commit, let's add the
    +    required parameters to the function and modify all call sites.
     
    -    The two paramaters added are `new_ref` and `old_ref`. The `new_ref` is
    -    used to denote what the reference should point to when the transaction
    -    is applied. Some functions allow this parameter to be NULL, meaning that
    -    the reference is not changed, or `""`, meaning that the reference should
    -    be deleted.
    +    The two parameters added are `new_target` and `old_target`. The
    +    `new_target` is used to denote what the reference should point to when
    +    the transaction is applied. Some functions allow this parameter to be
    +    NULL, meaning that the reference is not changed.
     
    -    The `old_ref` denotes the value of that the reference must have before
    -    the update. Some functions allow this parameter to be NULL, meaning that
    -    the old value of the reference is not checked, or `""`, meaning that the
    -    reference must not exist before the update. A copy of this value is made
    +    The `old_target` denotes the value the reference must have before the
    +    update. Some functions allow this parameter to be NULL, meaning that the
    +    old value of the reference is not checked. A copy of this value is made
         in the transaction.
     
         The handling logic of these parameters will be added in consequent
    -    commits as we implement symref-{create, update, delete, verify}.
    +    commits as we add symref support to the existing 'git-update-ref'
    +    commands.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    @@ refs.c: struct ref_update *ref_transaction_add_update(
      		const char *refname, unsigned int flags,
      		const struct object_id *new_oid,
      		const struct object_id *old_oid,
    -+		const char *new_ref, const char *old_ref,
    ++		const char *new_target, const char *old_target,
      		const char *msg)
      {
      	struct ref_update *update;
    +@@ refs.c: struct ref_update *ref_transaction_add_update(
    + 	if (transaction->state != REF_TRANSACTION_OPEN)
    + 		BUG("update called for transaction that is not open");
    + 
    ++	if (old_oid && !is_null_oid(old_oid) && old_target)
    ++		BUG("Only one of old_oid and old_target should be non NULL");
    ++	if (new_oid && !is_null_oid(new_oid) && new_target)
    ++		BUG("Only one of new_oid and new_target should be non NULL");
    ++
    + 	FLEX_ALLOC_STR(update, refname, refname);
    + 	ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
    + 	transaction->updates[transaction->nr++] = update;
     @@ refs.c: int ref_transaction_update(struct ref_transaction *transaction,
      			   const char *refname,
      			   const struct object_id *new_oid,
      			   const struct object_id *old_oid,
    -+			   const char *new_ref, const char *old_ref,
    ++			   const char *new_target,
    ++			   const char *old_target,
      			   unsigned int flags, const char *msg,
      			   struct strbuf *err)
      {
    @@ refs.c: int ref_transaction_update(struct ref_transaction *transaction,
      
      	ref_transaction_add_update(transaction, refname, flags,
     -				   new_oid, old_oid, msg);
    -+				   new_oid, old_oid, new_ref, old_ref, msg);
    ++				   new_oid, old_oid, new_target,
    ++				   old_target, msg);
      	return 0;
      }
      
    @@ refs.c: int refs_update_ref(struct ref_store *refs, const char *msg,
     
      ## refs.h ##
     @@ refs.h: struct ref_transaction *ref_transaction_begin(struct strbuf *err);
    -  */
    - #define REF_SKIP_REFNAME_VERIFICATION (1 << 11)
    - 
    -+/*
    -+ * The reference update is considered to be done on a symbolic reference. This
    -+ * ensures that we verify, delete, create and update the ref correspondingly.
    -+ */
    -+#define REF_SYMREF_UPDATE (1 << 12)
    -+
    - /*
    -  * Bitmask of all of the flags that are allowed to be passed in to
    -  * ref_transaction_update() and friends:
    -  */
    - #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS                                  \
    - 	(REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION | \
    --	 REF_SKIP_REFNAME_VERIFICATION)
    -+	 REF_SKIP_REFNAME_VERIFICATION | REF_SYMREF_UPDATE )
    - 
    - /*
    -  * Add a reference update to transaction. `new_oid` is the value that
    +  *         before the update. A copy of this value is made in the
    +  *         transaction.
    +  *
    ++ *     new_target -- the target reference that the reference will be
    ++ *         update to point to. This takes precedence over new_oid when
    ++ *         set. If the reference is a regular reference, it will be
    ++ *         converted to a symbolic reference.
    ++ *
    ++ *     old_target -- the reference that the reference must be pointing to.
    ++ *         Will only be taken into account when the reference is a symbolic
    ++ *         reference.
    ++ *
    +  *     flags -- flags affecting the update, passed to
    +  *         update_ref_lock(). Possible flags: REF_NO_DEREF,
    +  *         REF_FORCE_CREATE_REFLOG. See those constants for more
    +@@ refs.h: struct ref_transaction *ref_transaction_begin(struct strbuf *err);
    +  * beforehand. The old value is checked after the lock is taken to
    +  * prevent races. If the old value doesn't agree with old_oid, the
    +  * whole transaction fails. If old_oid is NULL, then the previous
    +- * value is not checked.
    ++ * value is not checked. If `old_target` is not NULL, treat the reference
    ++ * as a symbolic ref and validate that its target before the update is
    ++ * `old_target`. If the `new_target` is not NULL, then the reference
    ++ * will be updated to a symbolic ref which targets `new_target`.
    ++ * Together, these allow us to update between regular refs and symrefs.
    +  *
    +  * See the above comment "Reference transaction updates" for more
    +  * information.
     @@ refs.h: int ref_transaction_update(struct ref_transaction *transaction,
      			   const char *refname,
      			   const struct object_id *new_oid,
      			   const struct object_id *old_oid,
    -+			   const char *new_ref, const char *old_ref,
    ++			   const char *new_target,
    ++			   const char *old_target,
      			   unsigned int flags, const char *msg,
      			   struct strbuf *err);
      
    @@ refs/refs-internal.h: struct ref_update {
      	struct object_id old_oid;
      
     +	/*
    -+	 * If (flags & REF_SYMREF_UPDATE), set the reference to this
    -+	 * value (or delete it, if `new_ref` is an empty string).
    ++	 * If set, point the reference to this value. This can also be
    ++	 * used to convert regular references to become symbolic refs.
     +	 */
    -+	const char *new_ref;
    ++	const char *new_target;
     +
     +	/*
    -+	 * If (type & REF_SYMREF_UPDATE), check that the reference
    -+	 * previously had this value (or didn't previously exist,
    -+	 * if `old_ref` is an empty string).
    ++	 * If set and the reference is a symbolic ref, check that the
    ++	 * reference previously pointed to this value.
     +	 */
    -+	const char *old_ref;
    ++	const char *old_target;
     +
      	/*
      	 * One or more of REF_NO_DEREF, REF_FORCE_CREATE_REFLOG,
    @@ refs/refs-internal.h: struct ref_update *ref_transaction_add_update(
      		const char *refname, unsigned int flags,
      		const struct object_id *new_oid,
      		const struct object_id *old_oid,
    -+		const char *new_ref, const char *old_ref,
    ++		const char *new_target, const char *old_target,
      		const char *msg);
      
      /*
2:  a8cb0e0a1d < -:  ---------- update-ref: add support for symref-verify
3:  37c3e006da < -:  ---------- update-ref: add support for symref-delete
-:  ---------- > 2:  37b7aadca4 update-ref: support parsing ref targets in `parse_next_oid`
4:  53fdb408ef = 3:  9cb7817f94 files-backend: extract out `create_symref_lock`
5:  8fa0151f94 < -:  ---------- update-ref: add support for symref-create
6:  714492ede3 < -:  ---------- update-ref: add support for symref-update
-:  ---------- > 4:  c7f43f6058 update-ref: support symrefs in the verify command
-:  ---------- > 5:  4016d6ca98 update-ref: support symrefs in the delete command
-:  ---------- > 6:  9f19e82f00 update-ref: support symrefs in the create command
-:  ---------- > 7:  132dbfcc5f update-ref: support symrefs in the update command
7:  c483104562 ! 8:  1b709f995b refs: support symrefs in 'reference-transaction' hook
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    refs: support symrefs in 'reference-transaction' hook
    +    ref: support symrefs in 'reference-transaction' hook
     
         The 'reference-transaction' hook runs whenever a reference update is
    -    made to the system. In the previous commits, we added support for
    -    various symref commands in `git-update-ref`. While it allowed us to now
    +    made to the system. In the previous commits, we added symref support for
    +    various commands in `git-update-ref`. While it allowed us to now
         manipulate symbolic refs via `git-update-ref`, it didn't activate the
         'reference-transaction' hook.
     
    @@ Commit message
         new format described for this and we stick to the existing format of:
             <old-value> SP <new-value> SP <ref-name> LF
         but now, <old-value> and <new-value> could also denote references
    -    instead of objects.
    +    instead of objects, where the format is similar to that in
    +    'git-update-ref', i.e. 'ref:<ref-target>'.
     
         While this seems to be backward incompatible, it is okay, since the only
         way the `reference-transaction` hook has refs in its output is when
    -    `git-update-ref` is used with `update-symref` command. Also the
    -    documentation for reference-transaction hook always stated that support
    -    for symbolic references may be added in the future.
    +    `git-update-ref` is used to manipulate symrefs. Also the documentation
    +    for reference-transaction hook always stated that support for symbolic
    +    references may be added in the future.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    @@ Documentation/githooks.txt: given reference transaction is in:
      `<ref-name>` via `git rev-parse`.
      
     +For symbolic reference updates the `<old_value>` and `<new-value>`
    -+fields could denote references instead of objects.
    ++fields could denote references instead of objects, denoted via the
    ++`ref:<ref-target>` format.
     +
      The exit status of the hook is ignored for any state except for the
      "prepared" state. In the "prepared" state, a non-zero exit status will
    @@ refs.c: static int run_transaction_hook(struct ref_transaction *transaction,
      
      	for (i = 0; i < transaction->nr; i++) {
      		struct ref_update *update = transaction->updates[i];
    -+		const char *new_value, *old_value;
    ++		strbuf_reset(&buf);
      
    --		if (update->flags & REF_SYMREF_UPDATE)
    +-		/*
    +-		 * Skip reference transaction for symbolic refs.
    +-		 */
    +-		if (update->new_target || update->old_target)
     -			continue;
    -+		new_value = oid_to_hex(&update->new_oid);
    -+		old_value = oid_to_hex(&update->old_oid);
    -+
    -+		if (update->flags & REF_SYMREF_UPDATE) {
    -+			if (update->flags & REF_HAVE_NEW && !null_new_value(update))
    -+				new_value = update->new_ref;
    -+			if (update->flags & REF_HAVE_OLD && update->old_ref)
    -+				old_value = update->old_ref;
    -+		}
    ++		if (update->flags & REF_HAVE_OLD && update->old_target)
    ++			strbuf_addf(&buf, "ref:%s ", update->old_target);
    ++		else
    ++			strbuf_addf(&buf, "%s ", oid_to_hex(&update->old_oid));
      
    - 		strbuf_reset(&buf);
    +-		strbuf_reset(&buf);
     -		strbuf_addf(&buf, "%s %s %s\n",
     -			    oid_to_hex(&update->old_oid),
     -			    oid_to_hex(&update->new_oid),
     -			    update->refname);
    -+		strbuf_addf(&buf, "%s %s %s\n", old_value, new_value, update->refname);
    ++		if (update->flags & REF_HAVE_NEW && update->new_target)
    ++			strbuf_addf(&buf, "ref:%s ", update->new_target);
    ++		else
    ++			strbuf_addf(&buf, "%s ", oid_to_hex(&update->new_oid));
    ++
    ++		strbuf_addf(&buf, "%s\n", update->refname);
      
      		if (write_in_full(proc.in, buf.buf, buf.len) < 0) {
      			if (errno != EPIPE) {
     
      ## t/t1416-ref-transaction-hooks.sh ##
    -@@ t/t1416-ref-transaction-hooks.sh: test_expect_success 'hook gets all queued updates in aborted state' '
    - 	test_cmp expect actual
    +@@ t/t1416-ref-transaction-hooks.sh: test_expect_success 'interleaving hook calls succeed' '
    + 	test_cmp expect target-repo.git/actual
      '
      
    -+# This test doesn't add a check for 'symref-delete' since there is a
    ++# This test doesn't add a check for symref 'delete' since there is a
     +# variation between the ref backends WRT 'delete'. In the files backend,
     +# 'delete' also triggers an additional transaction update on the
     +# packed-refs backend, which constitutes additional reflog entries.
    - test_expect_success 'interleaving hook calls succeed' '
    - 	test_when_finished "rm -r target-repo.git" &&
    - 
    -@@ t/t1416-ref-transaction-hooks.sh: test_expect_success 'interleaving hook calls succeed' '
    - 	test_cmp expect target-repo.git/actual
    - '
    - 
     +test_expect_success 'hook gets all queued symref updates' '
     +	test_when_finished "rm actual" &&
     +
    @@ t/t1416-ref-transaction-hooks.sh: test_expect_success 'interleaving hook calls s
     +
     +	cat >expect <<-EOF &&
     +		prepared
    -+		refs/heads/main $ZERO_OID refs/heads/symref
    -+		$ZERO_OID refs/heads/main refs/heads/symrefc
    -+		refs/heads/main refs/heads/branch refs/heads/symrefu
    ++		ref:refs/heads/main $ZERO_OID refs/heads/symref
    ++		$ZERO_OID ref:refs/heads/main refs/heads/symrefc
    ++		ref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu
     +		committed
    -+		refs/heads/main $ZERO_OID refs/heads/symref
    -+		$ZERO_OID refs/heads/main refs/heads/symrefc
    -+		refs/heads/main refs/heads/branch refs/heads/symrefu
    ++		ref:refs/heads/main $ZERO_OID refs/heads/symref
    ++		$ZERO_OID ref:refs/heads/main refs/heads/symrefc
    ++		ref:refs/heads/main ref:refs/heads/branch refs/heads/symrefu
     +	EOF
     +
     +	git update-ref --no-deref --stdin <<-EOF &&
     +		start
    -+		symref-verify refs/heads/symref refs/heads/main
    -+		symref-create refs/heads/symrefc refs/heads/main
    -+		symref-update refs/heads/symrefu refs/heads/branch refs/heads/main
    ++		verify refs/heads/symref ref:refs/heads/main
    ++		create refs/heads/symrefc ref:refs/heads/main
    ++		update refs/heads/symrefu ref:refs/heads/branch ref:refs/heads/main
     +		prepare
     +		commit
     +	EOF


Karthik Nayak (8):
  refs: accept symref values in `ref_transaction[_add]_update`
  update-ref: support parsing ref targets in `parse_next_oid`
  files-backend: extract out `create_symref_lock`
  update-ref: support symrefs in the verify command
  update-ref: support symrefs in the delete command
  update-ref: support symrefs in the create command
  update-ref: support symrefs in the update command
  ref: support symrefs in 'reference-transaction' hook

 Documentation/git-update-ref.txt |  41 ++--
 Documentation/githooks.txt       |  14 +-
 branch.c                         |   2 +-
 builtin/clone.c                  |   2 +-
 builtin/fast-import.c            |   5 +-
 builtin/fetch.c                  |   4 +-
 builtin/receive-pack.c           |   4 +-
 builtin/replace.c                |   2 +-
 builtin/tag.c                    |   1 +
 builtin/update-ref.c             | 106 ++++++++--
 refs.c                           |  78 +++++--
 refs.h                           |  23 +-
 refs/files-backend.c             | 141 +++++++++++--
 refs/refs-internal.h             |  20 ++
 refs/reftable-backend.c          |  49 ++++-
 sequencer.c                      |   9 +-
 t/t0600-reffiles-backend.sh      |  32 +++
 t/t1400-update-ref.sh            | 346 ++++++++++++++++++++++++++++++-
 t/t1416-ref-transaction-hooks.sh |  41 ++++
 walker.c                         |   2 +-
 20 files changed, 817 insertions(+), 105 deletions(-)

-- 
2.43.GIT


^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-06-10  8:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <https://lore.kernel.org/r/20240530120940.456817-1-knayak@gitlab.com>
2024-06-05 10:29 ` [PATCH v4 0/7] update-ref: add symref support for --stdin Karthik Nayak
2024-06-06  8:22   ` Karthik Nayak
2024-06-06 11:03   ` Karthik Nayak
2024-06-07 13:32   ` [PATCH v5 " Karthik Nayak
2024-06-07 13:32     ` [PATCH v5 1/7] refs: create and use `ref_update_expects_existing_old_ref()` Karthik Nayak
2024-06-07 13:32     ` [PATCH v5 2/7] refs: specify error for regular refs with `old_target` Karthik Nayak
2024-06-10  6:54       ` Patrick Steinhardt
2024-06-10  8:26         ` Karthik Nayak
2024-06-07 13:33     ` [PATCH v5 3/7] update-ref: add support for 'symref-verify' command Karthik Nayak
2024-06-07 13:33     ` [PATCH v5 4/7] update-ref: add support for 'symref-delete' command Karthik Nayak
2024-06-07 13:33     ` [PATCH v5 5/7] update-ref: add support for 'symref-create' command Karthik Nayak
2024-06-07 13:33     ` [PATCH v5 6/7] reftable: pick either 'oid' or 'target' for new updates Karthik Nayak
2024-06-07 13:33     ` [PATCH v5 7/7] update-ref: add support for 'symref-update' command Karthik Nayak
2024-06-05 10:29 ` [PATCH v4 1/7] refs: create and use `ref_update_expects_existing_old_ref()` Karthik Nayak
2024-06-05 10:29 ` [PATCH v4 2/7] refs: specify error for regular refs with `old_target` Karthik Nayak
2024-06-06 11:02   ` Patrick Steinhardt
2024-06-06 14:20     ` Karthik Nayak
2024-06-05 10:29 ` [PATCH v4 3/7] update-ref: add support for 'symref-verify' command Karthik Nayak
2024-06-05 10:29 ` [PATCH v4 4/7] update-ref: add support for 'symref-delete' command Karthik Nayak
2024-06-05 10:29 ` [PATCH v4 5/7] update-ref: add support for 'symref-create' command Karthik Nayak
2024-06-05 10:29 ` [PATCH v4 6/7] reftable: pick either 'oid' or 'target' for new updates Karthik Nayak
2024-06-05 10:29 ` [PATCH v4 7/7] update-ref: add support for 'symref-update' command Karthik Nayak
2024-04-23 21:28 [PATCH v3 0/8] refs: add symref support to 'git-update-ref' Karthik Nayak
2024-04-26 15:24 ` [PATCH v4 0/7] add symref-* commands to 'git-update-ref --stdin' Karthik Nayak
2024-04-26 15:24   ` [PATCH v4 4/7] update-ref: add support for 'symref-delete' command Karthik Nayak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).