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>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 00/21] refs: stop having the API set "errno"
Date: Sat, 16 Oct 2021 11:39:06 +0200 [thread overview]
Message-ID: <cover-v2-00.21-00000000000-20211016T093845Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-00.20-00000000000-20211013T235900Z-avarab@gmail.com>
See [1] for the v1 summary. There was a regression there with "git
reflog"'s "--updateref" option, which had no meaningful tests in the
test suite (only one test in "t1503-rev-parse-verify.sh"). Test for
it, and address Eric's comment about "t3200-branch.sh" by
re-structuring that test away from a for-loop.
1. https://lore.kernel.org/git/cover-00.20-00000000000-20211013T235900Z-avarab@gmail.com/
Han-Wen Nienhuys (3):
branch tests: test for errno propagating on failing read
refs API: make refs_read_raw_ref() not set errno
refs API: make parse_loose_ref_contents() not set errno
Ævar Arnfjörð Bjarmason (18):
refs API: add a version of refs_resolve_ref_unsafe() with "errno"
refs API: make refs_rename_ref_available() static
reflog tests: add --updateref tests
refs/files: remove "name exist?" check in lock_ref_oid_basic()
refs API: remove refs_read_ref_full() wrapper
refs API: make resolve_gitlink_ref() not set errno
refs API: make loose_fill_ref_dir() not set errno
refs API: make files_copy_or_rename_ref() et al not set errno
refs API: ignore errno in worktree.c's add_head_info()
refs API: ignore errno in worktree.c's find_shared_symref()
refs tests: ignore ignore errno in test-ref-store helper
refs API: make refs_resolve_refdup() not set errno
refs API: make refs_ref_exists() not set errno
refs API: make resolve_ref_unsafe() not set errno
refs API: make expand_ref() & repo_dwim_log() not set errno
refs API: don't expose "errno" in run_transaction_hook()
refs API: post-migration API renaming [1/2]
refs API: post-migration API renaming [2/2]
refs.c | 124 +++++++++++++++--------------
refs.h | 10 ++-
refs/files-backend.c | 153 ++++++++++++++++++++++++------------
refs/packed-backend.c | 7 +-
refs/refs-internal.h | 26 ++----
sequencer.c | 10 ++-
t/helper/test-ref-store.c | 3 +-
t/t1417-reflog-updateref.sh | 65 +++++++++++++++
t/t3200-branch.sh | 22 ++++++
worktree.c | 17 ++--
10 files changed, 294 insertions(+), 143 deletions(-)
create mode 100755 t/t1417-reflog-updateref.sh
Range-diff against v1:
1: bea88e382c0 ! 1: 1863c597c98 branch tests: test for errno propagating on failing read
@@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m u v should fail w
+test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
+ test_when_finished "rm -rf subdir" &&
-+ git init subdir &&
++ git init --bare subdir &&
+
-+ (
-+ cd subdir &&
-+ for d in refs objects packed-refs
-+ do
-+ rm -rf .git/$d &&
-+ ln -s ../../.git/$d .git/$d
-+ done
-+ ) &&
-+ git --git-dir subdir/.git/ branch rename-src &&
++ rm -rfv subdir/refs subdir/objects subdir/packed-refs &&
++ ln -s ../.git/refs subdir/refs &&
++ ln -s ../.git/objects subdir/objects &&
++ ln -s ../.git/packed-refs subdir/packed-refs &&
++
++ git -C subdir rev-parse --absolute-git-dir >subdir.dir &&
++ git rev-parse --absolute-git-dir >our.dir &&
++ ! test_cmp subdir.dir our.dir &&
++
++ git -C subdir log &&
++ git -C subdir branch rename-src &&
+ git rev-parse rename-src >expect &&
-+ git --git-dir subdir/.git/ branch -m rename-src rename-dest &&
++ git -C subdir branch -m rename-src rename-dest &&
+ git rev-parse rename-dest >actual &&
+ test_cmp expect actual &&
+ git branch -D rename-dest
2: 46641111885 = 2: f6d784b4979 refs API: add a version of refs_resolve_ref_unsafe() with "errno"
3: a1a80715ffe = 3: 8932b109087 refs API: make refs_read_raw_ref() not set errno
4: 758c761abcf ! 4: 988bf62b3f2 refs API: make parse_loose_ref_contents() not set errno
@@ refs/refs-internal.h: struct ref_store {
/*
* Fill in the generic part of refs and add it to our collection of
-
- ## t/t3200-branch.sh ##
-@@ t/t3200-branch.sh: test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
- ) &&
- git --git-dir subdir/.git/ branch rename-src &&
- git rev-parse rename-src >expect &&
-+ # Tests a BUG() assertion in files_read_raw_ref()
- git --git-dir subdir/.git/ branch -m rename-src rename-dest &&
- git rev-parse rename-dest >actual &&
- test_cmp expect actual &&
5: cdf17fa34a4 = 5: 8073f93a904 refs API: make refs_rename_ref_available() static
-: ----------- > 6: d3242f5f687 reflog tests: add --updateref tests
6: 3162bf28505 ! 7: 7db3b446632 refs/files: remove "name exist?" check in lock_ref_oid_basic()
@@ Commit message
already, or something we're about to discover by trying to lock the
ref with raceproof_create_file().
+ The one exception is the caller in files_reflog_expire(), who passes
+ us a "type" to find out if the reference is a symref or not. We can
+ move the that logic over to that caller, which can now defer its
+ discovery of whether or not the ref is a symref until it's needed. In
+ the preceding commit an exhaustive regression test was added for that
+ case in a new test in "t1417-reflog-updateref.sh".
+
The improved diagnostics here were added in
5b2d8d6f218 (lock_ref_sha1_basic(): improve diagnostics for ref D/F
conflicts, 2015-05-11), and then much of the surrounding code went
@@ Commit message
reflog we're expiring exists.
See ae35e16cd43 (reflog expire: don't lock reflogs using previously
- seen OID, 2021-08-23) for the current state of that code.
+ seen OID, 2021-08-23) for the current state of that code, and
+ 5e6f003ca8a (reflog_expire(): ignore --updateref for symbolic
+ references, 2015-03-03) for the code we'd break if we only did a
+ "update = !!ref" here, which is covered by the aforementioned
+ regression test in "t1417-reflog-updateref.sh".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## refs/files-backend.c ##
+@@ refs/files-backend.c: static int create_reflock(const char *path, void *cb)
+ * Locks a ref returning the lock on success and NULL on failure.
+ */
+ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
+- const char *refname, int *type,
++ const char *refname,
+ struct strbuf *err)
+ {
+ struct strbuf ref_file = STRBUF_INIT;
@@ refs/files-backend.c: static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
CALLOC_ARRAY(lock, 1);
@@ refs/files-backend.c: static int commit_ref_update(struct files_ref_store *refs,
*/
static int refs_rename_ref_available(struct ref_store *refs,
const char *old_refname,
+@@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_store,
+
+ logmoved = log;
+
+- lock = lock_ref_oid_basic(refs, newrefname, NULL, &err);
++ lock = lock_ref_oid_basic(refs, newrefname, &err);
+ if (!lock) {
+ if (copy)
+ error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+@@ refs/files-backend.c: static int files_copy_or_rename_ref(struct ref_store *ref_store,
+ goto out;
+
+ rollback:
+- lock = lock_ref_oid_basic(refs, oldrefname, NULL, &err);
++ lock = lock_ref_oid_basic(refs, oldrefname, &err);
+ if (!lock) {
+ error("unable to lock %s for rollback: %s", oldrefname, err.buf);
+ strbuf_release(&err);
+@@ refs/files-backend.c: static int files_create_symref(struct ref_store *ref_store,
+ struct ref_lock *lock;
+ int ret;
+
+- lock = lock_ref_oid_basic(refs, refname, NULL, &err);
++ lock = lock_ref_oid_basic(refs, refname, &err);
+ if (!lock) {
+ error("%s", err.buf);
+ strbuf_release(&err);
+@@ refs/files-backend.c: static int files_reflog_expire(struct ref_store *ref_store,
+ struct strbuf log_file_sb = STRBUF_INIT;
+ char *log_file;
+ int status = 0;
+- int type;
+ struct strbuf err = STRBUF_INIT;
+ const struct object_id *oid;
+
+@@ refs/files-backend.c: static int files_reflog_expire(struct ref_store *ref_store,
+ * reference itself, plus we might need to update the
+ * reference if --updateref was specified:
+ */
+- lock = lock_ref_oid_basic(refs, refname, &type, &err);
++ lock = lock_ref_oid_basic(refs, refname, &err);
+ if (!lock) {
+ error("cannot lock ref '%s': %s", refname, err.buf);
+ strbuf_release(&err);
+@@ refs/files-backend.c: static int files_reflog_expire(struct ref_store *ref_store,
+ * a reference if there are no remaining reflog
+ * entries.
+ */
+- int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
+- !(type & REF_ISSYMREF) &&
+- !is_null_oid(&cb.last_kept_oid);
++ int update = 0;
++
++ if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
++ !is_null_oid(&cb.last_kept_oid)) {
++ int ignore_errno;
++ int type;
++ const char *ref;
++
++ ref = refs_werrres_ref_unsafe(&refs->base, refname,
++ RESOLVE_REF_NO_RECURSE,
++ NULL, &type,
++ &ignore_errno);
++ update = !!(ref && !(type & REF_ISSYMREF));
++ }
+
+ if (close_lock_file_gently(&reflog_lock)) {
+ status |= error("couldn't write %s: %s", log_file,
7: 288237b1900 = 8: b2532614430 refs API: remove refs_read_ref_full() wrapper
8: acb484ea547 = 9: 65beb51ae4e refs API: make resolve_gitlink_ref() not set errno
9: 4be84c9bf53 = 10: c99c59dcc57 refs API: make loose_fill_ref_dir() not set errno
10: 8753210f9cc = 11: 22883846a6a refs API: make files_copy_or_rename_ref() et al not set errno
11: 9fe85926140 = 12: b90e65abd7d refs API: ignore errno in worktree.c's add_head_info()
12: 8d87db98041 = 13: 38f5f54938f refs API: ignore errno in worktree.c's find_shared_symref()
13: 954633bcbb2 = 14: 1289d78e4f3 refs tests: ignore ignore errno in test-ref-store helper
14: fbbc08d3ebd = 15: 152a831b8be refs API: make refs_resolve_refdup() not set errno
15: 4b2a4dbe7d5 = 16: c4e87181121 refs API: make refs_ref_exists() not set errno
16: 888b1884c29 = 17: cae56498b43 refs API: make resolve_ref_unsafe() not set errno
17: e2885f13c9b = 18: 146b030a142 refs API: make expand_ref() & repo_dwim_log() not set errno
18: df50373a272 = 19: 3d630872aff refs API: don't expose "errno" in run_transaction_hook()
19: 4c80b05bf1d = 20: 56f855d32b7 refs API: post-migration API renaming [1/2]
20: 54b18e3a719 ! 21: 0135fb5949f refs API: post-migration API renaming [2/2]
@@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *ref
referent.buf, 0,
&lock->old_oid, NULL,
&ignore_errno)) {
+@@ refs/files-backend.c: static int files_reflog_expire(struct ref_store *ref_store,
+ int type;
+ const char *ref;
+
+- ref = refs_werrres_ref_unsafe(&refs->base, refname,
++ ref = refs_resolve_ref_unsafe(&refs->base, refname,
+ RESOLVE_REF_NO_RECURSE,
+ NULL, &type,
+ &ignore_errno);
## sequencer.c ##
@@ sequencer.c: void print_commit_summary(struct repository *r,
--
2.33.1.1338.g20da966911a
next prev parent reply other threads:[~2021-10-16 9:40 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-14 0:06 [PATCH 00/20] refs: stop having the API set "errno" Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 01/20] branch tests: test for errno propagating on failing read Ævar Arnfjörð Bjarmason
2021-10-14 1:57 ` Eric Sunshine
2021-10-14 0:06 ` [PATCH 02/20] refs API: add a version of refs_resolve_ref_unsafe() with "errno" Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 03/20] refs API: make refs_read_raw_ref() not set errno Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 04/20] refs API: make parse_loose_ref_contents() " Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 05/20] refs API: make refs_rename_ref_available() static Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 06/20] refs/files: remove "name exist?" check in lock_ref_oid_basic() Ævar Arnfjörð Bjarmason
2021-10-14 20:53 ` Junio C Hamano
2021-10-14 0:06 ` [PATCH 07/20] refs API: remove refs_read_ref_full() wrapper Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 08/20] refs API: make resolve_gitlink_ref() not set errno Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 09/20] refs API: make loose_fill_ref_dir() " Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 10/20] refs API: make files_copy_or_rename_ref() et al " Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 11/20] refs API: ignore errno in worktree.c's add_head_info() Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 12/20] refs API: ignore errno in worktree.c's find_shared_symref() Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 13/20] refs tests: ignore ignore errno in test-ref-store helper Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 14/20] refs API: make refs_resolve_refdup() not set errno Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 15/20] refs API: make refs_ref_exists() " Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 16/20] refs API: make resolve_ref_unsafe() " Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 17/20] refs API: make expand_ref() & repo_dwim_log() " Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 18/20] refs API: don't expose "errno" in run_transaction_hook() Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 19/20] refs API: post-migration API renaming [1/2] Ævar Arnfjörð Bjarmason
2021-10-14 0:06 ` [PATCH 20/20] refs API: post-migration API renaming [2/2] Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` Ævar Arnfjörð Bjarmason [this message]
2021-10-16 9:39 ` [PATCH v2 01/21] branch tests: test for errno propagating on failing read Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 02/21] refs API: add a version of refs_resolve_ref_unsafe() with "errno" Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 03/21] refs API: make refs_read_raw_ref() not set errno Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 04/21] refs API: make parse_loose_ref_contents() " Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 05/21] refs API: make refs_rename_ref_available() static Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 06/21] reflog tests: add --updateref tests Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 07/21] refs/files: remove "name exist?" check in lock_ref_oid_basic() Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 08/21] refs API: remove refs_read_ref_full() wrapper Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 09/21] refs API: make resolve_gitlink_ref() not set errno Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 10/21] refs API: make loose_fill_ref_dir() " Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 11/21] refs API: make files_copy_or_rename_ref() et al " Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 12/21] refs API: ignore errno in worktree.c's add_head_info() Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 13/21] refs API: ignore errno in worktree.c's find_shared_symref() Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 14/21] refs tests: ignore ignore errno in test-ref-store helper Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 15/21] refs API: make refs_resolve_refdup() not set errno Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 16/21] refs API: make refs_ref_exists() " Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 17/21] refs API: make resolve_ref_unsafe() " Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 18/21] refs API: make expand_ref() & repo_dwim_log() " Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 19/21] refs API: don't expose "errno" in run_transaction_hook() Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 20/21] refs API: post-migration API renaming [1/2] Ævar Arnfjörð Bjarmason
2021-10-16 9:39 ` [PATCH v2 21/21] refs API: post-migration API renaming [2/2] Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover-v2-00.21-00000000000-20211016T093845Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hanwen@google.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).