* [PATCH v2 9/9] t: mark several tests that assume the files backend with REFFILES
From: Patrick Steinhardt @ 2023-10-24 14:05 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 8927 bytes --]
Add the REFFILES prerequisite to several tests that assume we're using
the files backend. There are various reasons why we cannot easily
convert those tests to be backend-independent, where the most common
one is that we have no way to write corrupt references into the refdb
via our tooling. We may at a later point in time grow the tooling to
make this possible, but for now we just mark these tests as requiring
the files backend.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 8 ++++----
t/t1450-fsck.sh | 6 +++---
t/t2011-checkout-invalid-head.sh | 16 ++++++++--------
t/t3200-branch.sh | 8 ++++----
t/t3400-rebase.sh | 2 +-
t/t5605-clone-local.sh | 2 +-
6 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b7d1e5deede..70a760ba378 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -236,7 +236,7 @@ test_expect_success 'update-ref --no-deref -d can delete self-reference' '
test_must_fail git show-ref --verify -q refs/heads/self
'
-test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '
+test_expect_success REFFILES 'update-ref --no-deref -d can delete reference to bad ref' '
>.git/refs/heads/bad &&
test_when_finished "rm -f .git/refs/heads/bad" &&
git symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&
@@ -288,7 +288,7 @@ test_expect_success "set $m (logged by touch)" '
test $A = $(git show-ref -s --verify $m)
'
-test_expect_success 'empty directory removal' '
+test_expect_success REFFILES 'empty directory removal' '
git branch d1/d2/r1 HEAD &&
git branch d1/r2 HEAD &&
test_path_is_file .git/refs/heads/d1/d2/r1 &&
@@ -300,7 +300,7 @@ test_expect_success 'empty directory removal' '
test_path_is_file .git/logs/refs/heads/d1/r2
'
-test_expect_success 'symref empty directory removal' '
+test_expect_success REFFILES 'symref empty directory removal' '
git branch e1/e2/r1 HEAD &&
git branch e1/r2 HEAD &&
git checkout e1/e2/r1 &&
@@ -1639,7 +1639,7 @@ test_expect_success PIPE 'transaction flushes status updates' '
test_cmp expected actual
'
-test_expect_success 'directory not created deleting packed ref' '
+test_expect_success REFFILES 'directory not created deleting packed ref' '
git branch d1/d2/r1 HEAD &&
git pack-refs --all &&
test_path_is_missing .git/refs/heads/d1/d2 &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index a6af550867c..50b15bd7fc0 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -123,7 +123,7 @@ test_expect_success 'branch pointing to non-commit' '
test_i18ngrep "not a commit" out
'
-test_expect_success 'HEAD link pointing at a funny object' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object' '
test_when_finished "git update-ref HEAD $orig_head" &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
@@ -139,7 +139,7 @@ test_expect_success 'HEAD link pointing at a funny place' '
test_i18ngrep "HEAD points to something strange" out
'
-test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object (from different wt)' '
test_when_finished "git update-ref HEAD $orig_head" &&
test_when_finished "git worktree remove -f wt" &&
git worktree add wt &&
@@ -149,7 +149,7 @@ test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
test_i18ngrep "main-worktree/HEAD: detached HEAD points" out
'
-test_expect_success 'other worktree HEAD link pointing at a funny object' '
+test_expect_success REFFILES 'other worktree HEAD link pointing at a funny object' '
test_when_finished "git worktree remove -f other" &&
git worktree add other &&
echo $ZERO_OID >.git/worktrees/other/HEAD &&
diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh
index d9997e7b6b4..3c8135831b8 100755
--- a/t/t2011-checkout-invalid-head.sh
+++ b/t/t2011-checkout-invalid-head.sh
@@ -18,18 +18,18 @@ test_expect_success 'checkout should not start branch from a tree' '
test_must_fail git checkout -b newbranch main^{tree}
'
-test_expect_success 'checkout main from invalid HEAD' '
+test_expect_success REFFILES 'checkout main from invalid HEAD' '
echo $ZERO_OID >.git/HEAD &&
git checkout main --
'
-test_expect_success 'checkout notices failure to lock HEAD' '
+test_expect_success REFFILES 'checkout notices failure to lock HEAD' '
test_when_finished "rm -f .git/HEAD.lock" &&
>.git/HEAD.lock &&
test_must_fail git checkout -b other
'
-test_expect_success 'create ref directory/file conflict scenario' '
+test_expect_success REFFILES 'create ref directory/file conflict scenario' '
git update-ref refs/heads/outer/inner main &&
# do not rely on symbolic-ref to get a known state,
@@ -39,26 +39,26 @@ test_expect_success 'create ref directory/file conflict scenario' '
}
'
-test_expect_success 'checkout away from d/f HEAD (unpacked, to branch)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to branch)' '
reset_to_df &&
git checkout main
'
-test_expect_success 'checkout away from d/f HEAD (unpacked, to detached)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to detached)' '
reset_to_df &&
git checkout --detach main
'
-test_expect_success 'pack refs' '
+test_expect_success REFFILES 'pack refs' '
git pack-refs --all --prune
'
-test_expect_success 'checkout away from d/f HEAD (packed, to branch)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to branch)' '
reset_to_df &&
git checkout main
'
-test_expect_success 'checkout away from d/f HEAD (packed, to detached)' '
+test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to detached)' '
reset_to_df &&
git checkout --detach main
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index a38601dbbb7..8d82f9bc52a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -28,7 +28,7 @@ test_expect_success 'git branch --help should not have created a bogus branch' '
test_path_is_missing .git/refs/heads/--help
'
-test_expect_success 'branch -h in broken repository' '
+test_expect_success REFFILES 'branch -h in broken repository' '
mkdir broken &&
(
cd broken &&
@@ -242,7 +242,7 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
git worktree prune
'
-test_expect_success 'git branch -M fails if updating any linked working tree fails' '
+test_expect_success REFFILES 'git branch -M fails if updating any linked working tree fails' '
git worktree add -b baz bazdir1 &&
git worktree add -f bazdir2 baz &&
touch .git/worktrees/bazdir1/HEAD.lock &&
@@ -833,14 +833,14 @@ test_expect_success 'renaming a symref is not allowed' '
test_path_is_missing .git/refs/heads/new-topic
'
-test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
+test_expect_success SYMLINKS,REFFILES 'git branch -m u v should fail when the reflog for u is a symlink' '
git branch --create-reflog u &&
mv .git/logs/refs/heads/u real-u &&
ln -s real-u .git/logs/refs/heads/u &&
test_must_fail git branch -m u v
'
-test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
+test_expect_success SYMLINKS,REFFILES 'git branch -m with symlinked .git/refs' '
test_when_finished "rm -rf subdir" &&
git init --bare subdir &&
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index d3df19a51f8..435943a0891 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -424,7 +424,7 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '
test_i18ngrep "already used by worktree at" err
'
-test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
+test_expect_success REFFILES,MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
git checkout main &&
mv .git/logs actual_logs &&
cmd //c "mklink /D .git\logs ..\actual_logs" &&
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index a26689de379..8ad6445eca7 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -157,7 +157,7 @@ test_expect_success 'cloning locally respects "-u" for fetching refs' '
test_must_fail git clone --bare -u false a should_not_work.git
'
-test_expect_success 'local clone from repo with corrupt refs fails gracefully' '
+test_expect_success REFFILES 'local clone from repo with corrupt refs fails gracefully' '
git init corrupt &&
test_commit -C corrupt one &&
echo a >corrupt/.git/refs/heads/topic &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 8/9] t7900: assert the absence of refs via git-for-each-ref(1)
From: Patrick Steinhardt @ 2023-10-24 14:05 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]
We're asserting that a prefetch of remotes via git-maintenance(1)
doesn't write any references in refs/remotes by validating that the
directory ".git/refs/remotes" is missing. This is quite roundabout: we
don't care about the directory existing, we care about the references
not existing, and the way these are stored is on the behest of the
reference database.
Convert the test to instead check via git-for-each-ref(1) whether any
remote reference exist.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t7900-maintenance.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index e56f5980dc4..cefecee732f 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -157,7 +157,8 @@ test_expect_success 'prefetch multiple remotes' '
fetchargs="--prefetch --prune --no-tags --no-write-fetch-head --recurse-submodules=no --quiet" &&
test_subcommand git fetch remote1 $fetchargs <run-prefetch.txt &&
test_subcommand git fetch remote2 $fetchargs <run-prefetch.txt &&
- test_path_is_missing .git/refs/remotes &&
+ git for-each-ref refs/remotes >actual &&
+ test_must_be_empty actual &&
git log prefetch/remotes/remote1/one &&
git log prefetch/remotes/remote2/two &&
git fetch --all &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 7/9] t7300: assert exact states of repo
From: Patrick Steinhardt @ 2023-10-24 14:05 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2913 bytes --]
Some of the tests in t7300 verify that git-clean(1) doesn't touch
repositories that are embedded into the main repository. This is done by
asserting a small set of substructures that are assumed to always exist,
like the "refs/", "objects/" or "HEAD". This has the downside that we
need to assume a specific repository structure that may be subject to
change when new backends for the refdb land. At the same time, we don't
thoroughly assert that git-clean(1) really didn't end up cleaning any
files in the repository either.
Convert the tests to instead assert that all files continue to exist
after git-clean(1) by comparing a file listing via find(1) before and
after executing clean. This makes our actual assertions stricter while
having to care less about the repository's actual on-disk format.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t7300-clean.sh | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 0ef7b784573..d7d9202f37f 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -517,8 +517,12 @@ test_expect_success 'nested (empty) git should be kept' '
git init empty_repo &&
mkdir to_clean &&
>to_clean/should_clean.this &&
+ # Note that we put the expect file in the .git directory so that it
+ # does not get cleaned.
+ find empty_repo | sort >.git/expect &&
git clean -f -d &&
- test_path_is_file empty_repo/.git/HEAD &&
+ find empty_repo | sort >actual &&
+ test_cmp .git/expect actual &&
test_path_is_missing to_clean
'
@@ -559,10 +563,10 @@ test_expect_success 'giving path in nested git work tree will NOT remove it' '
mkdir -p bar/baz &&
test_commit msg bar/baz/hello.world
) &&
+ find repo | sort >expect &&
git clean -f -d repo/bar/baz &&
- test_path_is_file repo/.git/HEAD &&
- test_path_is_dir repo/bar/ &&
- test_path_is_file repo/bar/baz/hello.world
+ find repo | sort >actual &&
+ test_cmp expect actual
'
test_expect_success 'giving path to nested .git will not remove it' '
@@ -573,10 +577,10 @@ test_expect_success 'giving path to nested .git will not remove it' '
git init &&
test_commit msg hello.world
) &&
+ find repo | sort >expect &&
git clean -f -d repo/.git &&
- test_path_is_file repo/.git/HEAD &&
- test_path_is_dir repo/.git/refs &&
- test_path_is_dir repo/.git/objects &&
+ find repo | sort >actual &&
+ test_cmp expect actual &&
test_path_is_dir untracked/
'
@@ -588,9 +592,10 @@ test_expect_success 'giving path to nested .git/ will NOT remove contents' '
git init &&
test_commit msg hello.world
) &&
+ find repo | sort >expect &&
git clean -f -d repo/.git/ &&
- test_path_is_dir repo/.git &&
- test_path_is_file repo/.git/HEAD &&
+ find repo | sort >actual &&
+ test_cmp expect actual &&
test_path_is_dir untracked/
'
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 6/9] t4207: delete replace references via git-update-ref(1)
From: Patrick Steinhardt @ 2023-10-24 14:05 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]
In t4207 we set up a set of replace objects via git-replace(1). Because
these references should not be impacting subsequent tests we also set up
some cleanup logic that deletes the replacement references via a call to
`rm -rf`. This reaches into the internal implementation details of the
reference backend and will thus break when we grow an alternative refdb
implementation.
Refactor the tests to delete the replacement refs via Git commands so
that we become independent of the actual refdb that's in use. As we
don't have a nice way to delete all replacements or all references in a
certain namespace, we opt for a combination of git-for-each-ref(1) and
git-update-ref(1)'s `--stdin` mode.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t4207-log-decoration-colors.sh | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index 21986a866df..73ea9e51550 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -70,8 +70,14 @@ ${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
cmp_filtered_decorations
'
+remove_replace_refs () {
+ git for-each-ref 'refs/replace*/**' --format='delete %(refname)' >in &&
+ git update-ref --stdin <in &&
+ rm in
+}
+
test_expect_success 'test coloring with replace-objects' '
- test_when_finished rm -rf .git/refs/replace* &&
+ test_when_finished remove_replace_refs &&
test_commit C &&
test_commit D &&
@@ -99,7 +105,7 @@ EOF
'
test_expect_success 'test coloring with grafted commit' '
- test_when_finished rm -rf .git/refs/replace* &&
+ test_when_finished remove_replace_refs &&
git replace --graft HEAD HEAD~2 &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 5/9] t1450: convert tests to remove worktrees via git-worktree(1)
From: Patrick Steinhardt @ 2023-10-24 14:05 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]
Some of our tests in t1450 create worktrees and then corrupt them.
As it is impossible to delete such worktrees via a normal call to `git
worktree remove`, we instead opt to remove them manually by calling
rm(1) instead.
This is ultimately unnecessary though as we can use the `-f` switch to
remove the worktree. Let's convert the tests to do so such that we don't
have to reach into internal implementation details of worktrees.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1450-fsck.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index a3c97b9c7fc..a6af550867c 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -141,7 +141,7 @@ test_expect_success 'HEAD link pointing at a funny place' '
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
test_when_finished "git update-ref HEAD $orig_head" &&
- test_when_finished "rm -rf .git/worktrees wt" &&
+ test_when_finished "git worktree remove -f wt" &&
git worktree add wt &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
@@ -150,7 +150,7 @@ test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
'
test_expect_success 'other worktree HEAD link pointing at a funny object' '
- test_when_finished "rm -rf .git/worktrees other" &&
+ test_when_finished "git worktree remove -f other" &&
git worktree add other &&
echo $ZERO_OID >.git/worktrees/other/HEAD &&
test_must_fail git fsck 2>out &&
@@ -158,7 +158,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny object' '
'
test_expect_success 'other worktree HEAD link pointing at missing object' '
- test_when_finished "rm -rf .git/worktrees other" &&
+ test_when_finished "git worktree remove -f other" &&
git worktree add other &&
object_id=$(echo "Contents missing from repo" | git hash-object --stdin) &&
test-tool -C other ref-store main update-ref msg HEAD $object_id "" REF_NO_DEREF,REF_SKIP_OID_VERIFICATION &&
@@ -167,7 +167,7 @@ test_expect_success 'other worktree HEAD link pointing at missing object' '
'
test_expect_success 'other worktree HEAD link pointing at a funny place' '
- test_when_finished "rm -rf .git/worktrees other" &&
+ test_when_finished "git worktree remove -f other" &&
git worktree add other &&
git -C other symbolic-ref HEAD refs/funny/place &&
test_must_fail git fsck 2>out &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 4/9] t: convert tests to not access reflog via the filesystem
From: Patrick Steinhardt @ 2023-10-24 14:05 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 7139 bytes --]
Some of our tests reach directly into the filesystem in order to both
read or modify the reflog, which will break once we have a second
reference backend in our codebase that stores reflogs differently.
Refactor these tests to either use git-reflog(1) or the ref-store test
helper. Note that the refactoring to use git-reflog(1) also requires us
to adapt our expectations in some cases where we previously verified the
exact on-disk log entries. This seems like an acceptable tradeoff though
to ensure that different backends have the same user-visible behaviour
as any user would typically use git-reflog(1) anyway to access the logs.
Any backend-specific verification of the written on-disk format should
be implemented in a separate, backend-specific test.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 17 +++++++++++------
t/t3200-branch.sh | 24 ++++++++++++------------
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 51a8d0bba98..b7d1e5deede 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -90,7 +90,8 @@ test_expect_success "deleting current branch adds message to HEAD's log" '
git symbolic-ref HEAD $m &&
git update-ref -m delete-$m -d $m &&
test_must_fail git show-ref --verify -q $m &&
- grep "delete-$m$" .git/logs/HEAD
+ test-tool ref-store main for-each-reflog-ent HEAD >actual &&
+ grep "delete-$m$" actual
'
test_expect_success "deleting by HEAD adds message to HEAD's log" '
@@ -99,7 +100,8 @@ test_expect_success "deleting by HEAD adds message to HEAD's log" '
git symbolic-ref HEAD $m &&
git update-ref -m delete-by-head -d HEAD &&
test_must_fail git show-ref --verify -q $m &&
- grep "delete-by-head$" .git/logs/HEAD
+ test-tool ref-store main for-each-reflog-ent HEAD >actual &&
+ grep "delete-by-head$" actual
'
test_expect_success 'update-ref does not create reflogs by default' '
@@ -130,7 +132,7 @@ test_expect_success 'creates no reflog in bare repository' '
test_expect_success 'core.logAllRefUpdates=true creates reflog in bare repository' '
test_when_finished "git -C $bare config --unset core.logAllRefUpdates && \
- rm $bare/logs/$m" &&
+ test-tool ref-store main delete-reflog $m" &&
git -C $bare config core.logAllRefUpdates true &&
git -C $bare update-ref $m $bareB &&
git -C $bare rev-parse $bareB >expect &&
@@ -263,7 +265,10 @@ test_expect_success "(not) changed .git/$m" '
! test $B = $(git show-ref -s --verify $m)
'
-rm -f .git/logs/refs/heads/main
+test_expect_success "clean up reflog" '
+ test-tool ref-store main delete-reflog $m
+'
+
test_expect_success "create $m (logged by touch)" '
test_config core.logAllRefUpdates false &&
GIT_COMMITTER_DATE="2005-05-26 23:30" \
@@ -316,7 +321,7 @@ $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000 Switch
$B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150860 +0000
EOF
test_expect_success "verifying $m's log (logged by touch)" '
- test_when_finished "git update-ref -d $m && rm -rf .git/logs actual expect" &&
+ test_when_finished "git update-ref -d $m && git reflog expire --expire=all --all && rm -rf actual expect" &&
test-tool ref-store main for-each-reflog-ent $m >actual &&
test_cmp actual expect
'
@@ -346,7 +351,7 @@ $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 +0000 Switch
$B $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150980 +0000
EOF
test_expect_success "verifying $m's log (logged by config)" '
- test_when_finished "git update-ref -d $m && rm -rf .git/logs actual expect" &&
+ test_when_finished "git update-ref -d $m && git reflog expire --expire=all --all && rm -rf actual expect" &&
test-tool ref-store main for-each-reflog-ent $m >actual &&
test_cmp actual expect
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 7d9393f190d..a38601dbbb7 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -73,14 +73,14 @@ test_expect_success 'git branch HEAD should fail' '
'
cat >expect <<EOF
-$ZERO_OID $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from main
+$HEAD refs/heads/d/e/f@{0}: branch: Created from main
EOF
test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
test_path_is_file .git/refs/heads/d/e/f &&
- test_path_is_file .git/logs/refs/heads/d/e/f &&
- test_cmp expect .git/logs/refs/heads/d/e/f
+ git reflog show --no-abbrev-commit refs/heads/d/e/f >actual &&
+ test_cmp expect actual
'
test_expect_success 'git branch -d d/e/f should delete a branch and a log' '
@@ -200,10 +200,9 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
test $(git rev-parse --abbrev-ref HEAD) = bam
'
-test_expect_success 'git branch -M baz bam should add entries to .git/logs/HEAD' '
- msg="Branch: renamed refs/heads/baz to refs/heads/bam" &&
- grep " $ZERO_OID.*$msg$" .git/logs/HEAD &&
- grep "^$ZERO_OID.*$msg$" .git/logs/HEAD
+test_expect_success 'git branch -M baz bam should add entries to HEAD reflog' '
+ git reflog show HEAD >actual &&
+ grep "HEAD@{0}: Branch: renamed refs/heads/baz to refs/heads/bam" actual
'
test_expect_success 'git branch -M should leave orphaned HEAD alone' '
@@ -225,7 +224,7 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
test_expect_success 'resulting reflog can be shown by log -g' '
oid=$(git rev-parse HEAD) &&
cat >expect <<-EOF &&
- HEAD@{0} $oid $msg
+ HEAD@{0} $oid Branch: renamed refs/heads/baz to refs/heads/bam
HEAD@{2} $oid checkout: moving from foo to baz
EOF
git log -g --format="%gd %H %gs" -2 HEAD >actual &&
@@ -699,7 +698,8 @@ test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out'
test_expect_success 'git branch -C c1 c2 should never touch HEAD' '
msg="Branch: copied refs/heads/c1 to refs/heads/c2" &&
- ! grep "$msg$" .git/logs/HEAD
+ git reflog HEAD >actual &&
+ ! grep "$msg$" actual
'
test_expect_success 'git branch -C main should work when main is checked out' '
@@ -1140,14 +1140,14 @@ test_expect_success '--set-upstream-to notices an error to set branch as own ups
# Keep this test last, as it changes the current branch
cat >expect <<EOF
-$ZERO_OID $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from main
+$HEAD refs/heads/g/h/i@{0}: branch: Created from main
EOF
test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git checkout -b g/h/i -l main &&
test_path_is_file .git/refs/heads/g/h/i &&
- test_path_is_file .git/logs/refs/heads/g/h/i &&
- test_cmp expect .git/logs/refs/heads/g/h/i
+ git reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&
+ test_cmp expect actual
'
test_expect_success 'checkout -b makes reflog by default' '
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 3/9] t: convert tests to not access symrefs via the filesystem
From: Patrick Steinhardt @ 2023-10-24 14:05 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 13728 bytes --]
Some of our tests access symbolic references via the filesystem
directly. While this works with the current files reference backend, it
this will break once we have a second reference backend in our codebase.
Refactor these tests to instead use git-symbolic-ref(1) or our
`ref-store` test tool. The latter is required in some cases where safety
checks of git-symbolic-ref(1) would otherwise reject writing a symbolic
reference.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 8 ++++----
t/t1430-bad-ref-name.sh | 12 ++++++------
t/t1450-fsck.sh | 4 ++--
t/t3200-branch.sh | 9 ++++++---
t/t4013-diff-various.sh | 2 +-
t/t4202-log.sh | 2 +-
t/t5605-clone-local.sh | 2 +-
t/t5702-protocol-v2.sh | 24 ++++++++++++++++++------
t/t9133-git-svn-nested-git-repo.sh | 2 +-
9 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 91cc6dff724..51a8d0bba98 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -221,15 +221,15 @@ test_expect_success 'delete symref without dereference when the referred ref is
test_expect_success 'update-ref -d is not confused by self-reference' '
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
- test_path_is_file .git/refs/heads/self
+ git symbolic-ref --no-recurse refs/heads/self
'
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
test_must_fail git show-ref --verify -q refs/heads/self
'
@@ -239,7 +239,7 @@ test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '
test_when_finished "rm -f .git/refs/heads/bad" &&
git symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&
test_when_finished "git update-ref -d refs/heads/ref-to-bad" &&
- test_path_is_file .git/refs/heads/ref-to-bad &&
+ git symbolic-ref --no-recurse refs/heads/ref-to-bad &&
git update-ref --no-deref -d refs/heads/ref-to-bad &&
test_must_fail git show-ref --verify -q refs/heads/ref-to-bad
'
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index ff1c967d550..7d2dfc97256 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -164,9 +164,9 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '
test_expect_success 'for-each-ref emits warnings for broken names' '
test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git for-each-ref >output 2>error &&
! grep -e "broken\.\.\.ref" output &&
@@ -252,7 +252,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
'
test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
@@ -261,7 +261,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name
'
test_expect_success 'branch -d can delete symref with broken name' '
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git branch -d broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
@@ -270,7 +270,7 @@ test_expect_success 'branch -d can delete symref with broken name' '
'
test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
@@ -279,7 +279,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
'
test_expect_success 'branch -d can delete dangling symref with broken name' '
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git branch -d broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index f55b539b083..a3c97b9c7fc 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -133,7 +133,7 @@ test_expect_success 'HEAD link pointing at a funny object' '
test_expect_success 'HEAD link pointing at a funny place' '
test_when_finished "git update-ref --no-deref HEAD $orig_head" &&
- echo "ref: refs/funny/place" >.git/HEAD &&
+ test-tool ref-store main create-symref HEAD refs/funny/place &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
test_i18ngrep "HEAD points to something strange" out
@@ -169,7 +169,7 @@ test_expect_success 'other worktree HEAD link pointing at missing object' '
test_expect_success 'other worktree HEAD link pointing at a funny place' '
test_when_finished "rm -rf .git/worktrees other" &&
git worktree add other &&
- echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
+ git -C other symbolic-ref HEAD refs/funny/place &&
test_must_fail git fsck 2>out &&
test_i18ngrep "worktrees/other/HEAD points to something strange" out
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 080e4f24a6e..7d9393f190d 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -212,10 +212,13 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
cd orphan &&
test_commit initial &&
git checkout --orphan lonely &&
- grep lonely .git/HEAD &&
+ git symbolic-ref HEAD >expect &&
+ echo refs/heads/lonely >actual &&
+ test_cmp expect actual &&
test_path_is_missing .git/refs/head/lonely &&
git branch -M main mistress &&
- grep lonely .git/HEAD
+ git symbolic-ref HEAD >expect &&
+ test_cmp expect actual
)
'
@@ -806,7 +809,7 @@ test_expect_success 'deleting a symref' '
test_expect_success 'deleting a dangling symref' '
git symbolic-ref refs/heads/dangling-symref nowhere &&
- test_path_is_file .git/refs/heads/dangling-symref &&
+ git symbolic-ref --no-recurse refs/heads/dangling-symref &&
echo "Deleted branch dangling-symref (was nowhere)." >expect &&
git branch -d dangling-symref >actual &&
test_path_is_missing .git/refs/heads/dangling-symref &&
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759..5abbea36b39 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -514,7 +514,7 @@ test_expect_success 'log -S requires an argument' '
'
test_expect_success 'diff --cached on unborn branch' '
- echo ref: refs/heads/unborn >.git/HEAD &&
+ git symbolic-ref HEAD refs/heads/unborn &&
git diff --cached >result &&
process_diffs result >actual &&
process_diffs "$TEST_DIRECTORY/t4013/diff.diff_--cached" >expected &&
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index af4a123cd22..57b298a4e22 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2265,7 +2265,7 @@ test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
test_expect_success REFFILES 'log diagnoses bogus HEAD symref' '
git init empty &&
- echo "ref: refs/heads/invalid.lock" > empty/.git/HEAD &&
+ test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep broken stderr &&
test_must_fail git -C empty log --default totally-bogus 2>stderr &&
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 1d7b1abda1a..a26689de379 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -65,7 +65,7 @@ test_expect_success 'Even without -l, local will make a hardlink' '
'
test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
- echo "ref: refs/heads/nonexistent" > a.git/HEAD &&
+ git -C a.git symbolic-ref HEAD refs/heads/nonexistent &&
git clone a d &&
(cd d &&
git fetch &&
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 6af5c2062fd..dcc4cd95fe7 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -221,7 +221,9 @@ test_expect_success 'clone of empty repo propagates name of default branch' '
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
git -c init.defaultBranch=main -c protocol.version=2 \
clone "file://$(pwd)/file_empty_parent" file_empty_child &&
- grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
+ echo refs/heads/mydefaultbranch >expect &&
+ git -C file_empty_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success '...but not if explicitly forbidden by config' '
@@ -234,7 +236,9 @@ test_expect_success '...but not if explicitly forbidden by config' '
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= \
git -c init.defaultBranch=main -c protocol.version=2 \
clone "file://$(pwd)/file_empty_parent" file_empty_child &&
- ! grep "refs/heads/mydefaultbranch" file_empty_child/.git/HEAD
+ echo refs/heads/main >expect &&
+ git -C file_empty_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success 'bare clone propagates empty default branch' '
@@ -247,7 +251,9 @@ test_expect_success 'bare clone propagates empty default branch' '
git -c init.defaultBranch=main -c protocol.version=2 \
clone --bare \
"file://$(pwd)/file_empty_parent" file_empty_child.git &&
- grep "refs/heads/mydefaultbranch" file_empty_child.git/HEAD
+ echo "refs/heads/mydefaultbranch" >expect &&
+ git -C file_empty_child.git symbolic-ref HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success 'clone propagates unborn HEAD from non-empty repo' '
@@ -265,7 +271,9 @@ test_expect_success 'clone propagates unborn HEAD from non-empty repo' '
git -c init.defaultBranch=main -c protocol.version=2 \
clone "file://$(pwd)/file_unborn_parent" \
file_unborn_child 2>stderr &&
- grep "refs/heads/mydefaultbranch" file_unborn_child/.git/HEAD &&
+ echo "refs/heads/mydefaultbranch" >expect &&
+ git -C file_unborn_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
grep "warning: remote HEAD refers to nonexistent ref" stderr
'
@@ -295,7 +303,9 @@ test_expect_success 'bare clone propagates unborn HEAD from non-empty repo' '
git -c init.defaultBranch=main -c protocol.version=2 \
clone --bare "file://$(pwd)/file_unborn_parent" \
file_unborn_child.git 2>stderr &&
- grep "refs/heads/mydefaultbranch" file_unborn_child.git/HEAD &&
+ echo "refs/heads/mydefaultbranch" >expect &&
+ git -C file_unborn_child.git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
! grep "warning:" stderr
'
@@ -315,7 +325,9 @@ test_expect_success 'defaulted HEAD uses remote branch if available' '
git -c init.defaultBranch=branchwithstuff -c protocol.version=2 \
clone "file://$(pwd)/file_unborn_parent" \
file_unborn_child 2>stderr &&
- grep "refs/heads/branchwithstuff" file_unborn_child/.git/HEAD &&
+ echo "refs/heads/branchwithstuff" >expect &&
+ git -C file_unborn_child symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
test_path_is_file file_unborn_child/stuff.t &&
! grep "warning:" stderr
'
diff --git a/t/t9133-git-svn-nested-git-repo.sh b/t/t9133-git-svn-nested-git-repo.sh
index d8d536269cf..8ca24670acb 100755
--- a/t/t9133-git-svn-nested-git-repo.sh
+++ b/t/t9133-git-svn-nested-git-repo.sh
@@ -11,7 +11,7 @@ test_expect_success 'setup repo with a git repo inside it' '
(
cd s &&
git init &&
- test -f .git/HEAD &&
+ git symbolic-ref HEAD &&
> .git/a &&
echo a > a &&
svn_cmd add .git a &&
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 2/9] t: convert tests to not write references via the filesystem
From: Patrick Steinhardt @ 2023-10-24 14:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 9376 bytes --]
Some of our tests manually create, update or delete references by
writing the respective new values into the filesystem directly. While
this works with the current files reference backend, this will break
once we have a second reference backend implementation in our codebase.
Refactor these tests to instead use git-update-ref(1) or our `ref-store`
test tool. The latter is required in some cases where safety checks of
git-update-ref(1) would otherwise reject a reference update.
While at it, refactor some of the tests to schedule the cleanup command
via `test_when_finished` before modifying the repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1400-update-ref.sh | 17 ++++++++---------
t/t1450-fsck.sh | 26 +++++++++++++-------------
t/t3404-rebase-interactive.sh | 2 +-
t/t5526-fetch-submodules.sh | 2 +-
4 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 4d66cd7f4a1..91cc6dff724 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -9,8 +9,6 @@ test_description='Test git update-ref and basic ref logging'
Z=$ZERO_OID
m=refs/heads/main
-n_dir=refs/heads/gu
-n=$n_dir/fixes
outside=refs/foo
bare=bare-repo
@@ -62,10 +60,10 @@ test_expect_success "delete $m without oldvalue verification" '
test_must_fail git show-ref --verify -q $m
'
-test_expect_success "fail to create $n" '
- test_when_finished "rm -f .git/$n_dir" &&
- touch .git/$n_dir &&
- test_must_fail git update-ref $n $A
+test_expect_success "fail to create $n due to file/directory conflict" '
+ test_when_finished "git update-ref -d refs/heads/gu" &&
+ git update-ref refs/heads/gu $A &&
+ test_must_fail git update-ref refs/heads/gu/fixes $A
'
test_expect_success "create $m (by HEAD)" '
@@ -221,16 +219,16 @@ test_expect_success 'delete symref without dereference when the referred ref is
'
test_expect_success 'update-ref -d is not confused by self-reference' '
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
test_path_is_file .git/refs/heads/self
'
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
test_must_fail git show-ref --verify -q refs/heads/self
@@ -434,7 +432,8 @@ test_expect_success 'Query "main@{2005-05-28}" (past end of history)' '
test_i18ngrep -F "warning: log for ref $m unexpectedly ended on $ld" e
'
-rm -f .git/$m .git/logs/$m expect
+rm -f expect
+git update-ref -d $m
test_expect_success 'creating initial files' '
test_when_finished rm -f M &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 10a539158c4..f55b539b083 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -15,6 +15,7 @@ test_expect_success setup '
git config --unset i18n.commitencoding &&
git checkout HEAD^0 &&
test_commit B fileB two &&
+ orig_head=$(git rev-parse HEAD) &&
git tag -d A B &&
git reflog expire --expire=now --all
'
@@ -115,15 +116,15 @@ test_expect_success 'zlib corrupt loose object output ' '
'
test_expect_success 'branch pointing to non-commit' '
- git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
+ tree_oid=$(git rev-parse --verify HEAD^{tree}) &&
test_when_finished "git update-ref -d refs/heads/invalid" &&
+ test-tool ref-store main update-ref msg refs/heads/invalid $tree_oid $ZERO_OID REF_SKIP_OID_VERIFICATION &&
test_must_fail git fsck 2>out &&
test_i18ngrep "not a commit" out
'
test_expect_success 'HEAD link pointing at a funny object' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ -131,8 +132,7 @@ test_expect_success 'HEAD link pointing at a funny object' '
'
test_expect_success 'HEAD link pointing at a funny place' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
+ test_when_finished "git update-ref --no-deref HEAD $orig_head" &&
echo "ref: refs/funny/place" >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ -140,10 +140,9 @@ test_expect_success 'HEAD link pointing at a funny place' '
'
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
test_when_finished "rm -rf .git/worktrees wt" &&
git worktree add wt &&
- mv .git/HEAD .git/SAVED_HEAD &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail git -C wt fsck 2>out &&
@@ -161,7 +160,8 @@ test_expect_success 'other worktree HEAD link pointing at a funny object' '
test_expect_success 'other worktree HEAD link pointing at missing object' '
test_when_finished "rm -rf .git/worktrees other" &&
git worktree add other &&
- echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD &&
+ object_id=$(echo "Contents missing from repo" | git hash-object --stdin) &&
+ test-tool -C other ref-store main update-ref msg HEAD $object_id "" REF_NO_DEREF,REF_SKIP_OID_VERIFICATION &&
test_must_fail git fsck 2>out &&
test_i18ngrep "worktrees/other/HEAD: invalid sha1 pointer" out
'
@@ -391,7 +391,7 @@ test_expect_success 'tag pointing to nonexistent' '
tag=$(git hash-object -t tag -w --stdin <invalid-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/invalid &&
+ git update-ref refs/tags/invalid $tag &&
test_when_finished "git update-ref -d refs/tags/invalid" &&
test_must_fail git fsck --tags >out &&
test_i18ngrep "broken link" out
@@ -411,7 +411,7 @@ test_expect_success 'tag pointing to something else than its type' '
tag=$(git hash-object -t tag -w --stdin <wrong-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
test_must_fail git fsck --tags
'
@@ -428,7 +428,7 @@ test_expect_success 'tag with incorrect tag name & missing tagger' '
tag=$(git hash-object --literally -t tag -w --stdin <wrong-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
git fsck --tags 2>out &&
@@ -452,7 +452,7 @@ test_expect_success 'tag with bad tagger' '
tag=$(git hash-object --literally -t tag -w --stdin <wrong-tag) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
test_must_fail git fsck --tags 2>out &&
test_i18ngrep "error in tag .*: invalid author/committer" out
@@ -471,7 +471,7 @@ test_expect_success 'tag with NUL in header' '
tag=$(git hash-object --literally -t tag -w --stdin <tag-NUL-header) &&
test_when_finished "remove_object $tag" &&
- echo $tag >.git/refs/tags/wrong &&
+ git update-ref refs/tags/wrong $tag &&
test_when_finished "git update-ref -d refs/tags/wrong" &&
test_must_fail git fsck --tags 2>out &&
test_i18ngrep "error in tag $tag.*unterminated header: NUL at offset" out
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8ea2bf13026..d2a7a91f170 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -2160,7 +2160,7 @@ test_expect_success '--update-refs: check failed ref update' '
# recorded in the update-refs file. We will force-update the
# "second" ref, but "git branch -f" will not work because of
# the lock in the update-refs file.
- git rev-parse third >.git/refs/heads/second &&
+ git update-ref refs/heads/second third &&
test_must_fail git rebase --continue 2>err &&
grep "update_ref failed for ref '\''refs/heads/second'\''" err &&
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 26e933f93ae..7ab220fa313 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -771,7 +771,7 @@ test_expect_success 'fetching submodule into a broken repository' '
git -C dst fetch --recurse-submodules &&
# Break the receiving submodule
- rm -f dst/sub/.git/HEAD &&
+ test-tool -C dst/sub ref-store main delete-refs REF_NO_DEREF msg HEAD &&
# NOTE: without the fix the following tests will recurse forever!
# They should terminate with an error.
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 1/9] t: allow skipping expected object ID in `ref-store update-ref`
From: Patrick Steinhardt @ 2023-10-24 14:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1698156169.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]
We require the caller to pass both the old and new expected object ID to
our `test-tool ref-store update-ref` helper. When trying to update a
symbolic reference though it's impossible to specify the expected object
ID, which means that the test would instead have to force-update the
reference. This is currently impossible though.
Update the helper to optionally skip verification of the old object ID
in case the test passes in an empty old object ID as input.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/helper/test-ref-store.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 48552e6a9e0..702ec1f128a 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -298,16 +298,19 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv)
const char *new_sha1_buf = notnull(*argv++, "new-sha1");
const char *old_sha1_buf = notnull(*argv++, "old-sha1");
unsigned int flags = arg_flags(*argv++, "flags", transaction_flags);
- struct object_id old_oid;
+ struct object_id old_oid, *old_oid_ptr = NULL;
struct object_id new_oid;
- if (get_oid_hex(old_sha1_buf, &old_oid))
- die("cannot parse %s as %s", old_sha1_buf, the_hash_algo->name);
+ if (*old_sha1_buf) {
+ if (get_oid_hex(old_sha1_buf, &old_oid))
+ die("cannot parse %s as %s", old_sha1_buf, the_hash_algo->name);
+ old_oid_ptr = &old_oid;
+ }
if (get_oid_hex(new_sha1_buf, &new_oid))
die("cannot parse %s as %s", new_sha1_buf, the_hash_algo->name);
return refs_update_ref(refs, msg, refname,
- &new_oid, &old_oid,
+ &new_oid, old_oid_ptr,
flags, UPDATE_REFS_DIE_ON_ERR);
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v2 0/9] t: reduce direct disk access to data structures
From: Patrick Steinhardt @ 2023-10-24 14:04 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Eric Sunshine, Taylor Blau, Junio C Hamano
In-Reply-To: <cover.1697607222.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 19236 bytes --]
Hi,
this is the second version of my patch series that aims to reduce access
to on-disk data structures in favor of using plumbing tools where
possible.
Changes compared to v1:
- Patches 1, 3: I've dropped these patches that introduced and
started to use the test helper for reference existence. This has
been split out into a separate patch series now that instead
implements the logic as part of git-show-ref(1), see [1].
- Patch 4: I've made it more explicit that tests in t1450 are all
ran in detached HEAD mode via a new `orig_head` variable that is
set in the test setup. This variable is later used to reset HEAD
back to that original state.
- Patch 4, 5: I've reordered some of the logic such that we schedule
`test_when_finished` before doing the actual mutation of the repo.
- Patch 8: I've adopted the proposal of a `remove_replace_refs()`
helper function to clean up replace refs.
- Now comes with a base commit. Unbelievable.
Thanks for all the feedback so far!
Patrick
Patrick Steinhardt (9):
t: allow skipping expected object ID in `ref-store update-ref`
t: convert tests to not write references via the filesystem
t: convert tests to not access symrefs via the filesystem
t: convert tests to not access reflog via the filesystem
t1450: convert tests to remove worktrees via git-worktree(1)
t4207: delete replace references via git-update-ref(1)
t7300: assert exact states of repo
t7900: assert the absence of refs via git-for-each-ref(1)
t: mark several tests that assume the files backend with REFFILES
t/helper/test-ref-store.c | 11 ++++---
t/t1400-update-ref.sh | 50 ++++++++++++++++--------------
t/t1430-bad-ref-name.sh | 12 +++----
t/t1450-fsck.sh | 44 +++++++++++++-------------
t/t2011-checkout-invalid-head.sh | 16 +++++-----
t/t3200-branch.sh | 41 ++++++++++++------------
t/t3400-rebase.sh | 2 +-
t/t3404-rebase-interactive.sh | 2 +-
t/t4013-diff-various.sh | 2 +-
t/t4202-log.sh | 2 +-
t/t4207-log-decoration-colors.sh | 10 ++++--
t/t5526-fetch-submodules.sh | 2 +-
t/t5605-clone-local.sh | 4 +--
t/t5702-protocol-v2.sh | 24 ++++++++++----
t/t7300-clean.sh | 23 ++++++++------
t/t7900-maintenance.sh | 3 +-
t/t9133-git-svn-nested-git-repo.sh | 2 +-
17 files changed, 142 insertions(+), 108 deletions(-)
Range-diff against v1:
1: e947feb1c77 < -: ----------- t: add helpers to test for reference existence
2: 1f615d62f99 = 1: c868198f8c1 t: allow skipping expected object ID in `ref-store update-ref`
3: ac6a49c7c84 < -: ----------- t: convert tests to use helpers for reference existence
4: c79431c0bf1 ! 2: 4c0939d868e t: convert tests to not write references via the filesystem
@@ Commit message
test tool. The latter is required in some cases where safety checks of
git-update-ref(1) would otherwise reject a reference update.
+ While at it, refactor some of the tests to schedule the cleanup command
+ via `test_when_finished` before modifying the repository.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## t/t1400-update-ref.sh ##
@@ t/t1400-update-ref.sh: test_expect_success "delete $m without oldvalue verificat
test_expect_success "create $m (by HEAD)" '
@@ t/t1400-update-ref.sh: test_expect_success 'delete symref without dereference when the referred ref is
+ '
test_expect_success 'update-ref -d is not confused by self-reference' '
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
-+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
test_path_is_file .git/refs/heads/self
-@@ t/t1400-update-ref.sh: test_expect_success 'update-ref -d is not confused by self-reference' '
+ '
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
-+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
test_must_fail git show-ref --verify -q refs/heads/self
@@ t/t1400-update-ref.sh: test_expect_success 'Query "main@{2005-05-28}" (past end
'
-rm -f .git/$m .git/logs/$m expect
++rm -f expect
+git update-ref -d $m
test_expect_success 'creating initial files' '
test_when_finished rm -f M &&
## t/t1450-fsck.sh ##
+@@ t/t1450-fsck.sh: test_expect_success setup '
+ git config --unset i18n.commitencoding &&
+ git checkout HEAD^0 &&
+ test_commit B fileB two &&
++ orig_head=$(git rev-parse HEAD) &&
+ git tag -d A B &&
+ git reflog expire --expire=now --all
+ '
@@ t/t1450-fsck.sh: test_expect_success 'zlib corrupt loose object output ' '
'
test_expect_success 'branch pointing to non-commit' '
- git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
+ tree_oid=$(git rev-parse --verify HEAD^{tree}) &&
-+ test-tool ref-store main update-ref msg refs/heads/invalid $tree_oid $ZERO_OID REF_SKIP_OID_VERIFICATION &&
test_when_finished "git update-ref -d refs/heads/invalid" &&
++ test-tool ref-store main update-ref msg refs/heads/invalid $tree_oid $ZERO_OID REF_SKIP_OID_VERIFICATION &&
test_must_fail git fsck 2>out &&
test_i18ngrep "not a commit" out
'
@@ t/t1450-fsck.sh: test_expect_success 'zlib corrupt loose object output ' '
test_expect_success 'HEAD link pointing at a funny object' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
-+ saved_head=$(git rev-parse --verify HEAD) &&
-+ test_when_finished "git update-ref HEAD ${saved_head}" &&
++ test_when_finished "git update-ref HEAD $orig_head" &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny object' '
test_expect_success 'HEAD link pointing at a funny place' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
-+ saved_head=$(git rev-parse --verify HEAD) &&
-+ test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
++ test_when_finished "git update-ref --no-deref HEAD $orig_head" &&
echo "ref: refs/funny/place" >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny place' '
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
-+ saved_head=$(git rev-parse --verify HEAD) &&
-+ test_when_finished "git update-ref HEAD $saved_head" &&
++ test_when_finished "git update-ref HEAD $orig_head" &&
test_when_finished "rm -rf .git/worktrees wt" &&
git worktree add wt &&
- mv .git/HEAD .git/SAVED_HEAD &&
5: 1ac120368c6 ! 3: 048583ed2c3 t: convert tests to not access symrefs via the filesystem
@@ Commit message
## t/t1400-update-ref.sh ##
@@ t/t1400-update-ref.sh: test_expect_success 'delete symref without dereference when the referred ref is
test_expect_success 'update-ref -d is not confused by self-reference' '
- git symbolic-ref refs/heads/self refs/heads/self &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
+ git symbolic-ref refs/heads/self refs/heads/self &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
@@ t/t1400-update-ref.sh: test_expect_success 'delete symref without dereference wh
'
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
- git symbolic-ref refs/heads/self refs/heads/self &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
+ git symbolic-ref refs/heads/self refs/heads/self &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken na
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref with broken name
'
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref with broken name' '
'
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref with b
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
'
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
## t/t1450-fsck.sh ##
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny object' '
+
test_expect_success 'HEAD link pointing at a funny place' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
+ test_when_finished "git update-ref --no-deref HEAD $orig_head" &&
- echo "ref: refs/funny/place" >.git/HEAD &&
+ test-tool ref-store main create-symref HEAD refs/funny/place &&
# avoid corrupt/broken HEAD from interfering with repo discovery
@@ t/t3200-branch.sh: test_expect_success 'git branch -M should leave orphaned HEAD
+ git symbolic-ref HEAD >expect &&
+ echo refs/heads/lonely >actual &&
+ test_cmp expect actual &&
- test_ref_missing refs/head/lonely &&
+ test_path_is_missing .git/refs/head/lonely &&
git branch -M main mistress &&
- grep lonely .git/HEAD
+ git symbolic-ref HEAD >expect &&
@@ t/t3200-branch.sh: test_expect_success 'deleting a symref' '
+ git symbolic-ref --no-recurse refs/heads/dangling-symref &&
echo "Deleted branch dangling-symref (was nowhere)." >expect &&
git branch -d dangling-symref >actual &&
- test_ref_missing refs/heads/dangling-symref &&
+ test_path_is_missing .git/refs/heads/dangling-symref &&
## t/t4013-diff-various.sh ##
@@ t/t4013-diff-various.sh: test_expect_success 'log -S requires an argument' '
6: eaac658bbfd ! 4: 5e7937e7904 t: convert tests to not access reflog via the filesystem
@@ t/t3200-branch.sh: test_expect_success 'git branch HEAD should fail' '
test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
- test_ref_exists refs/heads/d/e/f &&
+ test_path_is_file .git/refs/heads/d/e/f &&
- test_path_is_file .git/logs/refs/heads/d/e/f &&
- test_cmp expect .git/logs/refs/heads/d/e/f
+ git reflog show --no-abbrev-commit refs/heads/d/e/f >actual &&
@@ t/t3200-branch.sh: test_expect_success '--set-upstream-to notices an error to se
test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git checkout -b g/h/i -l main &&
- test_ref_exists refs/heads/g/h/i &&
+ test_path_is_file .git/refs/heads/g/h/i &&
- test_path_is_file .git/logs/refs/heads/g/h/i &&
- test_cmp expect .git/logs/refs/heads/g/h/i
+ git reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&
7: 3dc65a80074 ! 5: 089565a358e t1450: convert tests to remove worktrees via git-worktree(1)
@@ Commit message
## t/t1450-fsck.sh ##
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny place' '
+
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref HEAD $saved_head" &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
- test_when_finished "rm -rf .git/worktrees wt" &&
+ test_when_finished "git worktree remove -f wt" &&
git worktree add wt &&
8: c4d09e3e5db ! 6: cb738888ed7 t4207: delete replace references via git-update-ref(1)
@@ Commit message
## t/t4207-log-decoration-colors.sh ##
@@ t/t4207-log-decoration-colors.sh: ${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ cmp_filtered_decorations
'
++remove_replace_refs () {
++ git for-each-ref 'refs/replace*/**' --format='delete %(refname)' >in &&
++ git update-ref --stdin <in &&
++ rm in
++}
++
test_expect_success 'test coloring with replace-objects' '
- test_when_finished rm -rf .git/refs/replace* &&
-+ test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
++ test_when_finished remove_replace_refs &&
test_commit C &&
test_commit D &&
@@ t/t4207-log-decoration-colors.sh: EOF
test_expect_success 'test coloring with grafted commit' '
- test_when_finished rm -rf .git/refs/replace* &&
-+ test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
++ test_when_finished remove_replace_refs &&
git replace --graft HEAD HEAD~2 &&
9: 153b5b199c8 = 7: e730e011de4 t7300: assert exact states of repo
10: b99d98b00a3 = 8: a1bdea52397 t7900: assert the absence of refs via git-for-each-ref(1)
11: 67cb282a414 ! 9: 497e43ae5c3 t: mark several tests that assume the files backend with REFFILES
@@ t/t1450-fsck.sh: test_expect_success 'branch pointing to non-commit' '
-test_expect_success 'HEAD link pointing at a funny object' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref HEAD ${saved_head}" &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
echo $ZERO_OID >.git/HEAD &&
+ # avoid corrupt/broken HEAD from interfering with repo discovery
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny place' '
test_i18ngrep "HEAD points to something strange" out
'
-test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object (from different wt)' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref HEAD $saved_head" &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
test_when_finished "git worktree remove -f wt" &&
+ git worktree add wt &&
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
test_i18ngrep "main-worktree/HEAD: detached HEAD points" out
'
@@ t/t2011-checkout-invalid-head.sh: test_expect_success 'create ref directory/file
## t/t3200-branch.sh ##
@@ t/t3200-branch.sh: test_expect_success 'git branch --help should not have created a bogus branch' '
- test_ref_missing refs/heads/--help
+ test_path_is_missing .git/refs/heads/--help
'
-test_expect_success 'branch -h in broken repository' '
@@ t/t3200-branch.sh: test_expect_success 'git branch -M baz bam should succeed whe
git worktree add -f bazdir2 baz &&
touch .git/worktrees/bazdir1/HEAD.lock &&
@@ t/t3200-branch.sh: test_expect_success 'renaming a symref is not allowed' '
- test_ref_missing refs/heads/new-topic
+ test_path_is_missing .git/refs/heads/new-topic
'
-test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
base-commit: a9ecda2788e229afc9b611acaa26d0d9d4da53ed
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Marc Branchaud @ 2023-10-24 14:01 UTC (permalink / raw)
To: Oswald Buddenhagen, git
Cc: Junio C Hamano, Phillip Wood, Christian Couder, Charvi Mendiratta
In-Reply-To: <20231023130016.1093356-1-oswald.buddenhagen@gmx.de>
On 2023-10-23 09:00, Oswald Buddenhagen wrote:
> Create a clear top-down structure which makes it hopefully unambiguous
> what happens when.
>
> Also mention the timestamp along with the author - this is primarily
> meant to include the keywords somebody might be searching for, like I
> did a year ago.
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>
> ---
> v2:
> - slight adjustments inspired by marc. however, i left most things
> unchanged or even went in the opposite direction, because i assume the
> readers to be sufficiently context-sensitive, and the objective is
> merely to be not actively confusing. adding redundancy in the name of
> clarity would just make the text stylistically inferior and arguably
> harder to read.
I disagree with this on many levels, but your tone seems to brook no
discussion and I do not want to get into a protracted debate here.
I will only say that, I personally don't read man pages from
start-to-end like a novel. I jump to the part that explains the thing I
need to learn about. So I think your assumptions about what context a
reader might have in mind when they see this text are invalid.
Since we have very different notions about who is reading this, I think
we'll never agree on the final wording. I'll continue to make my
suggestions, but I won't stand in the way of these changes if I'm the
only one who thinks they could be better.
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Christian Couder <christian.couder@gmail.com>
> Cc: Charvi Mendiratta <charvi077@gmail.com>
> Cc: Marc Branchaud <marcnarc@xiplink.com>
> ---
> Documentation/git-rebase.txt | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index e7b39ad244..337df9ef2f 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -890,20 +890,21 @@ command "pick" with the command "reword".
> To drop a commit, replace the command "pick" with "drop", or just
> delete the matching line.
>
> -If you want to fold two or more commits into one, replace the command
> -"pick" for the second and subsequent commits with "squash" or "fixup".
> -If the commits had different authors, the folded commit will be
> -attributed to the author of the first commit. The suggested commit
> -message for the folded commit is the concatenation of the first
> -commit's message with those identified by "squash" commands, omitting the
> -messages of commits identified by "fixup" commands, unless "fixup -c"
> -is used. In that case the suggested commit message is only the message
> -of the "fixup -c" commit, and an editor is opened allowing you to edit
> -the message. The contents (patch) of the "fixup -c" commit are still
> -incorporated into the folded commit. If there is more than one "fixup -c"
> -commit, the message from the final one is used. You can also use
> -"fixup -C" to get the same behavior as "fixup -c" except without opening
> -an editor.
> +If you want to fold two or more commits into one (that is, to combine
> +their contents/patches), replace the command "pick" for the second and
> +subsequent commits with "squash" or "fixup".
s/the command "pick"/the "pick" command/
> +The commit message for the folded commit is the concatenation of the
> +message of the first commit with those of commits identified by "squash"
s/message of the first commit/picked commit's message/
> +commands, omitting those of commits identified by "fixup" commands,
> +unless "fixup -c" is used. In the latter case, the message is obtained
> +only from the "fixup -c" commit (having more than one of these is
> +incorrect).
As Phillip said, this is wrong. I agree with Phillip that the
documentation should reflect the actual implementation, not what we hope
the implementation might be some day.
> +If the resulting commit message is a concatenation of multiple messages,
> +an editor is opened allowing you to edit it. This is also the case for a
> +message obtained via "fixup -c", while using "fixup -C" instead skips
> +the editor; this is analogous to the behavior of `git commit`.
> +The first commit which contributes to the suggested commit message also
s/suggested/folded/ -- with "fixup -C" there is no "suggested" message.
Thanks,
M.
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Phillip Wood @ 2023-10-24 14:00 UTC (permalink / raw)
To: Oswald Buddenhagen, phillip.wood; +Cc: Johannes Sixt, git
In-Reply-To: <ZTeZ3KEQLIVU/sq2@ugly>
On 24/10/2023 11:18, Oswald Buddenhagen wrote:
> On Tue, Oct 24, 2023 at 10:26:29AM +0100, Phillip Wood wrote:
>> On 20/03/2022 08:05, Johannes Sixt wrote:
>>> Am 19.03.22 um 13:48 schrieb Oswald Buddenhagen:
>>>> during interactive rebasing, i sometimes find it necessary to move a
>>>> hunk from one commit to a later one in the branch. now, if that hunk
>>>> cannot be re-ordered with the later commit due to conflicting with it,
>>>> it becomes necessary to squash the later commit onto a temporary commit
>>>> created from the extracted hunk, not the other way around (or using a
>>>> stash). unfortunately, this causes the author date of the later commit
>>>> to be reset, which can rather seriously falsify the date if the branch
>>>> is long-lived.
>>>
>>> You want `fixup -C` in the todo-list. See the hints near the end of the
>>> todo-list.
>>
>> Unfortunately "fixup -C" only copies the commit message not the
>> authorship
>
>> (that's usually a good thing
>>
> why? what would that be useful for?
> it seems rather counter-intuitive.
In the same way that you do not want to change the author date when
using a fixup to move a small hunk from one commit to another most users
do not want to update the author information when they make a small
change to a commit message using "fixup -C"
> it's also inconsistent with commit -c/-C's behavior, which seems like a
> red flag to me.
That could mean the option is mis-named instead rather than the behavior
being wrong.
>> but not it means it wont work for what Oswald wants to do).
>
>> Maybe we should add another flag for fixup/squash commands to take the
>> authorship from that commit.
>>
> that's a possibility. but given the above, it might be better to simply
> change the behavior of -c/-C to keep the UI lean and consistent with
> commit's behavior.
"fixup -c/-C" were conceived as a way to reword a commit message at the
same time as optionally fixing up the commit's content. I think changing
the behavior to automatically update the authorship would surprise
people and as I said above most of the time one does not want that behavior.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v1 3/4] config: factor out global config file retrievalync-mailbox>
From: Kristoffer Haugsbakk @ 2023-10-24 13:23 UTC (permalink / raw)
To: Taylor Blau, Patrick Steinhardt; +Cc: git, stolee
In-Reply-To: <ZTav2u1JWmLexEHL@nand.local>
Hi Taylor and Patrick
On Mon, Oct 23, 2023, at 19:40, Taylor Blau wrote:
>> Nit: we don't know about the intent of the caller, so they may not want
>> to write to the file but only read it.
>
> I was going to suggest that we allow the caller to pass in the flags
> that they wish for git_global_config() to pass down to access(2), but
> was surprised to see that we always use R_OK.
>
> But thinking on it for a moment longer, I realized that we don't care
> about write-level permissions for the config, since we want to instead
> open $GIT_DIR/config.lock for writing, and then rename() it into place,
> meaning we only care about whether or not we have write permissions on
> $GIT_DIR itself.
>
> I think in the existing location of this code, the "if we should write"
> portion of the comment is premature, since we don't know for sure
> whether or not we are writing. So I'd be fine with leaving it as-is, but
> changing the comment seems easy enough to do...
>
>> > + * location; error out even if XDG_CONFIG_HOME
>> > + * is set and points at a sane location.
>> > + */
>> > + die(_("$HOME not set"));
>>
>> Is it sensible to `die()` here in this new function that behaves more
>> like a library function? I imagine it would be more sensible to indicate
>> the error to the user and let them handle it accordingly.
>
> Agreed.
>
> Thanks,
> Taylor
What do you guys think the signature of `git_global_config` should be?
^ permalink raw reply
* [PATCH 12/12] t: use git-show-ref(1) to check for ref existence
From: Patrick Steinhardt @ 2023-10-24 13:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 14354 bytes --]
Convert tests that use `test_path_is_file` and `test_path_is_missing` to
instead use a set of helpers `test_ref_exists` and `test_ref_missing`.
These helpers are implemented via the newly introduced `git show-ref
--exists` command. Thus, we can avoid intimate knowledge of how the ref
backend stores references on disk.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1430-bad-ref-name.sh | 27 +++++++++++++-------
t/t3200-branch.sh | 33 ++++++++++++++-----------
t/t5521-pull-options.sh | 4 +--
t/t5605-clone-local.sh | 2 +-
t/test-lib-functions.sh | 55 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 94 insertions(+), 27 deletions(-)
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index ff1c967d550..7b7d6953c62 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -205,8 +205,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -216,8 +217,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git branch -d badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
test_must_be_empty error
'
@@ -225,8 +227,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -234,8 +237,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok
test_expect_success 'branch -d can delete dangling symref to broken name' '
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git branch -d badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
test_must_be_empty error
'
@@ -245,8 +249,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/broken...ref &&
git update-ref -d refs/heads/badname >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...ref &&
+ test_ref_missing refs/heads/broken...ref &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -254,8 +259,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -263,8 +269,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name
test_expect_success 'branch -d can delete symref with broken name' '
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
test_must_be_empty error
'
@@ -272,8 +279,9 @@ test_expect_success 'branch -d can delete symref with broken name' '
test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -281,8 +289,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
test_expect_success 'branch -d can delete dangling symref with broken name' '
printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
test_must_be_empty error
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 080e4f24a6e..bde4f1485b7 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -25,7 +25,7 @@ test_expect_success 'prepare a trivial repository' '
test_expect_success 'git branch --help should not have created a bogus branch' '
test_might_fail git branch --man --help </dev/null >/dev/null 2>&1 &&
- test_path_is_missing .git/refs/heads/--help
+ test_ref_missing refs/heads/--help
'
test_expect_success 'branch -h in broken repository' '
@@ -40,7 +40,8 @@ test_expect_success 'branch -h in broken repository' '
'
test_expect_success 'git branch abc should create a branch' '
- git branch abc && test_path_is_file .git/refs/heads/abc
+ git branch abc &&
+ test_ref_exists refs/heads/abc
'
test_expect_success 'git branch abc should fail when abc exists' '
@@ -61,11 +62,13 @@ test_expect_success 'git branch --force abc should succeed when abc exists' '
'
test_expect_success 'git branch a/b/c should create a branch' '
- git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c
+ git branch a/b/c &&
+ test_ref_exists refs/heads/a/b/c
'
test_expect_success 'git branch mb main... should create a branch' '
- git branch mb main... && test_path_is_file .git/refs/heads/mb
+ git branch mb main... &&
+ test_ref_exists refs/heads/mb
'
test_expect_success 'git branch HEAD should fail' '
@@ -78,14 +81,14 @@ EOF
test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
- test_path_is_file .git/refs/heads/d/e/f &&
+ test_ref_exists refs/heads/d/e/f &&
test_path_is_file .git/logs/refs/heads/d/e/f &&
test_cmp expect .git/logs/refs/heads/d/e/f
'
test_expect_success 'git branch -d d/e/f should delete a branch and a log' '
git branch -d d/e/f &&
- test_path_is_missing .git/refs/heads/d/e/f &&
+ test_ref_missing refs/heads/d/e/f &&
test_must_fail git reflog exists refs/heads/d/e/f
'
@@ -213,7 +216,7 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
test_commit initial &&
git checkout --orphan lonely &&
grep lonely .git/HEAD &&
- test_path_is_missing .git/refs/head/lonely &&
+ test_ref_missing refs/head/lonely &&
git branch -M main mistress &&
grep lonely .git/HEAD
)
@@ -799,8 +802,8 @@ test_expect_success 'deleting a symref' '
git symbolic-ref refs/heads/symref refs/heads/target &&
echo "Deleted branch symref (was refs/heads/target)." >expect &&
git branch -d symref >actual &&
- test_path_is_file .git/refs/heads/target &&
- test_path_is_missing .git/refs/heads/symref &&
+ test_ref_exists refs/heads/target &&
+ test_ref_missing refs/heads/symref &&
test_cmp expect actual
'
@@ -809,16 +812,16 @@ test_expect_success 'deleting a dangling symref' '
test_path_is_file .git/refs/heads/dangling-symref &&
echo "Deleted branch dangling-symref (was nowhere)." >expect &&
git branch -d dangling-symref >actual &&
- test_path_is_missing .git/refs/heads/dangling-symref &&
+ test_ref_missing refs/heads/dangling-symref &&
test_cmp expect actual
'
test_expect_success 'deleting a self-referential symref' '
git symbolic-ref refs/heads/self-reference refs/heads/self-reference &&
- test_path_is_file .git/refs/heads/self-reference &&
+ test_ref_exists refs/heads/self-reference &&
echo "Deleted branch self-reference (was refs/heads/self-reference)." >expect &&
git branch -d self-reference >actual &&
- test_path_is_missing .git/refs/heads/self-reference &&
+ test_ref_missing refs/heads/self-reference &&
test_cmp expect actual
'
@@ -826,8 +829,8 @@ test_expect_success 'renaming a symref is not allowed' '
git symbolic-ref refs/heads/topic refs/heads/main &&
test_must_fail git branch -m topic new-topic &&
git symbolic-ref refs/heads/topic &&
- test_path_is_file .git/refs/heads/main &&
- test_path_is_missing .git/refs/heads/new-topic
+ test_ref_exists refs/heads/main &&
+ test_ref_missing refs/heads/new-topic
'
test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
@@ -1142,7 +1145,7 @@ EOF
test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git checkout -b g/h/i -l main &&
- test_path_is_file .git/refs/heads/g/h/i &&
+ test_ref_exists refs/heads/g/h/i &&
test_path_is_file .git/logs/refs/heads/g/h/i &&
test_cmp expect .git/logs/refs/heads/g/h/i
'
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 079b2f2536e..3681859f983 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -143,7 +143,7 @@ test_expect_success 'git pull --dry-run' '
cd clonedry &&
git pull --dry-run ../parent &&
test_path_is_missing .git/FETCH_HEAD &&
- test_path_is_missing .git/refs/heads/main &&
+ test_ref_missing refs/heads/main &&
test_path_is_missing .git/index &&
test_path_is_missing file
)
@@ -157,7 +157,7 @@ test_expect_success 'git pull --all --dry-run' '
git remote add origin ../parent &&
git pull --all --dry-run &&
test_path_is_missing .git/FETCH_HEAD &&
- test_path_is_missing .git/refs/remotes/origin/main &&
+ test_ref_missing refs/remotes/origin/main &&
test_path_is_missing .git/index &&
test_path_is_missing file
)
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 1d7b1abda1a..946c5751885 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -69,7 +69,7 @@ test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
git clone a d &&
(cd d &&
git fetch &&
- test ! -e .git/refs/remotes/origin/HEAD)
+ test_ref_missing refs/remotes/origin/HEAD)
'
test_expect_success 'bundle clone without .bundle suffix' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 2f8868caa17..56b33536ed1 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -251,6 +251,61 @@ debug () {
done
}
+# Usage: test_ref_exists [options] <ref>
+#
+# -C <dir>:
+# Run all git commands in directory <dir>
+#
+# This helper function checks whether a reference exists. Symrefs or object IDs
+# will not be resolved. Can be used to check references with bad names.
+test_ref_exists () {
+ local indir=
+
+ while test $# != 0
+ do
+ case "$1" in
+ -C)
+ indir="$2"
+ shift
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done &&
+
+ indir=${indir:+"$indir"/} &&
+
+ if test "$#" != 1
+ then
+ BUG "expected exactly one reference"
+ fi &&
+
+ git ${indir:+ -C "$indir"} show-ref --exists "$1"
+}
+
+# Behaves the same as test_ref_exists, except that it checks for the absence of
+# a reference. This is preferable to `! test_ref_exists` as this function is
+# able to distinguish actually-missing references from other, generic errors.
+test_ref_missing () {
+ test_ref_exists "$@"
+ case "$?" in
+ 2)
+ # This is the good case.
+ return 0
+ ;;
+ 0)
+ echo >&4 "test_ref_missing: reference exists"
+ return 1
+ ;;
+ *)
+ echo >&4 "test_ref_missing: generic error"
+ return 1
+ ;;
+ esac
+}
+
# Usage: test_commit [options] <message> [<file> [<contents> [<tag>]]]
# -C <dir>:
# Run all git commands in directory <dir>
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 11/12] builtin/show-ref: add new mode to check for reference existence
From: Patrick Steinhardt @ 2023-10-24 13:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 9653 bytes --]
While we have multiple ways to show the value of a given reference, we
do not have any way to check whether a reference exists at all. While
commands like git-rev-parse(1) or git-show-ref(1) can be used to check
for reference existence in case the reference resolves to something
sane, neither of them can be used to check for existence in some other
scenarios where the reference does not resolve cleanly:
- References which have an invalid name cannot be resolved.
- References to nonexistent objects cannot be resolved.
- Dangling symrefs can be resolved via git-symbolic-ref(1), but this
requires the caller to special case existence checks depending on
whteher or not a reference is symbolic or direct.
Furthermore, git-rev-list(1) and other commands do not let the caller
distinguish easily between an actually missing reference and a generic
error.
Taken together, this gseems like sufficient motivation to introduce a
separate plumbing command to explicitly check for the existence of a
reference without trying to resolve its contents.
This new command comes in the form of `git show-ref --exists`. This
new mode will exit successfully when the reference exists, with a
specific error code of 2 when it does not exist, or with 1 when there
has been a generic error.
Note that the only way to properly implement this command is by using
the internal `refs_read_raw_ref()` function. While the public function
`refs_resolve_ref_unsafe()` can be made to behave in the same way by
passing various flags, it does not provide any way to obtain the errno
with which the reference backend failed when reading the reference. As
such, it becomes impossible for us to distinguish generic errors from
the explicit case where the reference wasn't found.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-show-ref.txt | 11 ++++++
builtin/show-ref.c | 47 ++++++++++++++++++++++--
t/t1403-show-ref.sh | 67 +++++++++++++++++++++++++++++++++-
3 files changed, 120 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index ab23e0b62e1..a7e9374bc2b 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[-s | --hash[=<n>]] [--abbrev[=<n>]]
[--] [<ref>...]
'git show-ref' --exclude-existing[=<pattern>]
+'git show-ref' --exists <ref>
DESCRIPTION
-----------
@@ -30,6 +31,10 @@ The `--exclude-existing` form is a filter that does the inverse. It reads
refs from stdin, one ref per line, and shows those that don't exist in
the local repository.
+The `--exists` form can be used to check for the existence of a single
+references. This form does not verify whether the reference resolves to an
+actual object.
+
Use of this utility is encouraged in favor of directly accessing files under
the `.git` directory.
@@ -65,6 +70,12 @@ OPTIONS
Aside from returning an error code of 1, it will also print an error
message if `--quiet` was not specified.
+--exists::
+
+ Check whether the given reference exists. Returns an error code of 0 if
+ it does, 2 if it is missing, and 128 in case looking up the reference
+ failed with an error other than the reference being missing.
+
--abbrev[=<n>]::
Abbreviate the object name. When using `--hash`, you do
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index d0a32d07404..617e754bbed 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -2,7 +2,7 @@
#include "config.h"
#include "gettext.h"
#include "hex.h"
-#include "refs.h"
+#include "refs/refs-internal.h"
#include "object-name.h"
#include "object-store-ll.h"
#include "object.h"
@@ -18,6 +18,7 @@ static const char * const show_ref_usage[] = {
" [-s | --hash[=<n>]] [--abbrev[=<n>]]\n"
" [--] [<ref>...]"),
N_("git show-ref --exclude-existing[=<pattern>]"),
+ N_("git show-ref --exists <ref>"),
NULL
};
@@ -214,6 +215,41 @@ static int cmd_show_ref__patterns(const struct patterns_options *opts,
return 0;
}
+static int cmd_show_ref__exists(const char **refs)
+{
+ struct strbuf unused_referent = STRBUF_INIT;
+ struct object_id unused_oid;
+ unsigned int unused_type;
+ int failure_errno = 0;
+ const char *ref;
+ int ret = 1;
+
+ if (!refs || !*refs)
+ die("--exists requires a reference");
+ ref = *refs++;
+ if (*refs)
+ die("--exists requires exactly one reference");
+
+ if (refs_read_raw_ref(get_main_ref_store(the_repository), ref,
+ &unused_oid, &unused_referent, &unused_type,
+ &failure_errno)) {
+ if (failure_errno == ENOENT) {
+ error(_("reference does not exist"));
+ ret = 2;
+ } else {
+ error(_("failed to look up reference: %s"), strerror(failure_errno));
+ }
+
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ strbuf_release(&unused_referent);
+ return ret;
+}
+
static int hash_callback(const struct option *opt, const char *arg, int unset)
{
struct show_one_options *opts = opt->value;
@@ -243,10 +279,11 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
struct exclude_existing_options exclude_existing_opts = {0};
struct patterns_options patterns_opts = {0};
struct show_one_options show_one_opts = {0};
- int verify = 0;
+ int verify = 0, exists = 0;
const struct option show_ref_options[] = {
OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with heads)")),
OPT_BOOL(0, "heads", &patterns_opts.heads_only, N_("only show heads (can be combined with tags)")),
+ OPT_BOOL(0, "exists", &exists, N_("check for reference existence without resolving")),
OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
"requires exact ref path")),
OPT_HIDDEN_BOOL('h', NULL, &patterns_opts.show_head,
@@ -272,13 +309,15 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, show_ref_options,
show_ref_usage, 0);
- if ((!!exclude_existing_opts.enabled + !!verify) > 1)
- die(_("only one of --exclude-existing or --verify can be given"));
+ if ((!!exclude_existing_opts.enabled + !!verify + !!exists) > 1)
+ die(_("only one of --exclude-existing, --exists or --verify can be given"));
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
else if (verify)
return cmd_show_ref__verify(&show_one_opts, argv);
+ else if (exists)
+ return cmd_show_ref__exists(argv);
else
return cmd_show_ref__patterns(&patterns_opts, &show_one_opts, argv);
}
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 3a312c8b27c..17eba350ce5 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -197,8 +197,73 @@ test_expect_success 'show-ref --verify with dangling ref' '
'
test_expect_success 'show-ref sub-modes are mutually exclusive' '
+ cat >expect <<-EOF &&
+ fatal: only one of --exclude-existing, --exists or --verify can be given
+ EOF
+
test_must_fail git show-ref --verify --exclude-existing 2>err &&
- grep "only one of --exclude-existing or --verify can be given" err
+ test_cmp expect err &&
+
+ test_must_fail git show-ref --verify --exists 2>err &&
+ test_cmp expect err &&
+
+ test_must_fail git show-ref --exclude-existing --exists 2>err &&
+ test_cmp expect err
+'
+
+test_expect_success '--exists with existing reference' '
+ git show-ref --exists refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+'
+
+test_expect_success '--exists with missing reference' '
+ test_expect_code 2 git show-ref --exists refs/heads/does-not-exist
+'
+
+test_expect_success '--exists does not use DWIM' '
+ test_expect_code 2 git show-ref --exists $GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 2>err &&
+ grep "reference does not exist" err
+'
+
+test_expect_success '--exists with HEAD' '
+ git show-ref --exists HEAD
+'
+
+test_expect_success '--exists with bad reference name' '
+ test_when_finished "git update-ref -d refs/heads/bad...name" &&
+ new_oid=$(git rev-parse HEAD) &&
+ test-tool ref-store main update-ref msg refs/heads/bad...name $new_oid $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+ git show-ref --exists refs/heads/bad...name
+'
+
+test_expect_success '--exists with arbitrary symref' '
+ test_when_finished "git symbolic-ref -d refs/symref" &&
+ git symbolic-ref refs/symref refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME &&
+ git show-ref --exists refs/symref
+'
+
+test_expect_success '--exists with dangling symref' '
+ test_when_finished "git symbolic-ref -d refs/heads/dangling" &&
+ git symbolic-ref refs/heads/dangling refs/heads/does-not-exist &&
+ git show-ref --exists refs/heads/dangling
+'
+
+test_expect_success '--exists with nonexistent object ID' '
+ test-tool ref-store main update-ref msg refs/heads/missing-oid $(test_oid 001) $ZERO_OID REF_SKIP_OID_VERIFICATION &&
+ git show-ref --exists refs/heads/missing-oid
+'
+
+test_expect_success '--exists with non-commit object' '
+ tree_oid=$(git rev-parse HEAD^{tree}) &&
+ test-tool ref-store main update-ref msg refs/heads/tree ${tree_oid} $ZERO_OID REF_SKIP_OID_VERIFICATION &&
+ git show-ref --exists refs/heads/tree
+'
+
+test_expect_success '--exists with directory fails with generic error' '
+ cat >expect <<-EOF &&
+ error: failed to look up reference: Is a directory
+ EOF
+ test_expect_code 1 git show-ref --exists refs/heads 2>err &&
+ test_cmp expect err
'
test_done
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 10/12] builtin/show-ref: explicitly spell out different modes in synopsis
From: Patrick Steinhardt @ 2023-10-24 13:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]
The synopsis treats the `--verify` and the implicit mode the same. They
are slightly different though:
- They accept different sets of flags.
- The implicit mode accepts patterns while the `--verify` mode
accepts references.
Split up the synopsis for these two modes such that we can disambiguate
those differences.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-show-ref.txt | 5 ++++-
builtin/show-ref.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index 2fe274b8faa..ab23e0b62e1 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -8,9 +8,12 @@ git-show-ref - List references in a local repository
SYNOPSIS
--------
[verse]
-'git show-ref' [-q | --quiet] [--verify] [--head] [-d | --dereference]
+'git show-ref' [-q | --quiet] [--head] [-d | --dereference]
[-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]
[--heads] [--] [<pattern>...]
+'git show-ref' --verify [-q | --quiet] [-d | --dereference]
+ [-s | --hash[=<n>]] [--abbrev[=<n>]]
+ [--] [<ref>...]
'git show-ref' --exclude-existing[=<pattern>]
DESCRIPTION
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 10d0213e687..d0a32d07404 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -11,9 +11,12 @@
#include "parse-options.h"
static const char * const show_ref_usage[] = {
- N_("git show-ref [-q | --quiet] [--verify] [--head] [-d | --dereference]\n"
+ N_("git show-ref [-q | --quiet] [--head] [-d | --dereference]\n"
" [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]\n"
" [--heads] [--] [<pattern>...]"),
+ N_("git show-ref --verify [-q | --quiet] [-d | --dereference]\n"
+ " [-s | --hash[=<n>]] [--abbrev[=<n>]]\n"
+ " [--] [<ref>...]"),
N_("git show-ref --exclude-existing[=<pattern>]"),
NULL
};
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 09/12] builtin/show-ref: ensure mutual exclusiveness of subcommands
From: Patrick Steinhardt @ 2023-10-24 13:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]
The git-show-ref(1) command has three different modes, of which one is
implicit and the other two can be chosen explicitly by passing a flag.
But while these modes are standalone and cause us to execute completely
separate code paths, we gladly accept the case where a user asks for
both `--exclude-existing` and `--verify` at the same time even though it
is not obvious what will happen. Spoiler: we ignore `--verify` and
execute the `--exclude-existing` mode.
Let's explicitly detect this invalid usage and die in case both modes
were requested.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 3 +++
t/t1403-show-ref.sh | 5 +++++
2 files changed, 8 insertions(+)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 5d5d7d22ed1..10d0213e687 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -269,6 +269,9 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, show_ref_options,
show_ref_usage, 0);
+ if ((!!exclude_existing_opts.enabled + !!verify) > 1)
+ die(_("only one of --exclude-existing or --verify can be given"));
+
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
else if (verify)
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 9252a581abf..3a312c8b27c 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -196,4 +196,9 @@ test_expect_success 'show-ref --verify with dangling ref' '
)
'
+test_expect_success 'show-ref sub-modes are mutually exclusive' '
+ test_must_fail git show-ref --verify --exclude-existing 2>err &&
+ grep "only one of --exclude-existing or --verify can be given" err
+'
+
test_done
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 08/12] builtin/show-ref: refactor options for patterns subcommand
From: Patrick Steinhardt @ 2023-10-24 13:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3943 bytes --]
The patterns subcommand is the last command that still uses global
variables to track its options. Convert it to use a structure instead
with the same motivation as preceding commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 589a91f15b9..5d5d7d22ed1 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -18,8 +18,6 @@ static const char * const show_ref_usage[] = {
NULL
};
-static int show_head, tags_only, heads_only, verify;
-
struct show_one_options {
int quiet;
int hash_only;
@@ -59,6 +57,7 @@ struct show_ref_data {
const struct show_one_options *show_one_opts;
const char **patterns;
int found_match;
+ int show_head;
};
static int show_ref(const char *refname, const struct object_id *oid,
@@ -66,7 +65,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
{
struct show_ref_data *data = cbdata;
- if (show_head && !strcmp(refname, "HEAD"))
+ if (data->show_head && !strcmp(refname, "HEAD"))
goto match;
if (data->patterns) {
@@ -180,20 +179,28 @@ static int cmd_show_ref__verify(const struct show_one_options *show_one_opts,
return 0;
}
-static int cmd_show_ref__patterns(const struct show_one_options *show_one_opts,
+struct patterns_options {
+ int show_head;
+ int heads_only;
+ int tags_only;
+};
+
+static int cmd_show_ref__patterns(const struct patterns_options *opts,
+ const struct show_one_options *show_one_opts,
const char **patterns)
{
struct show_ref_data show_ref_data = {
.show_one_opts = show_one_opts,
+ .show_head = opts->show_head,
.patterns = (patterns && *patterns) ? patterns : NULL,
};
- if (show_head)
+ if (opts->show_head)
head_ref(show_ref, &show_ref_data);
- if (heads_only || tags_only) {
- if (heads_only)
+ if (opts->heads_only || opts->tags_only) {
+ if (opts->heads_only)
for_each_fullref_in("refs/heads/", show_ref, &show_ref_data);
- if (tags_only)
+ if (opts->tags_only)
for_each_fullref_in("refs/tags/", show_ref, &show_ref_data);
} else {
for_each_ref(show_ref, &show_ref_data);
@@ -231,15 +238,17 @@ static int exclude_existing_callback(const struct option *opt, const char *arg,
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
struct exclude_existing_options exclude_existing_opts = {0};
+ struct patterns_options patterns_opts = {0};
struct show_one_options show_one_opts = {0};
+ int verify = 0;
const struct option show_ref_options[] = {
- OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
- OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
+ OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with heads)")),
+ OPT_BOOL(0, "heads", &patterns_opts.heads_only, N_("only show heads (can be combined with tags)")),
OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
"requires exact ref path")),
- OPT_HIDDEN_BOOL('h', NULL, &show_head,
+ OPT_HIDDEN_BOOL('h', NULL, &patterns_opts.show_head,
N_("show the HEAD reference, even if it would be filtered out")),
- OPT_BOOL(0, "head", &show_head,
+ OPT_BOOL(0, "head", &patterns_opts.show_head,
N_("show the HEAD reference, even if it would be filtered out")),
OPT_BOOL('d', "dereference", &show_one_opts.deref_tags,
N_("dereference tags into object IDs")),
@@ -265,5 +274,5 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
else if (verify)
return cmd_show_ref__verify(&show_one_opts, argv);
else
- return cmd_show_ref__patterns(&show_one_opts, argv);
+ return cmd_show_ref__patterns(&patterns_opts, &show_one_opts, argv);
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 07/12] builtin/show-ref: stop using global vars for `show_one()`
From: Patrick Steinhardt @ 2023-10-24 13:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6093 bytes --]
The `show_one()` function implicitly receives a bunch of options which
are tracked via global variables. This makes it hard to see which
subcommands of git-show-ref(1) actually make use of these options.
Introduce a `show_one_options` structure that gets passed down to this
function. This allows us to get rid of more global state and makes it
more explicit which subcommands use those options.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 59 +++++++++++++++++++++++++++++-----------------
1 file changed, 38 insertions(+), 21 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 4c039007dd1..589a91f15b9 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -18,10 +18,17 @@ static const char * const show_ref_usage[] = {
NULL
};
-static int deref_tags, show_head, tags_only, heads_only, verify,
- quiet, hash_only, abbrev;
+static int show_head, tags_only, heads_only, verify;
-static void show_one(const char *refname, const struct object_id *oid)
+struct show_one_options {
+ int quiet;
+ int hash_only;
+ int abbrev;
+ int deref_tags;
+};
+
+static void show_one(const struct show_one_options *opts,
+ const char *refname, const struct object_id *oid)
{
const char *hex;
struct object_id peeled;
@@ -30,25 +37,26 @@ static void show_one(const char *refname, const struct object_id *oid)
die("git show-ref: bad ref %s (%s)", refname,
oid_to_hex(oid));
- if (quiet)
+ if (opts->quiet)
return;
- hex = repo_find_unique_abbrev(the_repository, oid, abbrev);
- if (hash_only)
+ hex = repo_find_unique_abbrev(the_repository, oid, opts->abbrev);
+ if (opts->hash_only)
printf("%s\n", hex);
else
printf("%s %s\n", hex, refname);
- if (!deref_tags)
+ if (!opts->deref_tags)
return;
if (!peel_iterated_oid(oid, &peeled)) {
- hex = repo_find_unique_abbrev(the_repository, &peeled, abbrev);
+ hex = repo_find_unique_abbrev(the_repository, &peeled, opts->abbrev);
printf("%s %s^{}\n", hex, refname);
}
}
struct show_ref_data {
+ const struct show_one_options *show_one_opts;
const char **patterns;
int found_match;
};
@@ -81,7 +89,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
match:
data->found_match++;
- show_one(refname, oid);
+ show_one(data->show_one_opts, refname, oid);
return 0;
}
@@ -149,7 +157,8 @@ static int cmd_show_ref__exclude_existing(const struct exclude_existing_options
return 0;
}
-static int cmd_show_ref__verify(const char **refs)
+static int cmd_show_ref__verify(const struct show_one_options *show_one_opts,
+ const char **refs)
{
if (!refs || !*refs)
die("--verify requires a reference");
@@ -159,9 +168,9 @@ static int cmd_show_ref__verify(const char **refs)
if ((starts_with(*refs, "refs/") || !strcmp(*refs, "HEAD")) &&
!read_ref(*refs, &oid)) {
- show_one(*refs, &oid);
+ show_one(show_one_opts, *refs, &oid);
}
- else if (!quiet)
+ else if (!show_one_opts->quiet)
die("'%s' - not a valid ref", *refs);
else
return 1;
@@ -171,9 +180,11 @@ static int cmd_show_ref__verify(const char **refs)
return 0;
}
-static int cmd_show_ref__patterns(const char **patterns)
+static int cmd_show_ref__patterns(const struct show_one_options *show_one_opts,
+ const char **patterns)
{
struct show_ref_data show_ref_data = {
+ .show_one_opts = show_one_opts,
.patterns = (patterns && *patterns) ? patterns : NULL,
};
@@ -195,11 +206,16 @@ static int cmd_show_ref__patterns(const char **patterns)
static int hash_callback(const struct option *opt, const char *arg, int unset)
{
- hash_only = 1;
+ struct show_one_options *opts = opt->value;
+ struct option abbrev_opt = *opt;
+
+ opts->hash_only = 1;
/* Use full length SHA1 if no argument */
if (!arg)
return 0;
- return parse_opt_abbrev_cb(opt, arg, unset);
+
+ abbrev_opt.value = &opts->abbrev;
+ return parse_opt_abbrev_cb(&abbrev_opt, arg, unset);
}
static int exclude_existing_callback(const struct option *opt, const char *arg,
@@ -215,6 +231,7 @@ static int exclude_existing_callback(const struct option *opt, const char *arg,
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
struct exclude_existing_options exclude_existing_opts = {0};
+ struct show_one_options show_one_opts = {0};
const struct option show_ref_options[] = {
OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
@@ -224,13 +241,13 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
N_("show the HEAD reference, even if it would be filtered out")),
OPT_BOOL(0, "head", &show_head,
N_("show the HEAD reference, even if it would be filtered out")),
- OPT_BOOL('d', "dereference", &deref_tags,
+ OPT_BOOL('d', "dereference", &show_one_opts.deref_tags,
N_("dereference tags into object IDs")),
- OPT_CALLBACK_F('s', "hash", &abbrev, N_("n"),
+ OPT_CALLBACK_F('s', "hash", &show_one_opts, N_("n"),
N_("only show SHA1 hash using <n> digits"),
PARSE_OPT_OPTARG, &hash_callback),
- OPT__ABBREV(&abbrev),
- OPT__QUIET(&quiet,
+ OPT__ABBREV(&show_one_opts.abbrev),
+ OPT__QUIET(&show_one_opts.quiet,
N_("do not print results to stdout (useful with --verify)")),
OPT_CALLBACK_F(0, "exclude-existing", &exclude_existing_opts,
N_("pattern"), N_("show refs from stdin that aren't in local repository"),
@@ -246,7 +263,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
else if (verify)
- return cmd_show_ref__verify(argv);
+ return cmd_show_ref__verify(&show_one_opts, argv);
else
- return cmd_show_ref__patterns(argv);
+ return cmd_show_ref__patterns(&show_one_opts, argv);
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 06/12] builtin/show-ref: stop using global variable to count matches
From: Patrick Steinhardt @ 2023-10-24 13:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
When passing patterns to git-show-ref(1) we're checking whether any
reference matches -- if none does, we indicate this condition via an
unsuccessful exit code.
We're using a global variable to count these matches, which is required
because the counter is getting incremented in a callback function. But
now that we have the `struct show_ref_data` in place, we can get rid of
the global variable and put the counter in there instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index e130b999c0b..4c039007dd1 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -18,7 +18,7 @@ static const char * const show_ref_usage[] = {
NULL
};
-static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
+static int deref_tags, show_head, tags_only, heads_only, verify,
quiet, hash_only, abbrev;
static void show_one(const char *refname, const struct object_id *oid)
@@ -50,6 +50,7 @@ static void show_one(const char *refname, const struct object_id *oid)
struct show_ref_data {
const char **patterns;
+ int found_match;
};
static int show_ref(const char *refname, const struct object_id *oid,
@@ -78,7 +79,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
}
match:
- found_match++;
+ data->found_match++;
show_one(refname, oid);
@@ -186,7 +187,7 @@ static int cmd_show_ref__patterns(const char **patterns)
} else {
for_each_ref(show_ref, &show_ref_data);
}
- if (!found_match)
+ if (!show_ref_data.found_match)
return 1;
return 0;
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 05/12] builtin/show-ref: refactor `--exclude-existing` options
From: Patrick Steinhardt @ 2023-10-24 13:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5784 bytes --]
It's not immediately obvious options which options are applicable to
what subcommand ni git-show-ref(1) because all options exist as global
state. This can easily cause confusion for the reader.
Refactor options for the `--exclude-existing` subcommand to be contained
in a separate structure. This structure is stored on the stack and
passed down as required. Consequentially, it clearly delimits the scope
of those options and requires the reader to worry less about global
state.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 72 +++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 33 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index eb60f940a3c..e130b999c0b 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -19,8 +19,7 @@ static const char * const show_ref_usage[] = {
};
static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
- quiet, hash_only, abbrev, exclude_arg;
-static const char *exclude_existing_arg;
+ quiet, hash_only, abbrev;
static void show_one(const char *refname, const struct object_id *oid)
{
@@ -95,6 +94,11 @@ static int add_existing(const char *refname,
return 0;
}
+struct exclude_existing_options {
+ int enabled;
+ const char *pattern;
+};
+
/*
* read "^(?:<anything>\s)?<refname>(?:\^\{\})?$" from the standard input,
* and
@@ -104,11 +108,11 @@ static int add_existing(const char *refname,
* (4) ignore if refname is a ref that exists in the local repository;
* (5) otherwise output the line.
*/
-static int cmd_show_ref__exclude_existing(const char *match)
+static int cmd_show_ref__exclude_existing(const struct exclude_existing_options *opts)
{
struct string_list existing_refs = STRING_LIST_INIT_DUP;
char buf[1024];
- int matchlen = match ? strlen(match) : 0;
+ int matchlen = opts->pattern ? strlen(opts->pattern) : 0;
for_each_ref(add_existing, &existing_refs);
while (fgets(buf, sizeof(buf), stdin)) {
@@ -124,11 +128,11 @@ static int cmd_show_ref__exclude_existing(const char *match)
for (ref = buf + len; buf < ref; ref--)
if (isspace(ref[-1]))
break;
- if (match) {
+ if (opts->pattern) {
int reflen = buf + len - ref;
if (reflen < matchlen)
continue;
- if (strncmp(ref, match, matchlen))
+ if (strncmp(ref, opts->pattern, matchlen))
continue;
}
if (check_refname_format(ref, 0)) {
@@ -200,44 +204,46 @@ static int hash_callback(const struct option *opt, const char *arg, int unset)
static int exclude_existing_callback(const struct option *opt, const char *arg,
int unset)
{
+ struct exclude_existing_options *opts = opt->value;
BUG_ON_OPT_NEG(unset);
- exclude_arg = 1;
- *(const char **)opt->value = arg;
+ opts->enabled = 1;
+ opts->pattern = arg;
return 0;
}
-static const struct option show_ref_options[] = {
- OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
- OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
- OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
- "requires exact ref path")),
- OPT_HIDDEN_BOOL('h', NULL, &show_head,
- N_("show the HEAD reference, even if it would be filtered out")),
- OPT_BOOL(0, "head", &show_head,
- N_("show the HEAD reference, even if it would be filtered out")),
- OPT_BOOL('d', "dereference", &deref_tags,
- N_("dereference tags into object IDs")),
- OPT_CALLBACK_F('s', "hash", &abbrev, N_("n"),
- N_("only show SHA1 hash using <n> digits"),
- PARSE_OPT_OPTARG, &hash_callback),
- OPT__ABBREV(&abbrev),
- OPT__QUIET(&quiet,
- N_("do not print results to stdout (useful with --verify)")),
- OPT_CALLBACK_F(0, "exclude-existing", &exclude_existing_arg,
- N_("pattern"), N_("show refs from stdin that aren't in local repository"),
- PARSE_OPT_OPTARG | PARSE_OPT_NONEG, exclude_existing_callback),
- OPT_END()
-};
-
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
+ struct exclude_existing_options exclude_existing_opts = {0};
+ const struct option show_ref_options[] = {
+ OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
+ OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
+ OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
+ "requires exact ref path")),
+ OPT_HIDDEN_BOOL('h', NULL, &show_head,
+ N_("show the HEAD reference, even if it would be filtered out")),
+ OPT_BOOL(0, "head", &show_head,
+ N_("show the HEAD reference, even if it would be filtered out")),
+ OPT_BOOL('d', "dereference", &deref_tags,
+ N_("dereference tags into object IDs")),
+ OPT_CALLBACK_F('s', "hash", &abbrev, N_("n"),
+ N_("only show SHA1 hash using <n> digits"),
+ PARSE_OPT_OPTARG, &hash_callback),
+ OPT__ABBREV(&abbrev),
+ OPT__QUIET(&quiet,
+ N_("do not print results to stdout (useful with --verify)")),
+ OPT_CALLBACK_F(0, "exclude-existing", &exclude_existing_opts,
+ N_("pattern"), N_("show refs from stdin that aren't in local repository"),
+ PARSE_OPT_OPTARG | PARSE_OPT_NONEG, exclude_existing_callback),
+ OPT_END()
+ };
+
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, show_ref_options,
show_ref_usage, 0);
- if (exclude_arg)
- return cmd_show_ref__exclude_existing(exclude_existing_arg);
+ if (exclude_existing_opts.enabled)
+ return cmd_show_ref__exclude_existing(&exclude_existing_opts);
else if (verify)
return cmd_show_ref__verify(argv);
else
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 04/12] builtin/show-ref: fix dead code when passing patterns
From: Patrick Steinhardt @ 2023-10-24 13:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 914 bytes --]
When passing patterns to `git show-ref` we have some code that will
cause us to die of `verify && !quiet` is true. But because `verify`
indicates a different subcommand of git-show-ref(1) that causes us to
execute `cmd_show_ref__verify()` and not `cmd_show_ref__patterns()`, the
condition cannot ever be true.
Let's remove this dead code.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 761669d28de..eb60f940a3c 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -182,11 +182,8 @@ static int cmd_show_ref__patterns(const char **patterns)
} else {
for_each_ref(show_ref, &show_ref_data);
}
- if (!found_match) {
- if (verify && !quiet)
- die("No match");
+ if (!found_match)
return 1;
- }
return 0;
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 03/12] builtin/show-ref: fix leaking string buffer
From: Patrick Steinhardt @ 2023-10-24 13:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]
Fix a leaking string buffer in `git show-ref --exclude-existing`. While
the buffer is technically not leaking because its variable is declared
as static, there is no inherent reason why it should be.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 56ee3250c5f..761669d28de 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -106,7 +106,7 @@ static int add_existing(const char *refname,
*/
static int cmd_show_ref__exclude_existing(const char *match)
{
- static struct string_list existing_refs = STRING_LIST_INIT_DUP;
+ struct string_list existing_refs = STRING_LIST_INIT_DUP;
char buf[1024];
int matchlen = match ? strlen(match) : 0;
@@ -139,6 +139,8 @@ static int cmd_show_ref__exclude_existing(const char *match)
printf("%s\n", buf);
}
}
+
+ string_list_clear(&existing_refs, 0);
return 0;
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 02/12] builtin/show-ref: split up different subcommands
From: Patrick Steinhardt @ 2023-10-24 13:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 4249 bytes --]
While not immediately obvious, git-show-ref(1) actually implements three
different subcommands:
- `git show-ref <patterns>` can be used to list references that
match a specific pattern.
- `git show-ref --verify <refs>` can be used to list references.
These are _not_ patterns.
- `git show-ref --exclude-existing` can be used as a filter that
reads references from standard input, performing some conversions
on each of them.
Let's make this more explicit in the code by splitting up the three
subcommands into separate functions. This also allows us to address the
confusingly named `patterns` variable, which may hold either patterns or
reference names.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 100 ++++++++++++++++++++++++---------------------
1 file changed, 53 insertions(+), 47 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 7efab14b96c..56ee3250c5f 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -104,7 +104,7 @@ static int add_existing(const char *refname,
* (4) ignore if refname is a ref that exists in the local repository;
* (5) otherwise output the line.
*/
-static int exclude_existing(const char *match)
+static int cmd_show_ref__exclude_existing(const char *match)
{
static struct string_list existing_refs = STRING_LIST_INIT_DUP;
char buf[1024];
@@ -142,6 +142,53 @@ static int exclude_existing(const char *match)
return 0;
}
+static int cmd_show_ref__verify(const char **refs)
+{
+ if (!refs || !*refs)
+ die("--verify requires a reference");
+
+ while (*refs) {
+ struct object_id oid;
+
+ if ((starts_with(*refs, "refs/") || !strcmp(*refs, "HEAD")) &&
+ !read_ref(*refs, &oid)) {
+ show_one(*refs, &oid);
+ }
+ else if (!quiet)
+ die("'%s' - not a valid ref", *refs);
+ else
+ return 1;
+ refs++;
+ }
+
+ return 0;
+}
+
+static int cmd_show_ref__patterns(const char **patterns)
+{
+ struct show_ref_data show_ref_data = {
+ .patterns = (patterns && *patterns) ? patterns : NULL,
+ };
+
+ if (show_head)
+ head_ref(show_ref, &show_ref_data);
+ if (heads_only || tags_only) {
+ if (heads_only)
+ for_each_fullref_in("refs/heads/", show_ref, &show_ref_data);
+ if (tags_only)
+ for_each_fullref_in("refs/tags/", show_ref, &show_ref_data);
+ } else {
+ for_each_ref(show_ref, &show_ref_data);
+ }
+ if (!found_match) {
+ if (verify && !quiet)
+ die("No match");
+ return 1;
+ }
+
+ return 0;
+}
+
static int hash_callback(const struct option *opt, const char *arg, int unset)
{
hash_only = 1;
@@ -185,56 +232,15 @@ static const struct option show_ref_options[] = {
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
- struct show_ref_data show_ref_data = {0};
- const char **patterns;
-
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, show_ref_options,
show_ref_usage, 0);
if (exclude_arg)
- return exclude_existing(exclude_existing_arg);
-
- patterns = argv;
- if (!*patterns)
- patterns = NULL;
-
- if (verify) {
- if (!patterns)
- die("--verify requires a reference");
- while (*patterns) {
- struct object_id oid;
-
- if ((starts_with(*patterns, "refs/") || !strcmp(*patterns, "HEAD")) &&
- !read_ref(*patterns, &oid)) {
- show_one(*patterns, &oid);
- }
- else if (!quiet)
- die("'%s' - not a valid ref", *patterns);
- else
- return 1;
- patterns++;
- }
- return 0;
- }
-
- show_ref_data.patterns = patterns;
-
- if (show_head)
- head_ref(show_ref, &show_ref_data);
- if (heads_only || tags_only) {
- if (heads_only)
- for_each_fullref_in("refs/heads/", show_ref, &show_ref_data);
- if (tags_only)
- for_each_fullref_in("refs/tags/", show_ref, &show_ref_data);
- } else {
- for_each_ref(show_ref, &show_ref_data);
- }
- if (!found_match) {
- if (verify && !quiet)
- die("No match");
- return 1;
- }
- return 0;
+ return cmd_show_ref__exclude_existing(exclude_existing_arg);
+ else if (verify)
+ return cmd_show_ref__verify(argv);
+ else
+ return cmd_show_ref__patterns(argv);
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH 01/12] builtin/show-ref: convert pattern to a local variable
From: Patrick Steinhardt @ 2023-10-24 13:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3886 bytes --]
The `pattern` variable is a global variable that tracks either the
reference names (not patterns!) for the `--verify` mode or the patterns
for the non-verify mode. This is a bit confusing due to the slightly
different meanings.
Convert the variable to be local. While this does not yet fix the double
meaning of the variable, this change allows us to address it in a
subsequent patch more easily by explicitly splitting up the different
subcommands of git-show-ref(1).
Note that we introduce a `struct show_ref_data` to pass the patterns to
`show_ref()`. While this is overengineered now, we will extend this
structure in a subsequent patch.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 46 ++++++++++++++++++++++++++++------------------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 5110814f796..7efab14b96c 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -20,7 +20,6 @@ static const char * const show_ref_usage[] = {
static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
quiet, hash_only, abbrev, exclude_arg;
-static const char **pattern;
static const char *exclude_existing_arg;
static void show_one(const char *refname, const struct object_id *oid)
@@ -50,15 +49,21 @@ static void show_one(const char *refname, const struct object_id *oid)
}
}
+struct show_ref_data {
+ const char **patterns;
+};
+
static int show_ref(const char *refname, const struct object_id *oid,
- int flag UNUSED, void *cbdata UNUSED)
+ int flag UNUSED, void *cbdata)
{
+ struct show_ref_data *data = cbdata;
+
if (show_head && !strcmp(refname, "HEAD"))
goto match;
- if (pattern) {
+ if (data->patterns) {
int reflen = strlen(refname);
- const char **p = pattern, *m;
+ const char **p = data->patterns, *m;
while ((m = *p++) != NULL) {
int len = strlen(m);
if (len > reflen)
@@ -180,6 +185,9 @@ static const struct option show_ref_options[] = {
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
+ struct show_ref_data show_ref_data = {0};
+ const char **patterns;
+
git_config(git_default_config, NULL);
argc = parse_options(argc, argv, prefix, show_ref_options,
@@ -188,38 +196,40 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
if (exclude_arg)
return exclude_existing(exclude_existing_arg);
- pattern = argv;
- if (!*pattern)
- pattern = NULL;
+ patterns = argv;
+ if (!*patterns)
+ patterns = NULL;
if (verify) {
- if (!pattern)
+ if (!patterns)
die("--verify requires a reference");
- while (*pattern) {
+ while (*patterns) {
struct object_id oid;
- if ((starts_with(*pattern, "refs/") || !strcmp(*pattern, "HEAD")) &&
- !read_ref(*pattern, &oid)) {
- show_one(*pattern, &oid);
+ if ((starts_with(*patterns, "refs/") || !strcmp(*patterns, "HEAD")) &&
+ !read_ref(*patterns, &oid)) {
+ show_one(*patterns, &oid);
}
else if (!quiet)
- die("'%s' - not a valid ref", *pattern);
+ die("'%s' - not a valid ref", *patterns);
else
return 1;
- pattern++;
+ patterns++;
}
return 0;
}
+ show_ref_data.patterns = patterns;
+
if (show_head)
- head_ref(show_ref, NULL);
+ head_ref(show_ref, &show_ref_data);
if (heads_only || tags_only) {
if (heads_only)
- for_each_fullref_in("refs/heads/", show_ref, NULL);
+ for_each_fullref_in("refs/heads/", show_ref, &show_ref_data);
if (tags_only)
- for_each_fullref_in("refs/tags/", show_ref, NULL);
+ for_each_fullref_in("refs/tags/", show_ref, &show_ref_data);
} else {
- for_each_ref(show_ref, NULL);
+ for_each_ref(show_ref, &show_ref_data);
}
if (!found_match) {
if (verify && !quiet)
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox