git/refs, branch gitk-resize-error

Merge branch 'ab/refs-errno-cleanup'

2022-01-14T23:25:15Z

A brown-paper-bag fix on top of a topic that was merged during this cycle. * ab/refs-errno-cleanup: refs API: use "failure_errno", not "errno"

refs API: use "failure_errno", not "errno"

2022-01-13T18:53:54Z

Fix a logic error in refs_resolve_ref_unsafe() introduced in a recent series of mine to abstract the refs API away from errno. See 96f6623ada0 (Merge branch 'ab/refs-errno-cleanup', 2021-11-29)for that series. In that series introduction of "failure_errno" to refs_resolve_ref_unsafe came in ef18119dec8 (refs API: add a version of refs_resolve_ref_unsafe() with "errno", 2021-10-16). There we'd set "errno = 0" immediately before refs_read_raw_ref(), and then set "failure_errno" to "errno" if errno was non-zero afterwards. Then in the next commit 8b72fea7e91 (refs API: make refs_read_raw_ref() not set errno, 2021-10-16) we started expecting "refs_read_raw_ref()" to set "failure_errno". It would do that if refs_read_raw_ref() failed, but it wouldn't be the same errno. So we might set the "errno" here to any arbitrary bad value, and end up e.g. returning NULL when we meant to return the refname from refs_resolve_ref_unsafe(), or the other way around. Instrumenting this code will reveal cases where refs_read_raw_ref() will fail, and "errno" and "failure_errno" will be set to different values. In practice I haven't found a case where this scary bug changed anything in practice. The reason for that is that we'll not care about the actual value of "errno" here per-se, but only whether: 1. We have an errno 2. If it's one of ENOENT, EISDIR or ENOTDIR. See the adjacent code added in a1c1d8170db (refs_resolve_ref_unsafe: handle d/f conflicts for writes, 2017-10-06) I.e. if we clobber "failure_errno" with "errno", but it happened to be one of those three, and we'll clobber it with another one of the three we were OK. Perhaps there are cases where the difference ended up mattering, but I haven't found them. Instrumenting the test suite to fail if "errno" and "failure_errno" are different shows a lot of failures, checking if they're different *and* one is but not the other is outside that list of three "errno" values yields no failures. But let's fix the obvious bug. We should just stop paying attention to "errno" in refs_resolve_ref_unsafe(). In addition let's change the partial resetting of "errno" in files_read_raw_ref() to happen just before the "return", to ensure that any such bug will be more easily spotted in the future. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

Merge branch 'ab/reflog-prep'

2022-01-10T19:52:52Z

Code refactoring in the reflog part of refs API. * ab/reflog-prep: reflog + refs-backend: move "verbose" out of the backend refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN reflog: reduce scope of "struct rev_info" reflog expire: don't use lookup_commit_reference_gently() reflog expire: refactor & use "tip_commit" only for UE_NORMAL reflog expire: use "switch" over enum values reflog: change one->many worktree->refnames to use a string_list reflog expire: narrow scope of "cb" in cmd_reflog_expire() reflog delete: narrow scope of "cmd" passed to count_reflog_ent()

reflog + refs-backend: move "verbose" out of the backend

2021-12-23T00:24:14Z

Move the handling of the "verbose" flag entirely out of "refs/files-backend.c" and into "builtin/reflog.c". This allows the backend to stop knowing about the EXPIRE_REFLOGS_VERBOSE flag. The expire_reflog_ent() function shouldn't need to deal with the implementation detail of whether or not we're emitting verbose output, by doing this the --verbose output becomes backend-agnostic, so reftable will get the same output. I think the output is rather bad currently, and should e.g. be implemented with some better future mode of progress.[ch], but that's a topic for another improvement. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN

2021-12-23T00:24:14Z

It's not possible for "cb->newlog" to be NULL if !EXPIRE_REFLOGS_DRY_RUN, since files_reflog_expire() would have error()'d and taken the "goto failure" branch if it couldn't open the file. By not using the "newlog" field private to "file-backend.c"'s "struct expire_reflog_cb", we can move this verbosity logging to "builtin/reflog.c" in a subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

refs: centralize initialization of the base ref_store.

2021-12-22T21:51:38Z

Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano

refs: print error message in debug output

2021-12-22T21:51:37Z

Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano

refs: pass gitdir to packed_ref_store_create

2021-12-22T21:51:37Z

This is consistent with the calling convention for ref backend creation, and avoids storing ".git/packed-refs" (the name of a regular file) in a variable called ref_store::gitdir. Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano

Merge branch 'hn/allow-bogus-oid-in-ref-tests'

2021-12-15T17:39:54Z

The test helper for refs subsystem learned to write bogus and/or nonexistent object name to refs to simulate error situations we want to test Git in. * hn/allow-bogus-oid-in-ref-tests: t1430: create valid symrefs using test-helper t1430: remove refs using test-tool refs: introduce REF_SKIP_REFNAME_VERIFICATION flag refs: introduce REF_SKIP_OID_VERIFICATION flag refs: update comment. test-ref-store: plug memory leak in cmd_delete_refs test-ref-store: parse symbolic flag constants test-ref-store: remove force-create argument for create-reflog

Merge branch 'hn/reflog-tests'

2021-12-15T17:39:49Z

Prepare tests on ref API to help testing reftable backends. * hn/reflog-tests: refs/debug: trim trailing LF from reflog message test-ref-store: tweaks to for-each-reflog-ent format t1405: check for_each_reflog_ent_reverse() more thoroughly test-ref-store: don't add newline to reflog message show-branch: show reflog message