Re: [PATCH v2 2/8] refs/files-backend: stop setting errno from lock_ref_oid_basic

On Thu, Jul 1, 2021 at 1:30 PM Ævar Arnfjörð Bjarmason [off-list ref] wrote:

quoted

      }
      if (!resolved) {
-             last_errno = errno;
+             int last_errno = errno;
              if (last_errno != ENOTDIR ||
                  !refs_verify_refname_available(&refs->base, refname,
                                                 extras, skip, err))

...this particular change gives me some pause, because all the rest is
about squirreling away our own errno for our own caller (which it turns
out, we didn't need).

But in this case we're only guarding against
refs_verify_refname_available() possibly clobbering the errno we just
got on !resolved in refs_verify_refname_available().

This comes from 5b2d8d6f2184381b76c13504a2f5ec8a62cd584e

   .. invoke verify_refname_available() to try
    to generate a more helpful error message.

    That function might not detect an error. For example, some
    non-reference file might be blocking the deletion of an
    otherwise-empty directory tree, or there might be a race with another
    process that just deleted the offending reference. In such cases,
    generate the strerror-based error message like before.

I think we want to keep this behavior, hence I think this should be kept as is.

I'll add a comment.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado