[PATCH v8 0/7] refs: cleanup errno sideband ref related functions

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Han-Wen Nienhuys" <hanwen@google.com>,
	"Michael Haggerty" <mhagger@alum.mit.edu>,
	"Jonathan Tan" <jonathantanmy@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v8 0/7] refs: cleanup errno sideband ref related functions
Date: Fri, 16 Jul 2021 16:22:25 +0200	[thread overview]
Message-ID: <cover-0.7-00000000000-20210716T142032Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-0.6-0000000000-20210714T114301Z-avarab@gmail.com>

A v8 re-roll of v7 of this, covered in detail at:
https://lore.kernel.org/git/cover-0.6-0000000000-20210714T114301Z-avarab@gmail.com/

This topic relies on the now-re-rolled and bigger dependency topic to
cleanup the use of the reflog API, which is 1/4 callers (and the
complex one) that need an API being simplified here, see
https://lore.kernel.org/git/cover-00.11-00000000000-20210716T140631Z-avarab@gmail.com

Han-Wen Nienhuys (6):
  refs: remove EINVAL errno output from specification of read_raw_ref_fn
  refs/files-backend: stop setting errno from lock_ref_oid_basic
  refs: make errno output explicit for read_raw_ref_fn
  refs: add failure_errno to refs_read_raw_ref() signature
  refs: explicitly return failure_errno from parse_loose_ref_contents
  refs: make errno output explicit for refs_resolve_ref_unsafe

Ævar Arnfjörð Bjarmason (1):
  refs file backend: move raceproof_create_file() here

 cache.h               |  43 ---------
 object-file.c         |  68 --------------
 refs.c                |  69 +++++++++-----
 refs.h                |  11 +++
 refs/debug.c          |   4 +-
 refs/files-backend.c  | 207 +++++++++++++++++++++++++++++++++---------
 refs/packed-backend.c |  15 +--
 refs/refs-internal.h  |  32 ++++---
 8 files changed, 250 insertions(+), 199 deletions(-)

Range-diff against v7:
-:  ----------- > 1:  ce1ca2cf30f refs file backend: move raceproof_create_file() here
1:  4beba4443ab = 2:  2a69bbea821 refs: remove EINVAL errno output from specification of read_raw_ref_fn
2:  fd8e3561851 ! 3:  a3f80c6d2f7 refs/files-backend: stop setting errno from lock_ref_oid_basic
    @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_re
      	struct strbuf ref_file = STRBUF_INIT;
      	struct ref_lock *lock;
     -	int last_errno = 0;
    - 	int mustexist = (old_oid && !is_null_oid(old_oid));
    - 	int resolve_flags = RESOLVE_REF_NO_RECURSE;
    - 	int resolved;
    + 
    + 	files_assert_main_repository(refs, "lock_ref_oid_basic");
    + 	assert(err);
     @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
    - 	resolved = !!refs_resolve_ref_unsafe(&refs->base,
    - 					     refname, resolve_flags,
    - 					     &lock->old_oid, type);
    --	if (!resolved) {
    + 	files_ref_path(refs, &ref_file, refname);
    + 	if (!refs_resolve_ref_unsafe(&refs->base, refname,
    + 				     RESOLVE_REF_NO_RECURSE,
    +-				     &lock->old_oid, type)) {
     -		last_errno = errno;
     -		if (last_errno != ENOTDIR ||
     -		    !refs_verify_refname_available(&refs->base, refname,
    --						   extras, skip, err))
    +-						   NULL, NULL, err))
     -			strbuf_addf(err, "unable to resolve reference '%s': %s",
     -				    refname, strerror(last_errno));
    -+	if (!resolved &&
    +-
    ++				     &lock->old_oid, type) &&
     +	    (errno != ENOTDIR ||
     +	     /* in case of D/F conflict, try to generate a better error
     +	      * message. If that fails, fall back to strerror(ENOTDIR).
     +	      */
    -+	     !refs_verify_refname_available(&refs->base, refname, extras,
    -+					    skip, err))) {
    ++	     !refs_verify_refname_available(&refs->base, refname, NULL,
    ++					    NULL, err))) {
     +		strbuf_addf(err, "unable to resolve reference '%s': %s",
     +			    refname, strerror(errno));
    - 
      		goto error_return;
      	}
    + 
     @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
    + 	 */
      	if (is_null_oid(&lock->old_oid) &&
      	    refs_verify_refname_available(refs->packed_ref_store, refname,
    - 					  extras, skip, err)) {
    +-					  NULL, NULL, err)) {
     -		last_errno = ENOTDIR;
    ++					  NULL, NULL, err))
      		goto error_return;
    - 	}
    +-	}
      
      	lock->ref_name = xstrdup(refname);
      
    @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_re
      		unable_to_lock_message(ref_file.buf, errno, err);
      		goto error_return;
      	}
    - 
    - 	if (verify_lock(&refs->base, lock, old_oid, mustexist, err)) {
    --		last_errno = errno;
    - 		goto error_return;
    - 	}
    - 	goto out;
     @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
      
       out:
