* [PATCH v2 03/12] t1414: convert test to use Git commands instead of writing refs manually
From: John Cai via GitGitGadget @ 2024-01-19 20:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, John Cai, John Cai
In-Reply-To: <pull.1647.v2.git.git.1705695540.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
This test can be re-written to use Git commands rather than writing a
manual ref in the reflog. This way this test no longer needs the
REFFILES prerequisite.
Signed-off-by: John Cai <johncai86@gmail.com>
---
t/t1414-reflog-walk.sh | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
index ea64cecf47b..be6c3f472c1 100755
--- a/t/t1414-reflog-walk.sh
+++ b/t/t1414-reflog-walk.sh
@@ -121,13 +121,12 @@ test_expect_success 'min/max age uses entry date to limit' '
# Create a situation where the reflog and ref database disagree about the latest
# state of HEAD.
-test_expect_success REFFILES 'walk prefers reflog to ref tip' '
+test_expect_success 'walk prefers reflog to ref tip' '
+ test_commit A &&
+ test_commit B &&
+ git reflog delete HEAD@{0} &&
head=$(git rev-parse HEAD) &&
- one=$(git rev-parse one) &&
- ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
- echo "$head $one $ident broken reflog entry" >>.git/logs/HEAD &&
-
- echo $one >expect &&
+ git rev-parse A >expect &&
git log -g --format=%H -1 >actual &&
test_cmp expect actual
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 02/12] remove REFFILES prerequisite for some tests in t1405 and t2017
From: John Cai via GitGitGadget @ 2024-01-19 20:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, John Cai, John Cai
In-Reply-To: <pull.1647.v2.git.git.1705695540.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
These tests are compatible with the reftable backend and thus do not
need the REFFILES prerequisite. Even though 53af25e4
(t1405: mark test that checks existence as REFFILES, 2022-01-31) and
53af25e4 (t1405: mark test that checks existence as REFFILES,
2022-01-31) marked these tests to require REFFILES, the reftable backend
in its current state does indeed work with these tests.
Signed-off-by: John Cai <johncai86@gmail.com>
---
t/t1405-main-ref-store.sh | 2 +-
t/t2017-checkout-orphan.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index e4627cf1b61..62c1eadb190 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -112,7 +112,7 @@ test_expect_success 'delete_reflog(HEAD)' '
test_must_fail git reflog exists HEAD
'
-test_expect_success REFFILES 'create-reflog(HEAD)' '
+test_expect_success 'create-reflog(HEAD)' '
$RUN create-reflog HEAD &&
git reflog exists HEAD
'
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
index 947d1587ac8..a5c7358eeab 100755
--- a/t/t2017-checkout-orphan.sh
+++ b/t/t2017-checkout-orphan.sh
@@ -86,7 +86,7 @@ test_expect_success '--orphan makes reflog by default' '
git rev-parse --verify delta@{0}
'
-test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
+test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
git checkout main &&
git config core.logAllRefUpdates false &&
git checkout --orphan epsilon &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 00/12] Group reffiles tests
From: John Cai via GitGitGadget @ 2024-01-19 20:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>
This series groups REFFILES specific tests together. These tests are
currently grouped together across the test suite based on functionality.
However, since they exercise low-level behavior specific to the refs backend
being used (in these cases, the ref-files backend), group them together
based on which refs backend they test. This way, in the near future when the
reftables backend gets upstreamed we can add tests that exercise the
reftables backend close by in the t06xx area.
These patches also remove the REFFILES prerequisite, since all the tests in
t06xx are reffiles specific. In the near future, once the reftable backend
is upstreamed, all the tests in t06xx will be forced to run with the
reffiles backend.
Changes since V1:
* Moved some pack-refs tests to t0601 instead of t0600
* Clarified some commit messages
* Converted a test to be refs-backend agnostic
* Other minor rearranging of tests
John Cai (12):
t3210: move to t0601
remove REFFILES prerequisite for some tests in t1405 and t2017
t1414: convert test to use Git commands instead of writing refs
manually
t1404: move reffiles specific tests to t0600
t1405: move reffiles specific tests to t0601
t1406: move reffiles specific tests to t0600
t1410: move reffiles specific tests to t0600
t1415: move reffiles specific tests to t0601
t1503: move reffiles specific tests to t0600
t3903: make drop stash test ref backend agnostic
t4202: move reffiles specific tests to t0600
t5312: move reffiles specific tests to t0601
t/t0600-reffiles-backend.sh | 384 ++++++++++++++++++
...ck-refs.sh => t0601-reffiles-pack-refs.sh} | 64 +++
t/t1404-update-ref-errors.sh | 237 -----------
t/t1405-main-ref-store.sh | 10 +-
t/t1407-worktree-ref-store.sh | 37 --
t/t1410-reflog.sh | 42 --
t/t1414-reflog-walk.sh | 11 +-
t/t1415-worktree-refs.sh | 11 -
t/t1503-rev-parse-verify.sh | 5 -
t/t2017-checkout-orphan.sh | 2 +-
t/t3903-stash.sh | 12 +-
t/t4202-log.sh | 17 -
t/t5312-prune-corruption.sh | 26 --
13 files changed, 461 insertions(+), 397 deletions(-)
create mode 100755 t/t0600-reffiles-backend.sh
rename t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} (81%)
base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1647%2Fjohn-cai%2Fjc%2Fgroup-reffiles-tests-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1647/john-cai/jc/group-reffiles-tests-v2
Pull-Request: https://github.com/git/git/pull/1647
Range-diff vs v1:
1: 0e2b6e197ab < -: ----------- t3210: move to t0602
-: ----------- > 1: ca65b9e6122 t3210: move to t0601
2: 624ad202305 ! 2: 29c32d3e6f7 remove REFFILES prerequisite
@@ Metadata
Author: John Cai <johncai86@gmail.com>
## Commit message ##
- remove REFFILES prerequisite
+ remove REFFILES prerequisite for some tests in t1405 and t2017
These tests are compatible with the reftable backend and thus do not
- need the REFFILES prerequisite.
+ need the REFFILES prerequisite. Even though 53af25e4
+ (t1405: mark test that checks existence as REFFILES, 2022-01-31) and
+ 53af25e4 (t1405: mark test that checks existence as REFFILES,
+ 2022-01-31) marked these tests to require REFFILES, the reftable backend
+ in its current state does indeed work with these tests.
Signed-off-by: John Cai <johncai86@gmail.com>
3: 19233aa0d44 ! 3: 122d19a9095 t1414: convert test to use Git commands instead of writing refs manually
@@ t/t1414-reflog-walk.sh: test_expect_success 'min/max age uses entry date to limi
- one=$(git rev-parse one) &&
- ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
- echo "$head $one $ident broken reflog entry" >>.git/logs/HEAD &&
-+ A=$(git rev-parse A) &&
-
+-
- echo $one >expect &&
-+ echo $A >expect &&
++ git rev-parse A >expect &&
git log -g --format=%H -1 >actual &&
test_cmp expect actual
'
4: 0f6fea6d32d ! 4: c3f0b81200c t1404: move reffiles specific tests to t0600
@@ t/t0600-reffiles-backend.sh (new)
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
-+# Test adding and deleting D/F-conflicting references in a single
-+# transaction.
-+df_test() {
-+ prefix="$1"
-+ pack=: symadd=false symdel=false add_del=false addref= delref=
-+ shift
-+ while test $# -gt 0
-+ do
-+ case "$1" in
-+ --pack)
-+ pack="git pack-refs --all"
-+ shift
-+ ;;
-+ --sym-add)
-+ # Perform the add via a symbolic reference
-+ symadd=true
-+ shift
-+ ;;
-+ --sym-del)
-+ # Perform the del via a symbolic reference
-+ symdel=true
-+ shift
-+ ;;
-+ --del-add)
-+ # Delete first reference then add second
-+ add_del=false
-+ delref="$prefix/r/$2"
-+ addref="$prefix/r/$3"
-+ shift 3
-+ ;;
-+ --add-del)
-+ # Add first reference then delete second
-+ add_del=true
-+ addref="$prefix/r/$2"
-+ delref="$prefix/r/$3"
-+ shift 3
-+ ;;
-+ *)
-+ echo 1>&2 "Extra args to df_test: $*"
-+ return 1
-+ ;;
-+ esac
-+ done
-+ git update-ref "$delref" $C &&
-+ if $symadd
++if ! test_have_prereq REFFILES
+ then
-+ addname="$prefix/s/symadd" &&
-+ git symbolic-ref "$addname" "$addref"
-+ else
-+ addname="$addref"
-+ fi &&
-+ if $symdel
-+ then
-+ delname="$prefix/s/symdel" &&
-+ git symbolic-ref "$delname" "$delref"
-+ else
-+ delname="$delref"
-+ fi &&
-+ cat >expected-err <<-EOF &&
-+ fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
-+ EOF
-+ $pack &&
-+ if $add_del
-+ then
-+ printf "%s\n" "create $addname $D" "delete $delname"
-+ else
-+ printf "%s\n" "delete $delname" "create $addname $D"
-+ fi >commands &&
-+ test_must_fail git update-ref --stdin <commands 2>output.err &&
-+ test_cmp expected-err output.err &&
-+ printf "%s\n" "$C $delref" >expected-refs &&
-+ git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
-+ test_cmp expected-refs actual-refs
-+}
++ skip_all='skipping reffiles specific tests'
++ test_done
++fi
+
+test_expect_success 'setup' '
+ git commit --allow-empty -m Initial &&
@@ t/t0600-reffiles-backend.sh (new)
+ git update-ref --stdin
+'
+
-+test_expect_success 'D/F conflict prevents add long + delete short' '
-+ df_test refs/df-al-ds --add-del foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents add short + delete long' '
-+ df_test refs/df-as-dl --add-del foo foo/bar
-+'
-+
-+test_expect_success 'D/F conflict prevents delete long + add short' '
-+ df_test refs/df-dl-as --del-add foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents delete short + add long' '
-+ df_test refs/df-ds-al --del-add foo foo/bar
-+'
-+
-+test_expect_success 'D/F conflict prevents add long + delete short packed' '
-+ df_test refs/df-al-dsp --pack --add-del foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents add short + delete long packed' '
-+ df_test refs/df-as-dlp --pack --add-del foo foo/bar
-+'
-+
-+test_expect_success 'D/F conflict prevents delete long packed + add short' '
-+ df_test refs/df-dlp-as --pack --del-add foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents delete short packed + add long' '
-+ df_test refs/df-dsp-al --pack --del-add foo foo/bar
-+'
-+
-+# Try some combinations involving symbolic refs...
-+
-+test_expect_success 'D/F conflict prevents indirect add long + delete short' '
-+ df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short' '
-+ df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents indirect add short + indirect delete long' '
-+ df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
-+'
-+
-+test_expect_success 'D/F conflict prevents indirect delete long + indirect add short' '
-+ df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents indirect add long + delete short packed' '
-+ df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short packed' '
-+ df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents add long + indirect delete short packed' '
-+ df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
-+'
-+
-+test_expect_success 'D/F conflict prevents indirect delete long packed + indirect add short' '
-+ df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
-+'
-+
+test_expect_success 'non-empty directory blocks create' '
+ prefix=refs/ne-create &&
+ mkdir -p .git/$prefix/foo/bar &&
@@ t/t0600-reffiles-backend.sh (new)
+test_done
## t/t1404-update-ref-errors.sh ##
-@@ t/t1404-update-ref-errors.sh: test_update_rejected () {
- test_cmp unchanged actual
- }
-
--# Test adding and deleting D/F-conflicting references in a single
--# transaction.
--df_test() {
-- prefix="$1"
-- pack=: symadd=false symdel=false add_del=false addref= delref=
-- shift
-- while test $# -gt 0
-- do
-- case "$1" in
-- --pack)
-- pack="git pack-refs --all"
-- shift
-- ;;
-- --sym-add)
-- # Perform the add via a symbolic reference
-- symadd=true
-- shift
-- ;;
-- --sym-del)
-- # Perform the del via a symbolic reference
-- symdel=true
-- shift
-- ;;
-- --del-add)
-- # Delete first reference then add second
-- add_del=false
-- delref="$prefix/r/$2"
-- addref="$prefix/r/$3"
-- shift 3
-- ;;
-- --add-del)
-- # Add first reference then delete second
-- add_del=true
-- addref="$prefix/r/$2"
-- delref="$prefix/r/$3"
-- shift 3
-- ;;
-- *)
-- echo 1>&2 "Extra args to df_test: $*"
-- return 1
-- ;;
-- esac
-- done
-- git update-ref "$delref" $C &&
-- if $symadd
-- then
-- addname="$prefix/s/symadd" &&
-- git symbolic-ref "$addname" "$addref"
-- else
-- addname="$addref"
-- fi &&
-- if $symdel
-- then
-- delname="$prefix/s/symdel" &&
-- git symbolic-ref "$delname" "$delref"
-- else
-- delname="$delref"
-- fi &&
-- cat >expected-err <<-EOF &&
-- fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
-- EOF
-- $pack &&
-- if $add_del
-- then
-- printf "%s\n" "create $addname $D" "delete $delname"
-- else
-- printf "%s\n" "delete $delname" "create $addname $D"
-- fi >commands &&
-- test_must_fail git update-ref --stdin <commands 2>output.err &&
-- test_cmp expected-err output.err &&
-- printf "%s\n" "$C $delref" >expected-refs &&
-- git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
-- test_cmp expected-refs actual-refs
--}
--
- test_expect_success 'setup' '
-
- git commit --allow-empty -m Initial &&
@@ t/t1404-update-ref-errors.sh: test_expect_success 'one new ref is a simple prefix of another' '
'
@@ t/t1404-update-ref-errors.sh: test_expect_success 'one new ref is a simple prefi
- git update-ref --stdin
-'
-
--test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
-- df_test refs/df-al-ds --add-del foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents add short + delete long' '
-- df_test refs/df-as-dl --add-del foo foo/bar
--'
--
--test_expect_success REFFILES 'D/F conflict prevents delete long + add short' '
-- df_test refs/df-dl-as --del-add foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents delete short + add long' '
-- df_test refs/df-ds-al --del-add foo foo/bar
--'
--
--test_expect_success REFFILES 'D/F conflict prevents add long + delete short packed' '
-- df_test refs/df-al-dsp --pack --add-del foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents add short + delete long packed' '
-- df_test refs/df-as-dlp --pack --add-del foo foo/bar
--'
--
--test_expect_success REFFILES 'D/F conflict prevents delete long packed + add short' '
-- df_test refs/df-dlp-as --pack --del-add foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents delete short packed + add long' '
-- df_test refs/df-dsp-al --pack --del-add foo foo/bar
--'
--
--# Try some combinations involving symbolic refs...
--
--test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short' '
-- df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short' '
-- df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents indirect add short + indirect delete long' '
-- df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
--'
--
--test_expect_success REFFILES 'D/F conflict prevents indirect delete long + indirect add short' '
-- df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short packed' '
-- df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short packed' '
-- df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents add long + indirect delete short packed' '
-- df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
--'
--
--test_expect_success REFFILES 'D/F conflict prevents indirect delete long packed + indirect add short' '
-- df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
--'
--
- # Test various errors when reading the old values of references...
-
- test_expect_success 'missing old value blocks update' '
+ test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
+ df_test refs/df-al-ds --add-del foo/bar foo
+ '
@@ t/t1404-update-ref-errors.sh: test_expect_success 'incorrect old value blocks indirect no-deref delete' '
test_cmp expected output.err
'
5: c2af695f551 ! 5: 42dc9948aa5 t1405: move reffiles specific tests to t0600
@@ Metadata
Author: John Cai <johncai86@gmail.com>
## Commit message ##
- t1405: move reffiles specific tests to t0600
+ t1405: move reffiles specific tests to t0601
- Move this test to t0600 with other reffiles specific tests since it is
- reffiles specific in that it looks into the loose refs directory for an
- assertion.
+ Move this test to t0601 with other reffiles specific pack-refs tests
+ since it is reffiles specific in that it looks into the loose refs
+ directory for an assertion.
Signed-off-by: John Cai <johncai86@gmail.com>
- ## t/t0600-reffiles-backend.sh ##
-@@ t/t0600-reffiles-backend.sh: test_expect_success 'setup' '
- E=$(git rev-parse HEAD)
+ ## t/t0601-reffiles-pack-refs.sh ##
+@@ t/t0601-reffiles-pack-refs.sh: test_expect_success 'prepare a trivial repository' '
+ HEAD=$(git rev-parse --verify HEAD)
'
+test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
@@ t/t0600-reffiles-backend.sh: test_expect_success 'setup' '
+ test -z "$N"
+'
+
- test_expect_success 'empty directory should not fool rev-parse' '
- prefix=refs/e-rev-parse &&
- git update-ref $prefix/foo $C &&
+ SHA1=
+
+ test_expect_success 'see if git show-ref works as expected' '
## t/t1405-main-ref-store.sh ##
@@ t/t1405-main-ref-store.sh: test_expect_success 'setup' '
6: 69ea950cfea = 6: 98e40a024b9 t1406: move reffiles specific tests to t0600
7: ae71747871c = 7: d93c9c410b9 t1410: move reffiles specific tests to t0600
8: 9d105263695 ! 8: 8327b12a313 t1415: move reffiles specific tests to t0600
@@ Metadata
Author: John Cai <johncai86@gmail.com>
## Commit message ##
- t1415: move reffiles specific tests to t0600
+ t1415: move reffiles specific tests to t0601
- Move this test into t0600 with other reffiles specific tests since it
- checks for individua loose refs and thus is specific to the reffiles
- backend.
+ Move this test into t0601 with other reffiles pack-refs specific tests
+ since it checks for individua loose refs and thus is specific to the
+ reffiles backend.
Signed-off-by: John Cai <johncai86@gmail.com>
- ## t/t0600-reffiles-backend.sh ##
-@@ t/t0600-reffiles-backend.sh: test_expect_success 'empty reflog' '
- test_must_be_empty err
+ ## t/t0601-reffiles-pack-refs.sh ##
+@@ t/t0601-reffiles-pack-refs.sh: test_expect_success SYMLINKS 'pack symlinked packed-refs' '
+ test "$(test_readlink .git/packed-refs)" = "my-deviant-packed-refs"
'
+# The 'packed-refs' file is stored directly in .git/. This means it is global
9: dcec7f10ab6 ! 9: 891a3d057d2 t1503: move reffiles specific tests to t0600
@@ Commit message
Signed-off-by: John Cai <johncai86@gmail.com>
## t/t0600-reffiles-backend.sh ##
-@@ t/t0600-reffiles-backend.sh: test_expect_success 'refs/worktree must not be packed' '
- test_path_is_file .git/worktrees/wt2/refs/worktree/foo
+@@ t/t0600-reffiles-backend.sh: test_expect_success 'empty reflog' '
+ test_must_be_empty err
'
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
10: 56a9c8f20dd < -: ----------- t3903: move reffiles specific tests to t0600
-: ----------- > 10: bfd5b403170 t3903: make drop stash test ref backend agnostic
11: 39e69fde3d7 ! 11: 976be7efc89 t4202: move reffiles specific tests to t0600
@@ Commit message
Signed-off-by: John Cai <johncai86@gmail.com>
## t/t0600-reffiles-backend.sh ##
-@@ t/t0600-reffiles-backend.sh: test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
- test_cmp expect actual
+@@ t/t0600-reffiles-backend.sh: test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+ test_must_fail git rev-parse --verify broken
'
+test_expect_success 'log diagnoses bogus HEAD hash' '
12: 316a20ed179 ! 12: 7329e87148a t5312: move reffiles specific tests to t0600
@@ Metadata
Author: John Cai <johncai86@gmail.com>
## Commit message ##
- t5312: move reffiles specific tests to t0600
+ t5312: move reffiles specific tests to t0601
- Move a few tests into t0600 since they specifically test the packed-refs
+ Move a few tests into t0601 since they specifically test the packed-refs
file and thus are specific to the reffiles backend.
Signed-off-by: John Cai <johncai86@gmail.com>
- ## t/t0600-reffiles-backend.sh ##
-@@ t/t0600-reffiles-backend.sh: test_expect_success 'log diagnoses bogus HEAD symref' '
- test_grep broken stderr
+ ## t/t0601-reffiles-pack-refs.sh ##
+@@ t/t0601-reffiles-pack-refs.sh: test_expect_success 'refs/worktree must not be packed' '
+ test_path_is_file .git/worktrees/wt2/refs/worktree/foo
'
+# we do not want to count on running pack-refs to
@@ t/t0600-reffiles-backend.sh: test_expect_success 'log diagnoses bogus HEAD symre
+ test_cmp expect actual
+'
+
-+test_expect_success 'pack-refs does not drop broken refs during deletion' '
++test_expect_success 'pack-refs does not drop broken refs during deletion' '
+ git update-ref -d refs/heads/other &&
+ git rev-parse refs/heads/main >actual &&
+ test_cmp expect actual
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 01/12] t3210: move to t0601
From: John Cai via GitGitGadget @ 2024-01-19 20:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, John Cai, John Cai
In-Reply-To: <pull.1647.v2.git.git.1705695540.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
Move t3210 to t0601, since these tests are reffiles specific in that
they modify loose refs manually. This is part of the effort to
categorize these tests together based on the ref backend they test. When
we upstream the reftable backend, we can add more tests to t06xx. This
way, all tests that test specific ref backend behavior will be grouped
together.
Signed-off-by: John Cai <johncai86@gmail.com>
---
t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} | 6 ++++++
1 file changed, 6 insertions(+)
rename t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} (98%)
diff --git a/t/t3210-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
similarity index 98%
rename from t/t3210-pack-refs.sh
rename to t/t0601-reffiles-pack-refs.sh
index 7f4e98db7db..f7a3f693901 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -15,6 +15,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
+if ! test_have_prereq REFFILES
+ then
+ skip_all='skipping reffiles specific tests'
+ test_done
+fi
+
test_expect_success 'enable reflogs' '
git config core.logallrefupdates true
'
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs
From: Junio C Hamano @ 2024-01-19 20:09 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <abcf1f5cf428072d19639dc4209e0c1554f3eb53.1705659748.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> We're about to convert the MERGE_AUTOSTASH ref to become non-special,
> using the refs API instead of direct filesystem access to both read and
> write the ref. The current interfaces to write autostashes is entirely
> path-based though, so we need to extend them to also support writes via
> the refs API instead.
>
> Ideally, we would be able to fully replace the old set of path-based
> interfaces. But the sequencer will continue to write state into
> "rebase-merge/autostash". This path is not considered to be a ref at all
> and will thus stay is-is for now, which requires us to keep both path-
"is-is"???
> and refs-based interfaces to handle autostashes.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> sequencer.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----
> sequencer.h | 3 +++
> 2 files changed, 64 insertions(+), 5 deletions(-)
The conversion (rather, the introduction to allow refs API to be
used to access them) look correct, but offhand I do not know what
the implication of leaving the file based interface would be.
Thanks.
^ permalink raw reply
* Re: [PATCH v3] merge-ll: expose revision names to custom drivers
From: Antonin Delpeuch @ 2024-01-19 20:02 UTC (permalink / raw)
To: Antonin Delpeuch via GitGitGadget, git; +Cc: Junio C Hamano
In-Reply-To: <pull.1648.v3.git.git.1705615794307.gitgitgadget@gmail.com>
Hi Junio,
After more testing (combining custom merge drivers with rerere) I
realized that my patch can lead to a segmentation error. Many apologies
for not having caught that earlier!
On 18/01/2024 23:09, Antonin Delpeuch via GitGitGadget wrote:
> @@ -222,6 +222,12 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
> strbuf_addf(&cmd, "%d", marker_size);
> else if (skip_prefix(format, "P", &format))
> sq_quote_buf(&cmd, path);
> + else if (skip_prefix(format, "S", &format))
> + sq_quote_buf(&cmd, orig_name);
> + else if (skip_prefix(format, "X", &format))
> + sq_quote_buf(&cmd, name1);
> + else if (skip_prefix(format, "Y", &format))
> + sq_quote_buf(&cmd, name2);
The "orig_name", "name1" and "name2" pointers can be NULL at this stage.
This can happen when the merge is invoked from rerere, to resolve a
conflict using a previous resolution.
I wonder what the appropriate fallback would be in such a case. I am
tempted to use the temporary filenames of the files to merge instead, so
that the merge driver can rely on those names being non-empty and being
the best string to use to identify the files. Passing an empty string
seems dangerous to me, as it is likely to change the index of arguments
passed to the merge driver. Passing fixed strings such as "base", "ours"
and "theirs" could perhaps work too.
Let me know if you have any preference about this.
Best,
Antonin
^ permalink raw reply
* Re: [PATCH 3/7] refs: convert AUTO_MERGE to become a normal pseudo-ref
From: Junio C Hamano @ 2024-01-19 19:28 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Elijah Newren
In-Reply-To: <ae2df6316f79e372c51d59666d685e59981d2f98.1705659748.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have
> inrtoduced a new `is_special_ref()` function that classifies some refs
"introduced"
> @@ -4687,10 +4685,17 @@ void merge_switch_to_result(struct merge_options *opt,
> trace2_region_leave("merge", "record_conflicted", opt->repo);
>
> trace2_region_enter("merge", "write_auto_merge", opt->repo);
> - filename = git_path_auto_merge(opt->repo);
> - fp = xfopen(filename, "w");
> - fprintf(fp, "%s\n", oid_to_hex(&result->tree->object.oid));
> - fclose(fp);
> + if (refs_update_ref(get_main_ref_store(opt->repo), "", "AUTO_MERGE",
> + &result->tree->object.oid, NULL, REF_NO_DEREF,
> + UPDATE_REFS_MSG_ON_ERR)) {
> + /* failure to function */
> + opt->priv = NULL;
> + result->clean = -1;
> + merge_finalize(opt, result);
> + trace2_region_leave("merge", "write_auto_merge",
> + opt->repo);
> + return;
> + }
> trace2_region_leave("merge", "write_auto_merge", opt->repo);
> }
> if (display_update_msgs)
We used to ignore errors while updating AUTO_MERGE, implying that it
is an optional nicety that does not have to block the merge. Now we
explicitly mark the resulting index unclean. While my gut feeling
says that it should not matter all that much (as such a failure
would be rare enough that the user may want to inspect and double
check the situation before going forward), I am not 100% sure if the
change is behaviour is acceptable by everybody (cc'ed Elijah for
second opinion).
Everything else in this patch looked good.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/7] sequencer: clean up pseudo refs with REF_NO_DEREF
From: Junio C Hamano @ 2024-01-19 19:14 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <48b95fe954c1dbdd080ce7a0cc871f4850bddeae.1705659748.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> When cleaning up the state-tracking pseudorefs CHERRY_PICK_HEAD or
> REVERT_HEAD we do not set REF_NO_DEREF. In the unlikely case where those
> refs are a symref we would thus end up deleting the symref targets, and
> not the symrefs themselves.
>
> Harden the code to use REF_NO_DEREF to fix this.
This level of attention to detail is very much appreciated. I
suspect that this was inspired by the other topic we discussed the
other day to tighten accesses to {MERGE,CHERRY_PICK,REVERT}_HEAD
that is done via repo_get_oid() to the one based on read_ref_full()?
^ permalink raw reply
* RE: [PATCH v2 3/4] config: factor out global config file retrieval
From: rsbecker @ 2024-01-19 18:59 UTC (permalink / raw)
To: 'Junio C Hamano', 'Kristoffer Haugsbakk'
Cc: 'Patrick Steinhardt', git, stolee,
'Eric Sunshine', 'Taylor Blau'
In-Reply-To: <xmqq34utkw6i.fsf@gitster.g>
On Friday, January 19, 2024 1:36 PM, Junio C Hamano wrote:
>"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
>
>> On Fri, Jan 19, 2024, at 07:18, Patrick Steinhardt wrote:
>>> But second, I think that the new function you introduce here has the
>>> same issue as the old function that you refactored in the preceding
>>> patch: `git_config_global()` isn't very descriptive, and it is also
>>> inconsistent the new `git_config_global_paths()`. I'd propose to name
>>> the new function something like `git_config_global_preferred_path()`
>>> or `git_config_global_path()`.
>>
>> The choice of `git_config_global` is mostly motivated by it working
>> the same way as `git_config_system`:
>>
>> ```
>> given_config_source.file = git_system_config(); […]
>> given_config_source.file = git_global_config(); ```
>
>I shared the above understanding with you, so I didn't find the name "not very
>descriptive" during my review. If only we had two more functions that can replace
>our uses of repo_git_path(r, "config") and repo_git_path(r, "config.worktree") [*] in
>the code, to obtain the path to the repository local and worktree local configuration
>files, the convention may have been more obvious.
>
> Side note: the worktree specific one is messier; there are code
> paths that use "%s/config.worktree" on gitdir as well---if we
> were to introduce helpers, we should catch and convert them, too.
>
>> Your suggestion makes sense. But should `git_system_config` be renamed
>> as well?
>
>I do not mind including "path" in the names of these functions, but I do agree that
>such renaming should be done consistently across the family of functions (which
>we currently have only two members, but still).
Is this going to impact the libification effort? I am just curious what other on the team think.
--Randall
^ permalink raw reply
* Re: [PATCH] precious-files.txt: new document proposing new precious file type
From: Junio C Hamano @ 2024-01-19 18:45 UTC (permalink / raw)
To: Sebastian Thiel
Cc: Elijah Newren, Elijah Newren via GitGitGadget, git, Josh Triplett,
Phillip Wood
In-Reply-To: <4421D442-1BB2-4C89-834F-9E70F4CF360B@icloud.com>
Sebastian Thiel <sebastian.thiel@icloud.com> writes:
> I am glad I can pull my initial proposition of 'having both syntaxes' off
> the table to side with this version - it's gorgeous.
>
> It's easy to forget that the search-order when matching ignore patterns
> is back to front, which makes this 'trick' work.
The true gem is not the search-order, though. It is the "last one
wins" rule. Back to front search is merely an implementation detail
to optimize the search so that we can stop at the first hit ;-)
> If the insights gained with the last couple of emails would see their digest
> in the user-facing documentation, I think precious files wouldn't only become
> usable but would also allow projects to make the their choice during
> the transition period during which some users will inevitably access the repository
> with a Git that doesn't know about precious files yet.
OK.
^ permalink raw reply
* Re: [PATCH v2 3/4] config: factor out global config file retrieval
From: Junio C Hamano @ 2024-01-19 18:36 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: Patrick Steinhardt, git, stolee, Eric Sunshine, Taylor Blau
In-Reply-To: <7f0864ad-c846-42a6-8ddc-85d6be58a4ee@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> On Fri, Jan 19, 2024, at 07:18, Patrick Steinhardt wrote:
>> But second, I think that the new function you introduce here has the
>> same issue as the old function that you refactored in the preceding
>> patch: `git_config_global()` isn't very descriptive, and it is also
>> inconsistent the new `git_config_global_paths()`. I'd propose to name
>> the new function something like `git_config_global_preferred_path()` or
>> `git_config_global_path()`.
>
> The choice of `git_config_global` is mostly motivated by it working the
> same way as `git_config_system`:
>
> ```
> given_config_source.file = git_system_config();
> […]
> given_config_source.file = git_global_config();
> ```
I shared the above understanding with you, so I didn't find the name
"not very descriptive" during my review. If only we had two more
functions that can replace our uses of repo_git_path(r, "config")
and repo_git_path(r, "config.worktree") [*] in the code, to obtain
the path to the repository local and worktree local configuration
files, the convention may have been more obvious.
Side note: the worktree specific one is messier; there are code
paths that use "%s/config.worktree" on gitdir as well---if we
were to introduce helpers, we should catch and convert them, too.
> Your suggestion makes sense. But should `git_system_config` be renamed as
> well?
I do not mind including "path" in the names of these functions, but
I do agree that such renaming should be done consistently across the
family of functions (which we currently have only two members, but
still).
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/5] completion: improvements for git-bisect
From: Junio C Hamano @ 2024-01-19 18:27 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Britton Leo Kerin, git
In-Reply-To: <ZaofJhHsFjRxx7a3@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> On Thu, Jan 18, 2024 at 11:43:18AM -0900, Britton Leo Kerin wrote:
>> CC to add: CC: Junio C Hamano <gitster@pobox.com>, Patrick Steinhardt <ps@pks.im>
>>
>> Relative to v2 this only changes a wrong misleading commit message.
>>
>> Bring completion for bisect up to date with current features.
>
> Thanks for your patches! I've got a few comments to bring them more in
> line with how we're doing things in the Git project, but I agree with
> the overall direction that they are going into.
>
> It might be a good idea to also add a few tests in t9902-completion.sh
> to ensure that the newly introduced completions work as expected.
>
> Thanks!
>
> Patrick
And thanks for your review. Very much appreciated.
^ permalink raw reply
* Re: [PATCH v2 3/4] t7450: test submodule urls
From: Junio C Hamano @ 2024-01-19 18:16 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Victoria Dye via GitGitGadget, git, Jeff King, Victoria Dye
In-Reply-To: <ZaoRDniGoIBXmjVx@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
>> Add two tests to separately address cases where the URL check correctly
>> filters out invalid URLs and cases where the check misses invalid URLs. Mark
>> the latter ("url check misses invalid cases") with 'test_expect_failure' to
>> indicate that this not the undesired behavior.
>
> Nit: this should probably say "to indicate that this is not the desired
> behaviour." But given that the other patches in this series look good to
> me I don't think this warrants a reroll.
Good eyes.
I'll rewrite that part to "... to indicate that this is currently
broken, which will be fixed in the next step." before merging the
series to 'next'.
Thanks.
^ permalink raw reply
* Re: [PATCH] precious-files.txt: new document proposing new precious file type
From: Junio C Hamano @ 2024-01-19 17:17 UTC (permalink / raw)
To: Phillip Wood
Cc: Elijah Newren, Sebastian Thiel, Elijah Newren via GitGitGadget,
git, Josh Triplett
In-Reply-To: <7fc35078-a165-4b3c-96e2-37fbe55e109d@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> If you've added a secret then catching it after you've published the
> patch for review is likely to be too late. I agree there are a couple
> of chances to catch it before that though.
Yes, this is one of the two remaining things that still make me a
bit worried about the "$.config" syntax.
> Indeed, though "git clean" requires the user to pass a flag before it
> will delete anything does have a dry-run mode to check what's going to
> happen so there is an opportunity for users to avoid accidental
> deletions.
True, too.
The other one that still make me a bit worried about the "$.config"
syntax is what I called "the devil you already know" that is
applicable only for participants of a project that currently mark
precious files as ignored, to avoid the accidental "git add ." of
secrets.
I think we already are in agreement that all other points (aside
from possible ergonomics preferences and future extensibility, both
feel a lot minor) raised during this discussion are in favor of the
"$.config" syntax.
> While I can see it would be helpful to settle the syntax question I
> think parsing the new syntax is a relatively small part of the work
> that needs to be done to implement precious files.
True. The parser can be isolated and it should be relatively easy
to revamp. My current preference is to (at least) tentatively agree
on using the "$.config" syntax, which would allow us to update
dir.c:parse_path_pattern(), and that would make it possible for us
to start adjusting dir.c:is_excluded(), adding is_precious() next to
it, and adjusting all current callers of the former.
Thanks.
^ permalink raw reply
* Re: [PATCH] precious-files.txt: new document proposing new precious file type
From: Phillip Wood @ 2024-01-19 16:53 UTC (permalink / raw)
To: Elijah Newren, Junio C Hamano
Cc: Sebastian Thiel, Elijah Newren via GitGitGadget, git,
Josh Triplett
In-Reply-To: <CABPp-BHaUDdtH6igDmOx_wv8xYh-uA=4L9zDDycrZLaa9c9KLQ@mail.gmail.com>
Hi Elijah
On 19/01/2024 02:58, Elijah Newren wrote:
> On Thu, Jan 18, 2024 at 11:14 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
> [...]
>> So, all it boils down to is these two questions.
>
> Thanks for summarizing this.
Yes, thank you Junio - I found it very helpful as well
>> * Which one between "'git add .' adds '.config' that users did not
>> want to add" and "'git clean -f' removes '.config' together with
>> other files" a larger problem to the users, who participate in a
>> project that already decided to use the new .gitignore feature to
>> mark ".config" as "precious", of older versions of Git that
>> predate "precious"?
>
> Accidental "git add ." comes with 3 opportunities to correct the
> problem before it becomes permanent: before commiting, after
> committing but before pushing, and after publishing for patch review
> (where it can even be caught by third parties) but before the
> patch/PR/MR is accepted and included. At each stage there's a chance
> to go back and correct the problem.
If you've added a secret then catching it after you've published the
patch for review is likely to be too late. I agree there are a couple of
chances to catch it before that though.
> Accidental nuking of a file (via either git clean or git checkout or
> git merge or whatever), cannot be reviewed or corrected; it's
> immediately too late.
Indeed, though "git clean" requires the user to pass a flag before it
will delete anything does have a dry-run mode to check what's going to
happen so there is an opportunity for users to avoid accidental deletions.
> [...]
> However, on a closely related note, in my response to Sebastian I
> point out that the '$' syntax permits individual teams to prioritize
> avoiding either accidental deletions or accidental adds on a filename
> or glob granularity, so if folks are concerned with handling by older
> Git versions or are just extra concerned with certain files, they can
> optimize accordingly.
That is an advantage. I do worry that the '$' syntax is unintuitive and
will further add to the impression that git is hard to use. I think the
choice comes down how much we are worried about the way older versions
of git treat ".gitignore" files with the new syntax.
While I can see it would be helpful to settle the syntax question I
think parsing the new syntax is a relatively small part of the work that
needs to be done to implement precious files.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH 10/12] t3903: move reffiles specific tests to t0600
From: John Cai @ 2024-01-19 15:47 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: John Cai via GitGitGadget, git
In-Reply-To: <Zap7jfZlwlm-UZ1X@tanuki>
Hi Patrick,
On 19 Jan 2024, at 8:39, Patrick Steinhardt wrote:
> On Wed, Jan 17, 2024 at 07:52:33PM +0000, John Cai via GitGitGadget wrote:
>> From: John Cai <johncai86@gmail.com>
>>
>> Move this test into t0600 with other reffiles specific tests since it
>> modifies reflog refs manually and thus is specific to the reffiles
>> backend.
>>
>> This change also consolidates setup_stash() into test-lib-functions.sh
>>
>> Signed-off-by: John Cai <johncai86@gmail.com>
>> ---
>> t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
>> t/t3903-stash.sh | 43 -------------------------------------
>> t/test-lib-functions.sh | 16 ++++++++++++++
>> 3 files changed, 43 insertions(+), 43 deletions(-)
>>
>> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
>> index 704b73fdc54..bee61b2d19d 100755
>> --- a/t/t0600-reffiles-backend.sh
>> +++ b/t/t0600-reffiles-backend.sh
>> @@ -527,4 +527,31 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
>> test_must_fail git rev-parse --verify broken
>> '
>>
>> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
>> + git init repo &&
>> + (
>> + cd repo &&
>> + setup_stash
>> + ) &&
>> + echo 9 >repo/file &&
>> +
>> + old_oid="$(git -C repo rev-parse stash@{0})" &&
>> + git -C repo stash &&
>> + new_oid="$(git -C repo rev-parse stash@{0})" &&
>> +
>> + cat >expect <<-EOF &&
>> + $(test_oid zero) $old_oid
>> + $old_oid $new_oid
>> + EOF
>> + cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
>> + test_cmp expect actual &&
>> +
>> + git -C repo stash drop stash@{1} &&
>> + cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
>> + cat >expect <<-EOF &&
>> + $(test_oid zero) $new_oid
>> + EOF
>> + test_cmp expect actual
>> +'
>
> I think that there is no need to make this backend-specific. What we're
> testing here is that `git stash drop` is able to drop the latest reflog
> entry. The calls to cut(1) are only used to verify that the contents of
> the reflog entry look as expected while only verifying the old and new
> object IDs.
>
> So how about below patch to make it generic instead?
Nice catch. This sounds perfect to me.
>
> Patrick
>
> -- >8 --
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 34faeac3f1..3319240515 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -200,7 +200,7 @@ test_expect_success 'drop stash reflog updates refs/stash' '
> test_cmp expect actual
> '
>
> -test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
> git init repo &&
> (
> cd repo &&
> @@ -213,16 +213,16 @@ test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite'
> new_oid="$(git -C repo rev-parse stash@{0})" &&
>
> cat >expect <<-EOF &&
> - $(test_oid zero) $old_oid
> - $old_oid $new_oid
> + $new_oid
> + $old_oid
> EOF
> - cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> + git -C repo reflog show refs/stash --format=%H >actual &&
> test_cmp expect actual &&
>
> git -C repo stash drop stash@{1} &&
> - cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> + git -C repo reflog show refs/stash --format=%H >actual &&
> cat >expect <<-EOF &&
> - $(test_oid zero) $new_oid
> + $new_oid
> EOF
> test_cmp expect actual
> '
^ permalink raw reply
* [PATCH 5/5] for-each-ref: avoid filtering on empty pattern
From: Karthik Nayak @ 2024-01-19 14:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20240119142705.139374-1-karthik.188@gmail.com>
When the user uses an empty string pattern (""), we don't match any refs
in git-for-each-ref(1). This is because in git-for-each-ref(1), we use
path based matching and an empty string doesn't match any path.
In this commit we change this behavior by making empty string pattern
match all references. This is done by introducing a new flag
`FILTER_REFS_NO_FILTER` in `ref-filter.c`, which uses the newly
introduced `refs_for_each_all_refs()` function to iterate over all the
refs in the repository.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/git-for-each-ref.txt | 3 ++-
builtin/for-each-ref.c | 21 +++++++++++++++++-
ref-filter.c | 13 ++++++++++--
ref-filter.h | 4 +++-
t/t6302-for-each-ref-filter.sh | 34 ++++++++++++++++++++++++++++++
5 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index be9543f684..b1cb482bf5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -32,7 +32,8 @@ OPTIONS
If one or more patterns are given, only refs are shown that
match against at least one pattern, either using fnmatch(3) or
literally, in the latter case matching completely or from the
- beginning up to a slash.
+ beginning up to a slash. If an empty string is provided all refs
+ are printed, including HEAD and pseudorefs.
--stdin::
If `--stdin` is supplied, then the list of patterns is read from
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3885a9c28e..5aa879e8be 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -25,6 +25,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
struct ref_format format = REF_FORMAT_INIT;
int from_stdin = 0;
struct strvec vec = STRVEC_INIT;
+ unsigned int flags = FILTER_REFS_ALL;
struct option opts[] = {
OPT_BIT('s', "shell", &format.quote_style,
@@ -93,11 +94,29 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
/* vec.v is NULL-terminated, just like 'argv'. */
filter.name_patterns = vec.v;
} else {
+ size_t i;
+
filter.name_patterns = argv;
+
+ /*
+ * Search for any empty string pattern, if it exists then we
+ * print all refs without any filtering.
+ */
+ i = 0;
+ while (argv[i]) {
+ if (!argv[i][0]) {
+ flags = FILTER_REFS_NO_FILTER;
+ /* doing this removes any pattern from being matched */
+ filter.name_patterns[0] = NULL;
+ break;
+ }
+
+ i++;
+ }
}
filter.match_as_path = 1;
- filter_and_format_refs(&filter, FILTER_REFS_ALL, sorting, &format);
+ filter_and_format_refs(&filter, flags, sorting, &format);
ref_filter_clear(&filter);
ref_sorting_release(sorting);
diff --git a/ref-filter.c b/ref-filter.c
index 35b989e1df..6dac133b87 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2622,6 +2622,11 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
each_ref_fn cb,
void *cb_data)
{
+ if (filter->kind & FILTER_REFS_NO_FILTER) {
+ return refs_for_each_all_refs(
+ get_main_ref_store(the_repository), cb, cb_data);
+ }
+
if (!filter->match_as_path) {
/*
* in this case, the patterns are applied after
@@ -2775,8 +2780,12 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const struct
/* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */
kind = filter_ref_kind(filter, refname);
- if (!(kind & filter->kind))
+ if (filter->kind & FILTER_REFS_NO_FILTER) {
+ if (kind == FILTER_REFS_DETACHED_HEAD)
+ kind = FILTER_REFS_OTHERS;
+ } else if (!(kind & filter->kind)) {
return NULL;
+ }
if (!filter_pattern_match(filter, refname))
return NULL;
@@ -3041,7 +3050,7 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
ret = for_each_fullref_in("refs/remotes/", fn, cb_data);
else if (filter->kind == FILTER_REFS_TAGS)
ret = for_each_fullref_in("refs/tags/", fn, cb_data);
- else if (filter->kind & FILTER_REFS_ALL)
+ else if (filter->kind & FILTER_REFS_ALL || filter->kind & FILTER_REFS_NO_FILTER)
ret = for_each_fullref_in_pattern(filter, fn, cb_data);
if (!ret && (filter->kind & FILTER_REFS_DETACHED_HEAD))
head_ref(fn, cb_data);
diff --git a/ref-filter.h b/ref-filter.h
index 07cd6f6da3..1eab325ce0 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -22,7 +22,9 @@
#define FILTER_REFS_ALL (FILTER_REFS_TAGS | FILTER_REFS_BRANCHES | \
FILTER_REFS_REMOTES | FILTER_REFS_OTHERS)
#define FILTER_REFS_DETACHED_HEAD 0x0020
-#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD)
+#define FILTER_REFS_NO_FILTER 0x0040
+#define FILTER_REFS_KIND_MASK (FILTER_REFS_ALL | FILTER_REFS_DETACHED_HEAD | \
+ FILTER_REFS_NO_FILTER)
struct atom_value;
struct ref_sorting;
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 82f3d1ea0f..3922326cab 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -31,6 +31,40 @@ test_expect_success 'setup some history and refs' '
git update-ref refs/odd/spot main
'
+cat >expect <<-\EOF
+ HEAD
+ ORIG_HEAD
+ refs/heads/main
+ refs/heads/side
+ refs/odd/spot
+ refs/tags/annotated-tag
+ refs/tags/doubly-annotated-tag
+ refs/tags/doubly-signed-tag
+ refs/tags/four
+ refs/tags/one
+ refs/tags/signed-tag
+ refs/tags/three
+ refs/tags/two
+EOF
+
+test_expect_success 'empty pattern prints pseudorefs' '
+ git update-ref ORIG_HEAD main &&
+ git for-each-ref --format="%(refname)" "" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'empty pattern with other patterns' '
+ git update-ref ORIG_HEAD main &&
+ git for-each-ref --format="%(refname)" "" "refs/" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'empty pattern towards the end' '
+ git update-ref ORIG_HEAD main &&
+ git for-each-ref --format="%(refname)" "refs/" "" >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'filtering with --points-at' '
cat >expect <<-\EOF &&
refs/heads/main
--
2.43.GIT
^ permalink raw reply related
* [PATCH 4/5] refs: introduce `refs_for_each_all_refs()`
From: Karthik Nayak @ 2024-01-19 14:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20240119142705.139374-1-karthik.188@gmail.com>
Introduce a new ref iteration flag `DO_FOR_EACH_INCLUDE_ALL_REFS`, which
will be used to iterate over all refs. In the files backend this is
limited to regular refs, pseudorefs and HEAD. For other backends like
the reftable this is the universal set of all refs stored in the
backend.
Refs which fall outside the `refs/` and aren't either pseudorefs or HEAD
are more of a grey area. This is because we don't block the users from
creating such refs but they are not officially supported. In the files
backend, we cannot isolate such files from other files.
Introduce `refs_for_each_all_refs()` which calls `do_for_each_ref()`
with this newly introduced flag.
In `refs/files-backend.c`, introduce a new function
`add_pseudoref_like_entries()` to add pseudorefs and HEAD to the
`ref_dir`. We then finally call `add_pseudoref_like_entries()` whenever
the `DO_FOR_EACH_INCLUDE_ALL_REFS` flag is set. Any new ref backend will
also have to implement similar changes on its end.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs.c | 7 +++++
refs.h | 6 +++++
refs/files-backend.c | 64 ++++++++++++++++++++++++++++++++++++++++----
refs/refs-internal.h | 7 +++++
4 files changed, 79 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index b84e173762..e527c6199d 100644
--- a/refs.c
+++ b/refs.c
@@ -1722,6 +1722,13 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
return refs_for_each_rawref(get_main_ref_store(the_repository), fn, cb_data);
}
+int refs_for_each_all_refs(struct ref_store *refs, each_ref_fn fn,
+ void *cb_data)
+{
+ return do_for_each_ref(refs, "", NULL, fn, 0,
+ DO_FOR_EACH_INCLUDE_ALL_REFS, cb_data);
+}
+
static int qsort_strcmp(const void *va, const void *vb)
{
const char *a = *(const char **)va;
diff --git a/refs.h b/refs.h
index f1bbad83fb..fef70b6599 100644
--- a/refs.h
+++ b/refs.h
@@ -393,6 +393,12 @@ int for_each_namespaced_ref(const char **exclude_patterns,
int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
int for_each_rawref(each_ref_fn fn, void *cb_data);
+/*
+ * Iterates over all ref types, regular, pseudorefs and HEAD.
+ */
+int refs_for_each_all_refs(struct ref_store *refs, each_ref_fn fn,
+ void *cb_data);
+
/*
* Normalizes partial refs to their fully qualified form.
* Will prepend <prefix> to the <pattern> if it doesn't start with 'refs/'.
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a4884f557d..95a73b11bb 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -315,9 +315,58 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
add_per_worktree_entries_to_dir(dir, dirname);
}
-static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)
+/*
+ * Add pseudorefs and HEAD to the ref dir by parsing the directory
+ * for any files which follow the pseudoref syntax.
+ */
+static void add_pseudoref_like_entries(struct ref_store *ref_store,
+ struct ref_dir *dir,
+ const char *dirname)
+{
+ struct files_ref_store *refs =
+ files_downcast(ref_store, REF_STORE_READ, "fill_ref_dir");
+ struct strbuf path = STRBUF_INIT, refname = STRBUF_INIT;
+ struct dirent *de;
+ size_t dirnamelen;
+ DIR *d;
+
+ files_ref_path(refs, &path, dirname);
+
+ d = opendir(path.buf);
+ if (!d) {
+ strbuf_release(&path);
+ return;
+ }
+
+ strbuf_addstr(&refname, dirname);
+ dirnamelen = refname.len;
+
+ while ((de = readdir(d)) != NULL) {
+ unsigned char dtype;
+
+ if (de->d_name[0] == '.')
+ continue;
+ if (ends_with(de->d_name, ".lock"))
+ continue;
+ strbuf_addstr(&refname, de->d_name);
+
+ dtype = get_dtype(de, &path, 1);
+ if (dtype == DT_REG && is_pseudoref_syntax(de->d_name))
+ loose_fill_ref_dir_regular_file(refs, refname.buf, dir);
+
+ strbuf_setlen(&refname, dirnamelen);
+ }
+ strbuf_release(&refname);
+ strbuf_release(&path);
+ closedir(d);
+}
+
+static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs,
+ unsigned int flags)
{
if (!refs->loose) {
+ struct ref_dir *dir;
+
/*
* Mark the top-level directory complete because we
* are about to read the only subdirectory that can
@@ -328,12 +377,17 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)
/* We're going to fill the top level ourselves: */
refs->loose->root->flag &= ~REF_INCOMPLETE;
+ dir = get_ref_dir(refs->loose->root);
+
+ if (flags & DO_FOR_EACH_INCLUDE_ALL_REFS)
+ add_pseudoref_like_entries(dir->cache->ref_store, dir,
+ refs->loose->root->name);
+
/*
* Add an incomplete entry for "refs/" (to be filled
* lazily):
*/
- add_entry_to_dir(get_ref_dir(refs->loose->root),
- create_dir_entry(refs->loose, "refs/", 5));
+ add_entry_to_dir(dir, create_dir_entry(refs->loose, "refs/", 5));
}
return refs->loose;
}
@@ -861,7 +915,7 @@ static struct ref_iterator *files_ref_iterator_begin(
* disk, and re-reads it if not.
*/
- loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),
+ loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),
prefix, ref_store->repo, 1);
/*
@@ -1222,7 +1276,7 @@ static int files_pack_refs(struct ref_store *ref_store,
packed_refs_lock(refs->packed_ref_store, LOCK_DIE_ON_ERROR, &err);
- iter = cache_ref_iterator_begin(get_loose_ref_cache(refs), NULL,
+ iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL,
the_repository, 0);
while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
/*
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 4af83bf9a5..981a91c4c6 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -260,6 +260,13 @@ enum do_for_each_ref_flags {
* INCLUDE_BROKEN, since they are otherwise not included at all.
*/
DO_FOR_EACH_OMIT_DANGLING_SYMREFS = (1 << 2),
+
+ /*
+ * Include all refs in the $GIT_DIR in contrast to generally only listing
+ * references having the "refs/" prefix. In the files-backend this is
+ * limited to regular refs, pseudorefs and HEAD.
+ */
+ DO_FOR_EACH_INCLUDE_ALL_REFS = (1 << 3),
};
/*
--
2.43.GIT
^ permalink raw reply related
* [PATCH 3/5] refs: extract out `loose_fill_ref_dir_regular_file()`
From: Karthik Nayak @ 2024-01-19 14:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20240119142705.139374-1-karthik.188@gmail.com>
Extract out the code for adding a single file to the loose ref dir as
`loose_fill_ref_dir_regular_file()` from `loose_fill_ref_dir()` in
`refs/files-backend.c`.
This allows us to use this function independently in the following
commits where we add code to also add pseudorefs to the ref dir.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs/files-backend.c | 62 +++++++++++++++++++++++---------------------
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6734f2a309..a4884f557d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -229,6 +229,38 @@ static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dir
}
}
+static void loose_fill_ref_dir_regular_file(struct files_ref_store *refs,
+ const char *refname,
+ struct ref_dir *dir)
+{
+ struct object_id oid;
+ int flag;
+
+ if (!refs_resolve_ref_unsafe(&refs->base, refname, RESOLVE_REF_READING,
+ &oid, &flag)) {
+ oidclr(&oid);
+ flag |= REF_ISBROKEN;
+ } else if (is_null_oid(&oid)) {
+ /*
+ * It is so astronomically unlikely
+ * that null_oid is the OID of an
+ * actual object that we consider its
+ * appearance in a loose reference
+ * file to be repo corruption
+ * (probably due to a software bug).
+ */
+ flag |= REF_ISBROKEN;
+ }
+
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+ if (!refname_is_safe(refname))
+ die("loose refname is dangerous: %s", refname);
+ oidclr(&oid);
+ flag |= REF_BAD_NAME | REF_ISBROKEN;
+ }
+ add_entry_to_dir(dir, create_ref_entry(refname, &oid, flag));
+}
+
/*
* Read the loose references from the namespace dirname into dir
* (without recursing). dirname must end with '/'. dir must be the
@@ -257,8 +289,6 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
strbuf_add(&refname, dirname, dirnamelen);
while ((de = readdir(d)) != NULL) {
- struct object_id oid;
- int flag;
unsigned char dtype;
if (de->d_name[0] == '.')
@@ -274,33 +304,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
create_dir_entry(dir->cache, refname.buf,
refname.len));
} else if (dtype == DT_REG) {
- if (!refs_resolve_ref_unsafe(&refs->base,
- refname.buf,
- RESOLVE_REF_READING,
- &oid, &flag)) {
- oidclr(&oid);
- flag |= REF_ISBROKEN;
- } else if (is_null_oid(&oid)) {
- /*
- * It is so astronomically unlikely
- * that null_oid is the OID of an
- * actual object that we consider its
- * appearance in a loose reference
- * file to be repo corruption
- * (probably due to a software bug).
- */
- flag |= REF_ISBROKEN;
- }
-
- if (check_refname_format(refname.buf,
- REFNAME_ALLOW_ONELEVEL)) {
- if (!refname_is_safe(refname.buf))
- die("loose refname is dangerous: %s", refname.buf);
- oidclr(&oid);
- flag |= REF_BAD_NAME | REF_ISBROKEN;
- }
- add_entry_to_dir(dir,
- create_ref_entry(refname.buf, &oid, flag));
+ loose_fill_ref_dir_regular_file(refs, refname.buf, dir);
}
strbuf_setlen(&refname, dirnamelen);
}
--
2.43.GIT
^ permalink raw reply related
* [PATCH 2/5] refs: make `is_pseudoref_syntax()` stricter
From: Karthik Nayak @ 2024-01-19 14:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20240119142705.139374-1-karthik.188@gmail.com>
The `is_pseudoref_syntax()` function is used to determine if a
particular refname follows the pseudoref syntax. The pseudoref syntax is
loosely defined at this instance as all refs which are in caps and use
underscores. Most of the pseudorefs also have the "HEAD" suffix.
Using this information we make the `is_pseudoref_syntax()` function
stricter, by adding the check for "HEAD" suffix and for refs which don't
end with the HEAD suffix, matching them with a predetermined list.
This requires fixing up t1407 to use the "HEAD" suffix for creation of
pseudorefs.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs.c | 21 ++++++++++++++++++---
t/t1407-worktree-ref-store.sh | 12 ++++++------
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 5999605230..b84e173762 100644
--- a/refs.c
+++ b/refs.c
@@ -829,6 +829,14 @@ int is_per_worktree_ref(const char *refname)
int is_pseudoref_syntax(const char *refname)
{
+ /* TODO: move these pseudorefs to have _HEAD suffix */
+ static const char *const irregular_pseudorefs[] = {
+ "BISECT_EXPECTED_REV",
+ "NOTES_MERGE_PARTIAL",
+ "NOTES_MERGE_REF",
+ "AUTO_MERGE"
+ };
+ size_t i;
const char *c;
for (c = refname; *c; c++) {
@@ -837,10 +845,17 @@ int is_pseudoref_syntax(const char *refname)
}
/*
- * HEAD is not a pseudoref, but it certainly uses the
- * pseudoref syntax.
+ * Most pseudorefs end with _HEAD. HEAD itself is not a
+ * pseudoref, but it certainly uses the pseudoref syntax.
*/
- return 1;
+ if (ends_with(refname, "HEAD"))
+ return 1;
+
+ for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++)
+ if (!strcmp(refname, irregular_pseudorefs[i]))
+ return 1;
+
+ return 0;
}
static int is_current_worktree_ref(const char *ref) {
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 05b1881c59..53592c95f3 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -61,18 +61,18 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
# not testing a realistic scenario.
test_expect_success REFFILES 'for_each_reflog()' '
- echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
+ echo $ZERO_OID >.git/logs/PSEUDO_MAIN_HEAD &&
mkdir -p .git/logs/refs/bisect &&
- echo $ZERO_OID > .git/logs/refs/bisect/random &&
+ echo $ZERO_OID >.git/logs/refs/bisect/random &&
- echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
+ echo $ZERO_OID >.git/worktrees/wt/logs/PSEUDO_WT_HEAD &&
mkdir -p .git/worktrees/wt/logs/refs/bisect &&
- echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
+ echo $ZERO_OID >.git/worktrees/wt/logs/refs/bisect/wt-random &&
$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
- PSEUDO-WT 0x0
+ PSEUDO_WT_HEAD 0x0
refs/bisect/wt-random 0x0
refs/heads/main 0x0
refs/heads/wt-main 0x0
@@ -82,7 +82,7 @@ test_expect_success REFFILES 'for_each_reflog()' '
$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
- PSEUDO-MAIN 0x0
+ PSEUDO_MAIN_HEAD 0x0
refs/bisect/random 0x0
refs/heads/main 0x0
refs/heads/wt-main 0x0
--
2.43.GIT
^ permalink raw reply related
* [PATCH 1/5] refs: expose `is_pseudoref_syntax()`
From: Karthik Nayak @ 2024-01-19 14:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20240119142705.139374-1-karthik.188@gmail.com>
The `is_pseudoref_syntax()` function is static, since it is only used
within `refs.c`. In the following commit, we will use this function to
provide an utility to add pseudorefs to the loose refs cache. So let's
expose this function via `refs.h`.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
refs.c | 2 +-
refs.h | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 2f58a3460a..5999605230 100644
--- a/refs.c
+++ b/refs.c
@@ -827,7 +827,7 @@ int is_per_worktree_ref(const char *refname)
starts_with(refname, "refs/rewritten/");
}
-static int is_pseudoref_syntax(const char *refname)
+int is_pseudoref_syntax(const char *refname)
{
const char *c;
diff --git a/refs.h b/refs.h
index ff113bb12a..f1bbad83fb 100644
--- a/refs.h
+++ b/refs.h
@@ -846,6 +846,12 @@ const char **hidden_refs_to_excludes(const struct strvec *hide_refs);
/* Is this a per-worktree ref living in the refs/ namespace? */
int is_per_worktree_ref(const char *refname);
+/*
+ * Check whether a refname matches the pseudoref syntax. This is a surface
+ * level check and can present false positives.
+ */
+int is_pseudoref_syntax(const char *refname);
+
/* Describes how a refname relates to worktrees */
enum ref_worktree_type {
REF_WORKTREE_CURRENT, /* implicitly per worktree, eg. HEAD or
--
2.43.GIT
^ permalink raw reply related
* [PATCH 0/5] for-each-ref: print all refs on empty string pattern
From: Karthik Nayak @ 2024-01-19 14:27 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
With the upcoming introduction of the reftable backend, it becomes ever
so important to provide the necessary tooling for printing all refs
associated with a repository.
While regular refs stored within the "refs/" namespace are currently
supported by multiple commands like git-for-each-ref(1),
git-show-ref(1). Neither support printing all the operational refs
within the repository.
This is crucial because with the reftable backend, all these refs will
also move to reftable. It would be necessary to identify all the refs
that are stored within the reftable since there is no easy way to do so
otherwise. This is because the reftable itself is a binary format and
all access will be via git. Unlike the filesystem backend, which allows
access directly via the filesystem.
This patch series is a follow up to the RFC/discussion we had earlier on
the list [1].
The first 4 commits add the required functionality to ensure we can print
all refs (regular, pseudo, HEAD). The 5th commit modifies the
git-for-each-ref(1) command to print all refs when an empty string pattern
is used. This is a deviation from the current situation wherein the empty
string pattern currently matches and prints no refs.
[1]: https://lore.kernel.org/git/20231221170715.110565-1-karthik.188@gmail.com/#t
Karthik Nayak (5):
refs: expose `is_pseudoref_syntax()`
refs: make `is_pseudoref_syntax()` stricter
refs: extract out `loose_fill_ref_dir_regular_file()`
refs: introduce `refs_for_each_all_refs()`
for-each-ref: avoid filtering on empty pattern
Documentation/git-for-each-ref.txt | 3 +-
builtin/for-each-ref.c | 21 ++++-
ref-filter.c | 13 ++-
ref-filter.h | 4 +-
refs.c | 32 ++++++--
refs.h | 12 +++
refs/files-backend.c | 126 +++++++++++++++++++++--------
refs/refs-internal.h | 7 ++
t/t1407-worktree-ref-store.sh | 12 +--
t/t6302-for-each-ref-filter.sh | 34 ++++++++
10 files changed, 214 insertions(+), 50 deletions(-)
--
2.43.GIT
^ permalink raw reply
* Re: [PATCH 12/12] t5312: move reffiles specific tests to t0600
From: Patrick Steinhardt @ 2024-01-19 13:40 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <316a20ed17950e4e45d7ea13d8f6e8d4e064821e.1705521155.git.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]
On Wed, Jan 17, 2024 at 07:52:35PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
>
> Move a few tests into t0600 since they specifically test the packed-refs
> file and thus are specific to the reffiles backend.
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> t/t0600-reffiles-backend.sh | 30 ++++++++++++++++++++++++++++++
> t/t5312-prune-corruption.sh | 26 --------------------------
> 2 files changed, 30 insertions(+), 26 deletions(-)
>
> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> index c88576dfea5..190155f592d 100755
> --- a/t/t0600-reffiles-backend.sh
> +++ b/t/t0600-reffiles-backend.sh
> @@ -571,4 +571,34 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
> test_grep broken stderr
> '
>
> +# we do not want to count on running pack-refs to
> +# actually pack it, as it is perfectly reasonable to
> +# skip processing a broken ref
> +test_expect_success 'create packed-refs file with broken ref' '
> + test_tick && git commit --allow-empty -m one &&
> + recoverable=$(git rev-parse HEAD) &&
> + test_tick && git commit --allow-empty -m two &&
> + missing=$(git rev-parse HEAD) &&
> + rm -f .git/refs/heads/main &&
> + cat >.git/packed-refs <<-EOF &&
> + $missing refs/heads/main
> + $recoverable refs/heads/other
> + EOF
> + echo $missing >expect &&
> + git rev-parse refs/heads/main >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'pack-refs does not silently delete broken packed ref' '
> + git pack-refs --all --prune &&
> + git rev-parse refs/heads/main >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'pack-refs does not drop broken refs during deletion' '
> + git update-ref -d refs/heads/other &&
> + git rev-parse refs/heads/main >actual &&
> + test_cmp expect actual
> +'
Should these tests be moved into t0601-reffiles-pack-refs.sh instead?
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 10/12] t3903: move reffiles specific tests to t0600
From: Patrick Steinhardt @ 2024-01-19 13:39 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <56a9c8f20dd7c8f3e9401b2bd3929fb9c53c7d27.1705521155.git.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3234 bytes --]
On Wed, Jan 17, 2024 at 07:52:33PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
>
> Move this test into t0600 with other reffiles specific tests since it
> modifies reflog refs manually and thus is specific to the reffiles
> backend.
>
> This change also consolidates setup_stash() into test-lib-functions.sh
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
> t/t3903-stash.sh | 43 -------------------------------------
> t/test-lib-functions.sh | 16 ++++++++++++++
> 3 files changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> index 704b73fdc54..bee61b2d19d 100755
> --- a/t/t0600-reffiles-backend.sh
> +++ b/t/t0600-reffiles-backend.sh
> @@ -527,4 +527,31 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
> test_must_fail git rev-parse --verify broken
> '
>
> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
> + git init repo &&
> + (
> + cd repo &&
> + setup_stash
> + ) &&
> + echo 9 >repo/file &&
> +
> + old_oid="$(git -C repo rev-parse stash@{0})" &&
> + git -C repo stash &&
> + new_oid="$(git -C repo rev-parse stash@{0})" &&
> +
> + cat >expect <<-EOF &&
> + $(test_oid zero) $old_oid
> + $old_oid $new_oid
> + EOF
> + cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> + test_cmp expect actual &&
> +
> + git -C repo stash drop stash@{1} &&
> + cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> + cat >expect <<-EOF &&
> + $(test_oid zero) $new_oid
> + EOF
> + test_cmp expect actual
> +'
I think that there is no need to make this backend-specific. What we're
testing here is that `git stash drop` is able to drop the latest reflog
entry. The calls to cut(1) are only used to verify that the contents of
the reflog entry look as expected while only verifying the old and new
object IDs.
So how about below patch to make it generic instead?
Patrick
-- >8 --
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34faeac3f1..3319240515 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -200,7 +200,7 @@ test_expect_success 'drop stash reflog updates refs/stash' '
test_cmp expect actual
'
-test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
+test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
git init repo &&
(
cd repo &&
@@ -213,16 +213,16 @@ test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite'
new_oid="$(git -C repo rev-parse stash@{0})" &&
cat >expect <<-EOF &&
- $(test_oid zero) $old_oid
- $old_oid $new_oid
+ $new_oid
+ $old_oid
EOF
- cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+ git -C repo reflog show refs/stash --format=%H >actual &&
test_cmp expect actual &&
git -C repo stash drop stash@{1} &&
- cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+ git -C repo reflog show refs/stash --format=%H >actual &&
cat >expect <<-EOF &&
- $(test_oid zero) $new_oid
+ $new_oid
EOF
test_cmp expect actual
'
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Bugreport
From: Frank Schwidom @ 2024-01-19 13:25 UTC (permalink / raw)
To: git
This bug exists in possibly all git versions.
$ git init
$ touch a.txt
$ ln -s a.txt d
$ git add .
$ git commit -m + .
[master (root-commit) f6b4468] +
2 files changed, 1 insertion(+)
create mode 100644 a.txt
create mode 120000 d
$ ls -la
total 12
drwxr-xr-x 3 ox ox 4096 Jan 19 14:10 .
drwxr-xr-x 4 ox ox 4096 Jan 19 14:04 ..
drwxr-xr-x 8 ox ox 4096 Jan 19 14:10 .git
-rw-r--r-- 1 ox ox 0 Jan 19 14:10 a.txt
lrwxrwxrwx 1 ox ox 5 Jan 19 14:10 d -> a.txt
$ rm d
$ mkdir d
$ touch d/b.txt
$ git add .
$ git commit . -m +
error: 'd' does not have a commit checked out
fatal: updating files failed
# I expect that git just replaces the link by the directory. But it makes problems.
# Workaround:
$ rm -rf d
$ git add .
$ git commit -m + .
[master 522e6db] +
1 file changed, 1 deletion(-)
delete mode 120000 d
$ mkdir d
$ touch d/b.txt
$ git add .
$ git commit -m + .
[master 8a125ee] +
1 file changed, 0 insertions(+), 0 deletions(-)
create mode 100644 d/b.txt
[System Info]
git version:
git version 2.43.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.1.0-8-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.25-1 (2023-04-22) x86_64
compiler info: gnuc: 13.2
libc info: glibc: 2.37
$SHELL (typically, interactive shell): /bin/bash
[Enabled Hooks]
Thanks in advance,
Frank Schwidom
^ permalink raw reply
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