Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: chris.torek@gmail.com, git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH v2 5/7] update-ref: add support for symref-create
Date: Fri, 19 Apr 2024 11:40:41 +0200	[thread overview]
Message-ID: <ZiI8GaGupNzbLqnE@tanuki> (raw)
In-Reply-To: <20240412095908.1134387-6-knayak@gitlab.com>

[-- Attachment #1: Type: text/plain, Size: 5343 bytes --]

On Fri, Apr 12, 2024 at 11:59:06AM +0200, Karthik Nayak wrote:
> From: Karthik Nayak <karthik.188@gmail.com>
[snip]
> @@ -268,6 +268,39 @@ static void parse_cmd_create(struct ref_transaction *transaction,
>  	strbuf_release(&err);
>  }
>  
> +static void parse_cmd_symref_create(struct ref_transaction *transaction,
> +				    const char *next, const char *end)
> +{
> +	struct strbuf err = STRBUF_INIT;
> +	char *refname, *new_ref;
> +
> +	if (!(update_flags & REF_NO_DEREF))
> +                die("symref-create: cannot operate with deref mode");
> +
> +	refname = parse_refname(&next);
> +	if (!refname)
> +		die("symref-create: missing <ref>");
> +
> +	new_ref = parse_next_refname(&next);
> +	if (!new_ref)
> +		die("symref-create %s: missing <new-ref>", refname);
> +	if (read_ref(new_ref, NULL))
> +		die("symref-create %s: invalid <new-ref>", refname);

This restricts the creation of dangling symrefs, right? I think we might
want to lift this restriction, in which case we can eventually get rid
of the `create_symref` callback in ref backends completely.

> +	if (*next != line_termination)
> +		die("symref-create %s: extra input: %s", refname, next);
> +
> +	if (ref_transaction_create(transaction, refname, NULL, new_ref,
> +				   update_flags | create_reflog_flag |
> +				   REF_SYMREF_UPDATE, msg, &err))
> +		die("%s", err.buf);
> +
> +	update_flags = default_flags;
> +	free(refname);
> +	free(new_ref);
> +	strbuf_release(&err);
> +}
> +
>  static void parse_cmd_delete(struct ref_transaction *transaction,
>  			     const char *next, const char *end)
>  {
> @@ -476,6 +509,7 @@ static const struct parse_cmd {
>  	{ "create",        parse_cmd_create,        2, UPDATE_REFS_OPEN },
>  	{ "delete",        parse_cmd_delete,        2, UPDATE_REFS_OPEN },
>  	{ "verify",        parse_cmd_verify,        2, UPDATE_REFS_OPEN },
> +	{ "symref-create", parse_cmd_symref_create, 2, UPDATE_REFS_OPEN },
>  	{ "symref-delete", parse_cmd_symref_delete, 2, UPDATE_REFS_OPEN },
>  	{ "symref-verify", parse_cmd_symref_verify, 2, UPDATE_REFS_OPEN },
>  	{ "option",        parse_cmd_option,        1, UPDATE_REFS_OPEN },
> diff --git a/refs.c b/refs.c
> index 6d98d9652d..e62c0f4aca 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1305,15 +1305,20 @@ int ref_transaction_update(struct ref_transaction *transaction,
>  int ref_transaction_create(struct ref_transaction *transaction,
>  			   const char *refname,
>  			   const struct object_id *new_oid,
> +			   const char *new_ref,
>  			   unsigned int flags, const char *msg,
>  			   struct strbuf *err)
>  {
> -	if (!new_oid || is_null_oid(new_oid)) {
> +	if ((flags & REF_SYMREF_UPDATE) && !new_ref) {
> +		strbuf_addf(err, "'%s' has a no new ref", refname);
> +		return 1;
> +	}
> +	if (!(flags & REF_SYMREF_UPDATE) && (!new_oid || is_null_oid(new_oid))) {
>  		strbuf_addf(err, "'%s' has a null OID", refname);
>  		return 1;
>  	}
>  	return ref_transaction_update(transaction, refname, new_oid,
> -				      null_oid(), NULL, NULL, flags,
> +				      null_oid(), new_ref, NULL, flags,
>  				      msg, err);
>  }
>  
> diff --git a/refs.h b/refs.h
> index 60e6a21a31..c01a517e40 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -744,6 +744,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
>  int ref_transaction_create(struct ref_transaction *transaction,
>  			   const char *refname,
>  			   const struct object_id *new_oid,
> +			   const char *new_ref,
>  			   unsigned int flags, const char *msg,
>  			   struct strbuf *err);
>  
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 7c894ebe65..59d438878a 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2609,6 +2609,27 @@ static int lock_ref_for_update(struct files_ref_store *refs,
>  		}
>  	}
>  
> +	if (update->flags & REF_SYMREF_UPDATE && update->new_ref) {

Let's add braces around `(updaet->flags & REF_SYMREF_UPDATE)` to make
this easier to read.

> +		if (create_symref_lock(refs, lock, update->refname, update->new_ref)) {
> +			ret = TRANSACTION_GENERIC_ERROR;
> +			goto out;
> +		}
> +
> +		if (close_ref_gently(lock)) {
> +			strbuf_addf(err, "couldn't close '%s.lock'",
> +				    update->refname);
> +			ret = TRANSACTION_GENERIC_ERROR;
> +			goto out;
> +		}
> +
> +		/*
> +		 * Once we have created the symref lock, the commit
> +		 * phase of the transaction only needs to commit the lock.
> +		 */
> +		update->flags |= REF_NEEDS_COMMIT;
> +	}
> +
> +
>  	if ((update->flags & REF_HAVE_NEW) &&
>  	    !(update->flags & REF_DELETING) &&
>  	    !(update->flags & REF_LOG_ONLY)) {
> @@ -2904,6 +2925,14 @@ static int files_transaction_finish(struct ref_store *ref_store,
>  
>  		if (update->flags & REF_NEEDS_COMMIT ||
>  		    update->flags & REF_LOG_ONLY) {
> +			if (update->flags & REF_SYMREF_UPDATE && update->new_ref) {

Here, as well.

> +				/* for dangling symrefs we gracefully set the oid to zero */
> +				if (!refs_resolve_ref_unsafe(&refs->base, update->new_ref,
> +							     RESOLVE_REF_READING, &update->new_oid, NULL)) {
> +					update->new_oid = *null_oid();
> +				}

Can this actually happenn right now? I thought that the `read_ref()`
further up forbids this case.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2024-04-19  9:40 UTC|newest]

Thread overview: 194+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-30 22:46 [PATCH 0/8] update-ref: add support for update-symref option Karthik Nayak
2024-03-30 22:46 ` [PATCH 1/8] files-backend: extract out `create_symref_lock` Karthik Nayak
2024-04-02 12:20   ` Patrick Steinhardt
2024-04-03 14:52     ` Karthik Nayak
2024-03-30 22:46 ` [PATCH 2/8] reftable-backend: extract out `write_symref_with_log` Karthik Nayak
2024-04-02 12:20   ` Patrick Steinhardt
2024-03-30 22:46 ` [PATCH 3/8] reftable-backend: move `write_symref_with_log` up Karthik Nayak
2024-03-30 22:46 ` [PATCH 4/8] refs: accept symref in `ref_transaction_add_update` Karthik Nayak
2024-03-30 22:46 ` [PATCH 5/8] refs/files-backend: add support for symref updates Karthik Nayak
2024-04-02 12:20   ` Patrick Steinhardt
2024-03-30 22:46 ` [PATCH 6/8] refs/reftable-backend: " Karthik Nayak
2024-04-02 12:20   ` Patrick Steinhardt
2024-03-30 22:46 ` [PATCH 7/8] refs: add 'update-symref' command to 'update-ref' Karthik Nayak
2024-03-31 22:08   ` Junio C Hamano
2024-03-31 22:27     ` Chris Torek
2024-03-31 23:14       ` Junio C Hamano
2024-04-01  1:31         ` Junio C Hamano
2024-04-02 12:20           ` Patrick Steinhardt
2024-04-02 16:40             ` Junio C Hamano
2024-04-09 11:55               ` Patrick Steinhardt
2024-04-09 16:15                 ` Karthik Nayak
2024-04-10  4:20                   ` Patrick Steinhardt
2024-04-10 16:06                     ` Junio C Hamano
2024-04-10 17:31                       ` Patrick Steinhardt
2024-04-01 10:38       ` Karthik Nayak
2024-04-01 11:48     ` Karthik Nayak
2024-04-01 16:17       ` Junio C Hamano
2024-04-01 20:40         ` Junio C Hamano
2024-04-01 22:37         ` Karthik Nayak
2024-03-30 22:46 ` [PATCH 8/8] refs: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-04-02 12:20   ` Patrick Steinhardt
2024-04-12  9:59 ` [PATCH v2 0/7] update-ref: add symref oriented commands Karthik Nayak
2024-04-12  9:59   ` [PATCH v2 1/7] refs: accept symref values in `ref_transaction[_add]_update` Karthik Nayak
2024-04-18 14:25     ` Christian Couder
2024-04-19 10:28       ` Karthik Nayak
2024-04-18 15:08     ` Phillip Wood
2024-04-19  9:40       ` Patrick Steinhardt
2024-04-19 15:47       ` Karthik Nayak
2024-05-04 15:15         ` phillip.wood123
2024-04-19  9:40     ` Patrick Steinhardt
2024-04-19 18:09       ` Karthik Nayak
2024-04-23  6:31         ` Patrick Steinhardt
2024-04-23 10:48           ` Karthik Nayak
2024-04-12  9:59   ` [PATCH v2 2/7] update-ref: add support for symref-verify Karthik Nayak
2024-04-18 14:26     ` Christian Couder
2024-04-19 21:21       ` Karthik Nayak
2024-04-19  9:40     ` Patrick Steinhardt
2024-04-19 21:53       ` Karthik Nayak
2024-04-12  9:59   ` [PATCH v2 3/7] update-ref: add support for symref-delete Karthik Nayak
2024-04-18 14:52     ` Christian Couder
2024-04-21 10:43       ` Karthik Nayak
2024-04-19  9:40     ` Patrick Steinhardt
2024-04-21 10:45       ` Karthik Nayak
2024-04-12  9:59   ` [PATCH v2 4/7] files-backend: extract out `create_symref_lock` Karthik Nayak
2024-04-12  9:59   ` [PATCH v2 5/7] update-ref: add support for symref-create Karthik Nayak
2024-04-19  9:40     ` Patrick Steinhardt [this message]
2024-04-19 15:48       ` Junio C Hamano
2024-04-21 12:50       ` Karthik Nayak
2024-04-21 15:57         ` Karthik Nayak
2024-04-23  6:39         ` Patrick Steinhardt
2024-04-23 10:52           ` Karthik Nayak
2024-04-12  9:59   ` [PATCH v2 6/7] update-ref: add support for symref-update Karthik Nayak
2024-04-19  9:40     ` Patrick Steinhardt
2024-04-21 19:00       ` Karthik Nayak
2024-04-23  6:49         ` Patrick Steinhardt
2024-04-23 11:30           ` Karthik Nayak
2024-04-12  9:59   ` [PATCH v2 7/7] refs: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-04-12 18:01   ` [PATCH v2 0/7] update-ref: add symref oriented commands Junio C Hamano
2024-04-12 18:49     ` Karthik Nayak
2024-04-18 15:05   ` Christian Couder
2024-04-21 19:06     ` Karthik Nayak
2024-04-20  6:16   ` Patrick Steinhardt
2024-04-21 19:11     ` Karthik Nayak
2024-04-23 21:28   ` [PATCH v3 0/8] refs: add symref support to 'git-update-ref' Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 1/8] refs: accept symref values in `ref_transaction[_add]_update` Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 2/8] update-ref: support parsing ref targets in `parse_next_oid` Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 3/8] files-backend: extract out `create_symref_lock` Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 4/8] update-ref: support symrefs in the verify command Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 5/8] update-ref: support symrefs in the delete command Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 6/8] update-ref: support symrefs in the create command Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 7/8] update-ref: support symrefs in the update command Karthik Nayak
2024-04-23 21:28     ` [PATCH v3 8/8] ref: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-04-23 22:03     ` [PATCH v3 0/8] refs: add symref support to 'git-update-ref' Jeff King
2024-04-24  1:17       ` Junio C Hamano
2024-04-24 16:25       ` Karthik Nayak
2024-04-25  6:40         ` Patrick Steinhardt
2024-04-25 21:12           ` Karthik Nayak
2024-04-25 18:01         ` Junio C Hamano
2024-04-25 21:14           ` Karthik Nayak
2024-04-25 21:55             ` Junio C Hamano
2024-04-26 12:48               ` Karthik Nayak
2024-04-26 20:41         ` Jeff King
2024-04-25 17:09     ` Junio C Hamano
2024-04-25 21:07       ` 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 1/7] refs: accept symref values in `ref_transaction[_add]_update` Karthik Nayak
2024-04-26 19:31         ` Junio C Hamano
2024-04-26 21:15           ` Jeff King
2024-04-29  7:02             ` Patrick Steinhardt
2024-04-29  7:55               ` Jeff King
2024-04-29  9:29                 ` phillip.wood123
2024-04-29  9:32             ` phillip.wood123
2024-04-29 16:18               ` Junio C Hamano
2024-04-30 10:33                 ` Jeff King
2024-04-30 10:30               ` Jeff King
2024-04-28 19:36           ` Karthik Nayak
2024-04-29 13:38         ` Phillip Wood
2024-04-29 14:01           ` Karthik Nayak
2024-04-26 15:24       ` [PATCH v4 2/7] files-backend: extract out `create_symref_lock` Karthik Nayak
2024-04-26 21:39         ` Junio C Hamano
2024-04-28 19:57           ` Karthik Nayak
2024-04-26 15:24       ` [PATCH v4 3/7] update-ref: add support for 'symref-verify' command Karthik Nayak
2024-04-26 22:51         ` Junio C Hamano
2024-04-28 22:28           ` Karthik Nayak
2024-04-26 15:24       ` [PATCH v4 4/7] update-ref: add support for 'symref-delete' command Karthik Nayak
2024-04-26 15:24       ` [PATCH v4 5/7] update-ref: add support for 'symref-create' command Karthik Nayak
2024-04-26 15:24       ` [PATCH v4 6/7] update-ref: add support for 'symref-update' command Karthik Nayak
2024-04-26 15:24       ` [PATCH v4 7/7] ref: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-04-30 10:14       ` [PATCH v4 0/7] add symref-* commands to 'git-update-ref --stdin' Karthik Nayak
2024-05-01 20:22       ` [PATCH v5 0/7] refs: add support for transactional symref updates Karthik Nayak
2024-05-01 20:22         ` [PATCH v5 1/7] refs: accept symref values in `ref_transaction_update()` Karthik Nayak
2024-05-01 20:22         ` [PATCH v5 2/7] files-backend: extract out `create_symref_lock()` Karthik Nayak
2024-05-01 22:06           ` Junio C Hamano
2024-05-02  7:47             ` Patrick Steinhardt
2024-05-02 11:05               ` Karthik Nayak
2024-05-02 16:49               ` Junio C Hamano
2024-05-01 20:22         ` [PATCH v5 3/7] refs: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-05-01 23:05           ` Junio C Hamano
2024-05-02  5:32             ` Karthik Nayak
2024-05-01 20:22         ` [PATCH v5 4/7] refs: add support for transactional symref updates Karthik Nayak
2024-05-01 23:52           ` Junio C Hamano
2024-05-02  5:50             ` Karthik Nayak
2024-05-02  7:47               ` Patrick Steinhardt
2024-05-02 11:10                 ` Karthik Nayak
2024-05-02 16:51                 ` Junio C Hamano
2024-05-02 16:00               ` Junio C Hamano
2024-05-02 17:53           ` Junio C Hamano
2024-05-01 20:22         ` [PATCH v5 5/7] refs: use transaction in `refs_create_symref()` Karthik Nayak
2024-05-02  7:47           ` Patrick Steinhardt
2024-05-01 20:22         ` [PATCH v5 6/7] refs: rename `refs_create_symref()` to `refs_update_symref()` Karthik Nayak
2024-05-02  7:47           ` Patrick Steinhardt
2024-05-02 11:34             ` Karthik Nayak
2024-05-01 20:22         ` [PATCH v5 7/7] refs: remove `create_symref` and associated dead code Karthik Nayak
2024-05-02  7:47           ` Patrick Steinhardt
2024-05-02 16:53             ` Junio C Hamano
2024-05-02  0:20         ` [PATCH v5 0/7] refs: add support for transactional symref updates Junio C Hamano
2024-05-02  5:53           ` Karthik Nayak
2024-05-03 12:41         ` [PATCH v6 " Karthik Nayak
2024-05-03 12:41           ` [PATCH v6 1/7] refs: accept symref values in `ref_transaction_update()` Karthik Nayak
2024-05-04 15:18             ` Phillip Wood
2024-05-05 15:10               ` Karthik Nayak
2024-05-05 15:19                 ` phillip.wood123
2024-05-03 12:41           ` [PATCH v6 2/7] files-backend: extract out `create_symref_lock()` Karthik Nayak
2024-05-03 12:41           ` [PATCH v6 3/7] refs: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-05-03 12:41           ` [PATCH v6 4/7] refs: add support for transactional symref updates Karthik Nayak
2024-05-05 14:09             ` Phillip Wood
2024-05-05 16:09               ` Karthik Nayak
2024-05-06  9:35                 ` Phillip Wood
2024-05-06 11:19                   ` Karthik Nayak
2024-05-06 13:19                     ` Phillip Wood
2024-05-06  9:54                 ` Phillip Wood
2024-05-06 11:22                   ` Karthik Nayak
2024-05-06 13:17                     ` Phillip Wood
2024-05-03 12:41           ` [PATCH v6 5/7] refs: use transaction in `refs_create_symref()` Karthik Nayak
2024-05-03 12:41           ` [PATCH v6 6/7] refs: rename `refs_create_symref()` to `refs_update_symref()` Karthik Nayak
2024-05-03 12:41           ` [PATCH v6 7/7] refs: remove `create_symref` and associated dead code Karthik Nayak
2024-05-03 23:09             ` Junio C Hamano
2024-05-04  9:30               ` Karthik Nayak
2024-05-03 16:45           ` [PATCH v6 0/7] refs: add support for transactional symref updates Junio C Hamano
2024-05-06  7:36           ` Patrick Steinhardt
2024-05-07  6:00           ` [PATCH v7 0/8] " Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 1/8] refs: accept symref values in `ref_transaction_update()` Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 2/8] files-backend: extract out `create_symref_lock()` Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 3/8] refs: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 4/8] refs: move `original_update_refname` to 'refs.c' Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 5/8] refs: add support for transactional symref updates Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 6/8] refs: use transaction in `refs_create_symref()` Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 7/8] refs: rename `refs_create_symref()` to `refs_update_symref()` Karthik Nayak
2024-05-07  6:00             ` [PATCH v7 8/8] refs: remove `create_symref` and associated dead code Karthik Nayak
2024-05-07  6:25             ` [PATCH v7 0/8] refs: add support for transactional symref updates Junio C Hamano
2024-05-07  6:31               ` Junio C Hamano
2024-05-07 12:58             ` [PATCH v8 " Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 1/8] refs: accept symref values in `ref_transaction_update()` Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 2/8] files-backend: extract out `create_symref_lock()` Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 3/8] refs: support symrefs in 'reference-transaction' hook Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 4/8] refs: move `original_update_refname` to 'refs.c' Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 5/8] refs: add support for transactional symref updates Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 6/8] refs: use transaction in `refs_create_symref()` Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 7/8] refs: rename `refs_create_symref()` to `refs_update_symref()` Karthik Nayak
2024-05-07 12:58               ` [PATCH v8 8/8] refs: remove `create_symref` and associated dead code Karthik Nayak
2024-05-07 15:50               ` [PATCH v8 0/8] refs: add support for transactional symref updates phillip.wood123
2024-05-07 16:32               ` Junio C Hamano
2024-05-12 17:17                 ` Karthik Nayak
2024-05-13 17:15                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZiI8GaGupNzbLqnE@tanuki \
    --to=ps@pks.im \
    --cc=chris.torek@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).