3:  913e72e9ffd = 4:  147058c8c3d refs: make errno output explicit for read_raw_ref_fn
4:  dd191768f66 = 5:  b42a7474f18 refs: add failure_errno to refs_read_raw_ref() signature
5:  77c53dc0d93 = 6:  93b770c8bea refs: explicitly return failure_errno from parse_loose_ref_contents
6:  5d4b1b4ddff ! 7:  cb32b5c0526 refs: make errno output explicit for refs_resolve_ref_unsafe
    @@ refs.h: const char *refs_resolve_ref_unsafe(struct ref_store *refs,
     
      ## refs/files-backend.c ##
     @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
    - 	int mustexist = (old_oid && !is_null_oid(old_oid));
    - 	int resolve_flags = RESOLVE_REF_NO_RECURSE;
    - 	int resolved;
    + {
    + 	struct strbuf ref_file = STRBUF_INIT;
    + 	struct ref_lock *lock;
     +	int resolve_errno = 0;
      
      	files_assert_main_repository(refs, "lock_ref_oid_basic");
      	assert(err);
     @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
    - 		resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;
    + 	CALLOC_ARRAY(lock, 1);
      
      	files_ref_path(refs, &ref_file, refname);
    --	resolved = !!refs_resolve_ref_unsafe(&refs->base,
    --					     refname, resolve_flags,
    --					     &lock->old_oid, type);
    -+	resolved = !!refs_resolve_ref_unsafe_with_errno(&refs->base, refname,
    -+							resolve_flags,
    -+							&lock->old_oid, type,
    -+							&resolve_errno);
    - 	if (!resolved &&
    +-	if (!refs_resolve_ref_unsafe(&refs->base, refname,
    +-				     RESOLVE_REF_NO_RECURSE,
    +-				     &lock->old_oid, type) &&
     -	    (errno != ENOTDIR ||
    ++	if (!refs_resolve_ref_unsafe_with_errno(&refs->base, refname,
    ++						RESOLVE_REF_NO_RECURSE,
    ++						&lock->old_oid, type,
    ++						&resolve_errno) &&
     +	    (resolve_errno != ENOTDIR ||
      	     /* in case of D/F conflict, try to generate a better error
      	      * message. If that fails, fall back to strerror(ENOTDIR).
      	      */
    - 	     !refs_verify_refname_available(&refs->base, refname, extras,
    - 					    skip, err))) {
    + 	     !refs_verify_refname_available(&refs->base, refname, NULL,
    + 					    NULL, err))) {
      		strbuf_addf(err, "unable to resolve reference '%s': %s",
     -			    refname, strerror(errno));
    --
     +			    refname, strerror(resolve_errno));
      		goto error_return;
      	}
-- 
2.32.0.874.gfa1990a4f10