Re: [PATCH 06/10] refs: skip collision checks in initial transactions

From: karthik nayak <karthik.188@gmail.com>
Date: Nov 11, 2024, 10:53 UTC
Message-ID: <CAOLa=ZRrPx2BMe+b3iOQMK9U+GiY6UVYpRJyqxrUamAG2b2EYg@mail.gmail.com>
In-Reply-To: <20241108-pks-refs-optimize-migrations-v1-6-7fd37fa80e35@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

[snip]

Show 28 quoted lines

> diff --git a/refs.c b/refs.c
> index 0f10c565bbb4e0d91210c52a3221a224e4264d28..d690eb19b3fd7083a5309deb98738547e4f48040 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2324,6 +2324,7 @@ int refs_verify_refname_available(struct ref_store *refs,
>  				  const char *refname,
>  				  const struct string_list *extras,
>  				  const struct string_list *skip,
> +				  int initial_transaction,
>  				  struct strbuf *err)
>  {
>  	const char *slash;
> @@ -2332,8 +2333,6 @@ int refs_verify_refname_available(struct ref_store *refs,
>  	struct strbuf referent = STRBUF_INIT;
>  	struct object_id oid;
>  	unsigned int type;
> -	struct ref_iterator *iter;
> -	int ok;
>  	int ret = -1;
>
>  	/*
> @@ -2363,7 +2362,8 @@ int refs_verify_refname_available(struct ref_store *refs,
>  		if (skip && string_list_has_string(skip, dirname.buf))
>  			continue;
>
> -		if (!refs_read_raw_ref(refs, dirname.buf, &oid, &referent,
> +		if (!initial_transaction &&
> +		    !refs_read_raw_ref(refs, dirname.buf, &oid, &referent,

Nit: It would be nice to have a comment here, perhaps something like:
     /* in initial transaction there are no refs in the ref store */

Show 54 quoted lines

>  				       &type, &ignore_errno)) {
>  			strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
>  				    dirname.buf, refname);
> @@ -2388,21 +2388,26 @@ int refs_verify_refname_available(struct ref_store *refs,
>  	strbuf_addstr(&dirname, refname + dirname.len);
>  	strbuf_addch(&dirname, '/');
>
> -	iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0,
> -				       DO_FOR_EACH_INCLUDE_BROKEN);
> -	while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
> -		if (skip &&
> -		    string_list_has_string(skip, iter->refname))
> -			continue;
> +	if (!initial_transaction) {
> +		struct ref_iterator *iter;
> +		int ok;
>
> -		strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
> -			    iter->refname, refname);
> -		ref_iterator_abort(iter);
> -		goto cleanup;
> -	}
> +		iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0,
> +					       DO_FOR_EACH_INCLUDE_BROKEN);
> +		while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
> +			if (skip &&
> +			    string_list_has_string(skip, iter->refname))
> +				continue;
>
> -	if (ok != ITER_DONE)
> -		BUG("error while iterating over references");
> +			strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
> +				    iter->refname, refname);
> +			ref_iterator_abort(iter);
> +			goto cleanup;
> +		}
> +
> +		if (ok != ITER_DONE)
> +			BUG("error while iterating over references");
> +	}
>
>  	extra_refname = find_descendant_ref(dirname.buf, extras, skip);
>  	if (extra_refname)
> diff --git a/refs.h b/refs.h
> index 024a370554e013d66febee635e4c0415ba061fe6..980bd20cf24e15144aeff991eeba8b27a936d386 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -101,13 +101,16 @@ int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname,
>   * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
>   * "foo/barbados".
>   *
> + * If `initial_transaction` is truish, then all collision checks with
> + * preexisting refs are skipped.
> + *

Okay we have it here, so this should do.

[snip]