| Message ID | cover-0.7-00000000000-20210720T102644Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | refs: cleanup errno sideband ref related functions | expand |
On Tue, Jul 20 2021, Ævar Arnfjörð Bjarmason wrote: > A v9 re-roll of the v8, see > https://lore.kernel.org/git/cover-0.7-00000000000-20210716T142032Z-avarab@gmail.com/ > and more imporantly the real summary of what this is in v7 at > https://lore.kernel.org/git/cover-0.6-0000000000-20210714T114301Z-avarab@gmail.com/ FYI I did not re-roll a v10 of this on top of the just re-rolled base topic: https://lore.kernel.org/git/cover-00.11-0000000000-20210726T234237Z-avarab@gmail.com/#t The range-diff I have locally between the two (this version, and my rebased "should I need a re-roll" version) shows no changes, so this still cleanly applies on top.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Tue, Jul 20 2021, Ævar Arnfjörð Bjarmason wrote: > >> A v9 re-roll of the v8, see >> https://lore.kernel.org/git/cover-0.7-00000000000-20210716T142032Z-avarab@gmail.com/ >> and more imporantly the real summary of what this is in v7 at >> https://lore.kernel.org/git/cover-0.6-0000000000-20210714T114301Z-avarab@gmail.com/ > > FYI I did not re-roll a v10 of this on top of the just re-rolled base > topic: > https://lore.kernel.org/git/cover-00.11-0000000000-20210726T234237Z-avarab@gmail.com/#t > > The range-diff I have locally between the two (this version, and my > rebased "should I need a re-roll" version) shows no changes, so this > still cleanly applies on top. Thanks. I just rebased it on top of the base topic locally. No need to resend.
A v9 re-roll of the v8, see https://lore.kernel.org/git/cover-0.7-00000000000-20210716T142032Z-avarab@gmail.com/ and more imporantly the real summary of what this is in v7 at https://lore.kernel.org/git/cover-0.6-0000000000-20210714T114301Z-avarab@gmail.com/ As noted there this topic builds on my just-re-rolled v3 removing of dead code + fixing a gc race in the refs API, at: https://lore.kernel.org/git/cover-00.12-00000000000-20210720T102051Z-avarab@gmail.com/ The changes in this v8 are merely to re-roll on top of that. There was a stray comment that didn't apply anymore with its new 12th patch. I also removed the "in case of D/F conflict..." comment the 3rd patch previously added, with the changes in the base topic I think this codepath has become easily understood enough to not need an new comment anymore (the initial version was more complex, see https://lore.kernel.org/git/95025080c16f535599826ed4f013845d712b0e8d.1625684869.git.gitgitgadget@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 | 195 +++++++++++++++++++++++++++++++++--------- refs/packed-backend.c | 15 ++-- refs/refs-internal.h | 32 ++++--- 8 files changed, 243 insertions(+), 194 deletions(-) Range-diff against v8: 1: ce1ca2cf30f = 1: b7063c5af89 refs file backend: move raceproof_create_file() here 2: 2a69bbea821 = 2: 5a63b64f53f refs: remove EINVAL errno output from specification of read_raw_ref_fn 3: a3f80c6d2f7 ! 3: 0dd8a4c1209 refs/files-backend: stop setting errno from lock_ref_oid_basic @@ Commit message refs_reflog_exists() (which calls a function in a vtable that is not documented to use and/or preserve errno) - In the case of the "errno != ENOTDIR" case that originates in 5b2d8d6f218 - (lock_ref_sha1_basic(): improve diagnostics for ref D/F conflicts, - 2015-05-11), there the "last_errno" was saved away to return it from - lock_ref_oid_basic(), now that we're no longer doing that we can skip - that entirely and use "errno" directly. A follow-up change will - extract the specific errno we want earlier in this function. - Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_re 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, - files_ref_path(refs, &ref_file, refname); if (!refs_resolve_ref_unsafe(&refs->base, refname, RESOLVE_REF_NO_RECURSE, -- &lock->old_oid, type)) { + &lock->old_oid, type)) { - last_errno = errno; -- if (last_errno != ENOTDIR || -- !refs_verify_refname_available(&refs->base, refname, -- NULL, NULL, err)) -- strbuf_addf(err, "unable to resolve reference '%s': %s", + if (!refs_verify_refname_available(&refs->base, refname, + NULL, NULL, err)) + strbuf_addf(err, "unable to resolve reference '%s': %s", - refname, strerror(last_errno)); -- -+ &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, NULL, -+ NULL, err))) { -+ strbuf_addf(err, "unable to resolve reference '%s': %s", -+ refname, strerror(errno)); ++ 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) && 4: 147058c8c3d = 4: c54fb99714a refs: make errno output explicit for read_raw_ref_fn 5: b42a7474f18 = 5: 18825efce7d refs: add failure_errno to refs_read_raw_ref() signature 6: 93b770c8bea = 6: 57e3f246f4f refs: explicitly return failure_errno from parse_loose_ref_contents 7: cb32b5c0526 ! 7: 4b5e168b978 refs: make errno output explicit for refs_resolve_ref_unsafe @@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_re files_ref_path(refs, &ref_file, refname); - if (!refs_resolve_ref_unsafe(&refs->base, refname, - RESOLVE_REF_NO_RECURSE, -- &lock->old_oid, type) && -- (errno != ENOTDIR || +- &lock->old_oid, type)) { + 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, NULL, - NULL, err))) { - strbuf_addf(err, "unable to resolve reference '%s': %s", -- refname, strerror(errno)); -+ refname, strerror(resolve_errno)); ++ &resolve_errno)) { + if (!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; } -