* 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: [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
* [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
* [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 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 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 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 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 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 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 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 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
* Re: [Outreachy] Move existing tests to a unit testing framework
From: Achu Luma @ 2023-10-24 14:25 UTC (permalink / raw)
To: Christian Couder, git, Junio C Hamano
In-Reply-To: <CAP8UFD0A_vWCZ5cVAZqdTBebdhZNye_FmNNJF+vA7epUx2JWHQ@mail.gmail.com>
On Mon, Oct 23, 2023 at 2:41 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Fri, Oct 20, 2023 at 3:16 PM Achu Luma <ach.lumap@gmail.com> wrote:
> >
> > Dear Git Community and Mentors,
> >
> > I hope this email finds you well. As a follow-up to my previous application, I'd like to provide additional details on the process of migrating existing unit tests from the t/helper/ directory to the new Git unit test framework.
>
> Thanks for these details!
>
> > -- Identify Target Unit Tests: Start by identifying the specific unit tests in the t/helper/ directory that we want to port to the new Git unit test framework. Ensure that the tests are suitable for migration and that the benefits of doing so outweigh the effort(By avoiding integration tests). The following points have been developed with on going work on the unit-tests framework visible here:
> >
> > 1- https://lore.kernel.org/git/0169ce6fb9ccafc089b74ae406db0d1a8ff8ac65.1688165272.git.steadmon@google.com/
> > 2- https://github.com/steadmon/git/blob/unit-tests-asciidoc/Documentation/technical/unit-tests.adoc
>
> Maybe if you have time you could add some descriptions or comments
> related to the above emails and documents. For example you could tell
> what the new unit test framework will be like, how the unit tests will
> look like, etc. Maybe a short overview would be nice.
>
sure,
1- https://lore.kernel.org/git/0169ce6fb9ccafc089b74ae406db0d1a8ff8ac65.1688165272.git.steadmon@google.com/
:
The emails highlight the significant milestones achieved in
defining and testing the custom TAP
framework for writing git unit tests. It also contains some
examples of implementation such as
that of STRBUF_INIT with output:
ok 1 - static initialization works
1..1
2- https://github.com/steadmon/git/blob/unit-tests-asciidoc/Documentation/technical/unit-tests.adoc:
From this technical doc, the new unit test framework in the Git
project represents a significant
enhancement, introducing a systematic and efficient approach to
unit testing. The custom git
TAP implementation was selected from several alternatives based
on strict criteria as the most
suitable test framework for porting the unit tests.
The unit tests are written in pure C, eliminating the need for
the previous shell/test-tool helper
setup, simplifying test configuration, data handling, and
reducing testing runtime.
Each unit test is encapsulated as a function and employs a range
of predefined check functions
for validation. These checks can evaluate conditions, compare
integers or characters, validate
strings, and more, providing comprehensive coverage for test scenarios.
When a test is run using the TEST() macro, it undergoes a series
of checks, and if any check fails,
a diagnostic message is printed to aid in debugging. This
diagnostic output includes information
about the specific check that failed, the file and line number
where it occurred, and a clear comparison
of the expected and actual values. Such detailed reporting
simplifies the identification and resolution
of issues, contributing to codebase stability.
Additionally, the framework supports features like skipping tests
with explanations, sending custom
diagnostic messages using test_msg(), and marking known-to-fail
checks using TEST_TODO().
This flexibility allows developers to tailor their tests to
specific scenarios while ensuring a
comprehensive testing suite.
> You could also try to apply the patches in the series that adds the
> test framework, or alternatively use the 'seen' branch where the
> series has been merged, and start playing with it by writing, or
> porting, a small example test.
>
ok, I think I can push a patch for one.
> > -- Create a New C Test File: For each unit test I plan to migrate, create a new C source file (.c) in the Git project's test suite directory(t/unit-tests). Name it appropriately to reflect the purpose of the test.
>
> Could you provide an example of what the new name would be for an
> existing test that is worth porting?
>
Sure... let's consider an existing unit test in t/helper directory
such as t/helper/test-date.c or
its shell named t0006-date.sh, which is part of the current
shell-based test suite. In the context
of the new unit testing framework, this test could be reimagined and renamed as
"t-date.c". The "t-" prefix is typically used for test program files
in Git, and "date" is retained to
reflect the nature of the tests within this suite.
> > -- Include Necessary Headers:In the new C test file, include the necessary Git unit test framework headers. Typically, this includes headers like "test-lib.h" and others relevant to the specific test.
> > #include "test-lib.h"
>
> Maybe you could continue the above example and tell which headers
> would be needed for it?
>
> > -- Convert Test Logic: Refactor the test logic from the original Shell script into the new C-based test format. Use the testing macros provided by the Git unit test framework, such as test_expect_success, test_expect_failure, etc., to define the tests.
> > test_expect_success("simple progress display", "{
> > // Test logic here...
> > }");
>
> Ok, a simple example would be nice too.
>
we can continue with the example used for naming: test-date.c. a
typical t-date.c unit test would look
like the following:
--
#include "test-lib.h"
#include "date.h"
--
date.h here is a necessary header file. Now refactoring the test logic
from the original shell script:
--
#include "test-lib.h"
#include "date.h"
static void test_parse_dates(void)
{
const char *dates[] = { "invalid_date", "2023-10-17 10:00:00 +0200", NULL };
for (const char **argv = dates; *argv; argv++) {
check_int(parse_dates((const char *[]){ *argv, NULL }), 0);
}
}
--
> > -- Add Test Descriptions: Provide clear and informative descriptions for each test using the testing macros. These descriptions will help in identifying the purpose of each test when the test suite is run.
>
> This would seem to be part of the previous step, as you would have to
> provide a description when using the testing macro. But Ok.
>
> > -- Define a Test Entry Point: Create a cmd_main function as the entry point for the C-based tests. Inside this function, include the test functions using the testing macros.
> > int cmd_main(int argc, const char **argv) {
> > // Test functions...
> > return test_done();
> > }
>
> Yeah, continuing an example would be nice.
>
Continuing, we can add a test entrance as follows:
--
#include "test-lib.h"
#include "date.h"
static void test_parse_dates(void)
{
const char *dates[] = { "invalid_date", "2023-10-17 10:00:00 +0200", NULL };
for (const char **argv = dates; *argv; argv++) {
check_int(parse_dates((const char *[]){ *argv, NULL }), 0);
}
}
int main(int argc UNUSED, const char **argv UNUSED)
{
TEST(test_parse_dates, "Test date parsing");
return test_done();
}
--
A typical unit tests with the custom TAP framework would look
something like above. This might run in theory
but I have not yet run it as I used it here just for demonstration.
The unit tests can be built using
"make unit-tests." Additionally, Makefile can be modified to add the
file to the build:
--
UNIT_TEST_PROGRAMS += t-date
--
> > -- Ensure TAP Format Output: Ensure that the C-based tests produce output in the Test Anything Protocol (TAP) format. This format includes the test name, status (ok or not ok), and any diagnostic information.
>
> That means using TEST* macros in the cmd_main() function, as they
> should do the right thing or is there more to be done here?
>
> > -- Test Interaction: Ensure that the migrated tests interact correctly with the new Git unit test framework and any other tests that may be relevant. Consider dependencies and interactions with other parts of the Git project.
>
> I am not sure what work would be needed here. Is there more to do than
> compiling the test files? Having an example would be nice.
>
> > -- Test Execution: Run the migrated tests to verify that they produce the expected results when executed as part of the Git project's test suite. Use the Git testing framework's test runners to execute the tests.
>
> Ok.
>
> > -- Documentation Update: Update the Git project's documentation to reflect the changes made during the migration. Include a reference to the original unit tests in the t/helper/ directory and indicate that these tests have been ported to the new Git unit test framework.
>
> I am not sure that we would want that. I think we might instead want
> to document things in t/helper/ that we don't want to port to the new
> unit test framework and why.
>
Ok noted.
> > By following these points, I think I can successfully port existing unit tests from the t/helper/ directory to use the new Git unit test framework. This migration helps standardize and streamline the testing process within the Git project, improving code quality and maintainability.
>
> Yeah!
>
> > Next Steps:
> >
> > I am eager to discuss these suggestions and collaborate with the Git community to ensure the success of this project. I will continue to engage with the community, seek guidance, and refine my proposal as per your suggestions.
> > I look forward to the opportunity to contribute to the Git project and help make it even more robust and reliable.
>
> Thanks for this application and sorry for the late answer!
>
> Best,
> Christian.
I look forward to feedback and better understanding of the new testing
framework.
BR,
Achu Luma.
^ permalink raw reply
* Re: [PATCH v4 4/7] bulk-checkin: implement `SOURCE_INCORE` mode for `bulk_checkin_source`
From: Junio C Hamano @ 2023-10-24 17:08 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Jeff King, Taylor Blau, git, Elijah Newren, Eric W. Biederman
In-Reply-To: <ZTdlgEaRLpdSjB1n@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> I'm inclined to agree with your philosophy. Makes me wonder whether we
> should write a Coccinelle rule to catch this. But a quick-and-dirty grep
> in our codebase shows that such a rule would cause quite a lot of churn:
>
> $ git grep BUG\( | wc -l
> 677
> $ git grep assert\( | wc -l
> 549
>
> Probably not worth it.
Yeah, we can stick to our usual "do not add new instances, fix them
while touching near-by code" pattern for this one, I would say.
Thanks.
^ permalink raw reply
* Re: [RESEND v2] git-rebase.txt: rewrite docu for fixup/squash (again)
From: Junio C Hamano @ 2023-10-24 17:19 UTC (permalink / raw)
To: Phillip Wood
Cc: Oswald Buddenhagen, phillip.wood, git, Christian Couder,
Charvi Mendiratta, Marc Branchaud, Johannes Sixt
In-Reply-To: <b2b76344-11b7-4f21-8658-f18ffcca2dea@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> I agree there is an argument for improving the implementation of
> --autosquash but until we do I think it is counterproductive to change
> the documentation like this as it will cause users to wonder why
> "rebase --autosquash" generates a todo list that is incorrect
> according to the documentation.
That's a good point.
> I do think it is a good idea to document where the authorship of a
> rebased commit comes from.
Yeah, sounds like a good idea. As to the authorship information, it
might be nicer if the "rebase -i" insn language supported an option
to trigger --reset-author (or even better, --author=...) action for
a single commit, but I presume that it is rather a rare event, and
as long as people understand that they can stop the sequencing
(e.g., an "edit" of the commit would do) and run "commit --amend",
it should be OK, so it probably is OK to leave it as-is.
Thanks.
^ permalink raw reply
* Re: using oldest date when squashing commits
From: Junio C Hamano @ 2023-10-24 17:30 UTC (permalink / raw)
To: Phillip Wood; +Cc: Oswald Buddenhagen, phillip.wood, Johannes Sixt, git
In-Reply-To: <138631cd-ead3-4f22-95ce-61afccfa409f@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
>>> 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"
Exactly.
It would be OK to add "fixup -c --reset-author", but the default
should stay. In addition, I wouldn't be able to use "rebase -i" to
make typofixes to commits made out of received patches if the
operation changes the authorship.
> "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.
Yup, it still is a "fix", meaning the identity and the spirit of the
commit being fixed are unchanged. What it aims to achieve, how it
implements the behaviour it wants to give its users, who thought of
that change, all that are the same as the original. It may be a
nice addition to optionally allow users to use --reset-author (or
better yet, --author="Na Me <a@dd.re.ss>") with "fixup", but if the
"-c" variant can be concluded with "commit --amend --reset-author"
to achieve the same effect, that may be sufficient.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 3/3] rev-list: add commit object support in `--missing` option
From: Junio C Hamano @ 2023-10-24 17:45 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, ps
In-Reply-To: <20231024122631.158415-4-karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> @@ -1176,7 +1181,11 @@ static int process_parents(struct rev_info *revs,...
> break;
> continue;
> }
> - return -1;
> +
> + if (!revs->do_not_die_on_missing_objects)
> + return -1;
> + else
> + oidset_insert(&revs->missing_objects, &p->object.oid);
I would suspect that swapping if/else would make it easier to
follow. Everybody else in the patch guards the use of the oidset
with "were we told not to die on missing objects?", i.e.,
if (revs->do_not_die_on_missing_objects)
oidset_insert(&revs->missing_objects, &p->object.oid);
else
return -1; /* corrupt repository */
> @@ -3800,6 +3809,9 @@ int prepare_revision_walk(struct rev_info *revs)
> FOR_EACH_OBJECT_PROMISOR_ONLY);
> }
>
> + if (revs->do_not_die_on_missing_objects)
> + oidset_init(&revs->missing_objects, 0);
I read the patch to make sure that .missing_objects oidset is used
only when .do_not_die_on_missing_objects is set and the oidset is
untouched unless it is initialized. Well done.
I know I floated "perhaps oidset can replace the object bits,
especially because the number of objects that need marking is
expected to be small", but I am curious what the performance
implication of this would be. Is this something we can create
comparison easily?
I noticed that nobody releases the resource held by this new oidset.
Shouldn't we do so in revision.c:release_revisions()?
Thanks.
^ permalink raw reply
* Re: [PATCH 02/12] builtin/show-ref: split up different subcommands
From: Eric Sunshine @ 2023-10-24 17:55 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Han-Wen Nienhuys
In-Reply-To: <7e6ab5dee230dcb66cb8adfe4a8114a06c805802.1698152926.git.ps@pks.im>
On Tue, Oct 24, 2023 at 9:10 AM Patrick Steinhardt <ps@pks.im> wrote:
> 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>
> ---
> @@ -142,6 +142,53 @@ static int exclude_existing(const char *match)
> +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++;
> + }
A couple style-nits here caught my attention...
- "}" and "else" should be cuddled: `} else if`
- coding guidelines these days want braces on all branches if any
branch needs them
However, since this code is merely being relocated from elsewhere in
this file and since these style-nits were already present, moving the
code verbatim without correcting the style problems is more
reviewer-friendly. Okay.
> + return 0;
> +}
> +
> +static int cmd_show_ref__patterns(const char **patterns)
> +{
> + struct show_ref_data show_ref_data = {
> + .patterns = (patterns && *patterns) ? patterns : NULL,
> + };
Are we allowing non-constant initializers in the codebase? If not,
this should probably initialize .patterns to NULL and then
conditionally assign `patterns` separately in code below the
initializer.
> + 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;
> +}
^ permalink raw reply
* Re: [PATCH 04/12] builtin/show-ref: fix dead code when passing patterns
From: Eric Sunshine @ 2023-10-24 18:02 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Han-Wen Nienhuys
In-Reply-To: <29c0c0c6c9728f286efd0b78996f9a1394957862.1698152926.git.ps@pks.im>
On Tue, Oct 24, 2023 at 9:10 AM Patrick Steinhardt <ps@pks.im> wrote:
> 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`
s/of/if/
> 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>
^ permalink raw reply
* Re: [PATCH 2/2] fetch: no redundant error message for atomic fetch
From: Junio C Hamano @ 2023-10-24 18:16 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Jiang Xin, Git List, Jiang Xin
In-Reply-To: <ZTZF3AbNNuGpy38l@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> With that reasoning we could get rid of the error handling of abort
> completely as it's known not to fail. But only because it does not fail
> right now doesn't mean that it won't in the future, as the infra for it
> to fail is all in place. And in case it ever does the current code will
> run into the bug I described.
>
> So in my opinion, we should either refactor the code to clarify that
> this cannot fail indeed. Or do the right thing and handle the error case
> correctly, which right now we don't.
Sounds reasonable. Thanks for a good review.
^ permalink raw reply
* Re: [PATCH 05/12] builtin/show-ref: refactor `--exclude-existing` options
From: Eric Sunshine @ 2023-10-24 18:48 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Han-Wen Nienhuys
In-Reply-To: <8d0b0b5700c7ffed6b3a74760d0d9155b404bb4f.1698152926.git.ps@pks.im>
On Tue, Oct 24, 2023 at 9:11 AM Patrick Steinhardt <ps@pks.im> wrote:
> It's not immediately obvious options which options are applicable to
> what subcommand ni git-show-ref(1) because all options exist as global
s/ni/in/
> 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
s/Consequentially/Consequently/
> of those options and requires the reader to worry less about global
> state.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
> @@ -95,6 +94,11 @@ static int add_existing(const char *refname,
> +struct exclude_existing_options {
> + int enabled;
> + const char *pattern;
> +};
Do we need this `enabled` flag? Can't the same be achieved by checking
whether `pattern` is NULL or not (see below)?
> @@ -104,11 +108,11 @@ static int add_existing(const char *refname,
> -static int cmd_show_ref__exclude_existing(const char *match)
> +static int cmd_show_ref__exclude_existing(const struct exclude_existing_options *opts)
Since you're renaming `match` to `opts->pattern`...
> {
> - int matchlen = match ? strlen(match) : 0;
> + int matchlen = opts->pattern ? strlen(opts->pattern) : 0;
... and since you're touching this line anyway, maybe it makes sense
to rename `matchlen` to `patternlen`?
> @@ -124,11 +128,11 @@ static int cmd_show_ref__exclude_existing(const char *match)
> - if (strncmp(ref, match, matchlen))
> + if (strncmp(ref, opts->pattern, matchlen))
Especially since, as shown in this context, `matchlen` is really the
length of the _pattern_, not the length of the resulting _match_.
> @@ -200,44 +204,46 @@ static int hash_callback(const struct option *opt, const char *arg, int unset)
> int cmd_show_ref(int argc, const char **argv, const char *prefix)
> {
> [...]
> - 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);
(continued from above) Can't this be handled without a separate `enabled` flag?
if (exclude_existing_opts.pattern)
...
^ permalink raw reply
* Re: Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address"
From: Michael Strawbridge @ 2023-10-24 19:00 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: git, Luben Tuikov, entwicklung
In-Reply-To: <20231024130037.sbevzk2x7oclj7d7@pengutronix.de>
On 10/24/23 09:00, Uwe Kleine-König wrote:
> Hello,
>
> On Fri, Oct 20, 2023 at 05:06:36PM -0400, Michael Strawbridge wrote:
>> On 10/20/23 06:04, Uwe Kleine-König wrote:
>>> hello,
>>>
>>> On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
>>>> Hello,
>>>>
>>>> $ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
>>>> Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
>>>> error: unable to extract a valid address from:
>>>>
>>>> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
Hm. I tried reproing with master (59167d7d09fd, "The seventeenth batch") but I don't seem to see an error:
```
$ git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>' -1 --smtp-server="$(pwd)/fake.sendmail"
/tmp/FEGzLrm_4U/0001-test-test.patch
1
(mbox) Adding cc: Your Name <you@example.com> from line 'From: Your Name <you@example.com>'
(body) Adding cc: Your Name <you@example.com> from line 'Signed-off-by: Your Name <you@example.com>'
From: Your Name <you@example.com>
To: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Cc: Your Name <you@example.com>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:33:51 +0000
Message-ID: <20231024183350.10-2-you@example.com>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
The Cc list above has been expanded by additional
addresses found in the patch commit message. By default
send-email prompts before sending whenever this occurs.
This behavior is controlled by the sendemail.confirm
configuration setting.
For additional information, run 'git send-email --help'.
To retain the current behavior, but squelch this message,
run 'git config --global sendemail.confirm auto'.
Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
-i
OK. Log says:
Sendmail: /git/fake.sendmail -i u.kleine-koenig@pengutronix.de you@example.com
From: Your Name <you@example.com>
To: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Cc: Your Name <you@example.com>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:33:51 +0000
Message-ID: <20231024183350.10-2-you@example.com>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: OK
```
I think my terminal may be mangling the UTF-8 encoding but git send-email doesn't seem to complain.
Only when I go to an older git (version 2.42.0 for example) do I then get:
```
Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
error: unable to extract a valid address from:
```
This would be expected because https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#mae64003cbb72f015bf5c0c04216524fcb6bb8d09
is meant to fix it.
Just in case it was the "from" address, I tried using 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>'
as the from address. This still worked without issue for (59167d7d09fd, "The seventeenth batch").
>>>>
>>>> Bisection points at
>>>>
>>>> a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
>>>>
>>>> I didn't try to understand that change and fix the problem.
>>>
>>> Another (similar?) problem with non-ascii-chars:
>>>
>>> $ git send-email --to 'Will Deacon <will@kernel.org>' --to 'Krzysztof Wilczyński <kw@linux.com>' --to 'Lorenzo Pieralisi <lpieralisi@kernel.org>' --cc 'Rob Herring <robh@kernel.org>' --to 'Bjorn Helgaas <bhelgaas@google.com>' --cc 'linux-pci@vger.kernel.org' --cc kernel@pengutronix.de -1 --base=@~
>>> Use of uninitialized value $address in sprintf at /home/uwe/gsrc/git/git-send-email line 1162.
>>> error: unable to extract a valid address from:
>>>
>>> Bisection points to the same commit, when dropping ń in Krzysztof's
>>> name, it works fine.
I tried this as well with seventeen and it succeeds:
```
/tmp/GfTxbwKSx_/0001-test-test.patch
1
(mbox) Adding cc: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de> from line 'From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>'
(body) Adding cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> from line 'Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>'
From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To: Will Deacon <will@kernel.org>,
=?UTF-8?q?Krzysztof=20Wilczy=C5=84ski?= <kw@linux.com>,
Lorenzo Pieralisi <lpieralisi@kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh@kernel.org>,
linux-pci@vger.kernel.org,
kernel@pengutronix.de,
=?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:57:16 +0000
Message-ID: <20231024185715.10-2-u.kleine-koenig@pengutronix.de>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The Cc list above has been expanded by additional
addresses found in the patch commit message. By default
send-email prompts before sending whenever this occurs.
This behavior is controlled by the sendemail.confirm
configuration setting.
For additional information, run 'git send-email --help'.
To retain the current behavior, but squelch this message,
run 'git config --global sendemail.confirm auto'.
Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
-i
OK. Log says:
Sendmail: /git/fake.sendmail -i will@kernel.org kw@linux.com lpieralisi@kernel.org bhelgaas@google.com robh@kernel.org linux-pci@vger.kernel.org kernel@pengutronix.de u.kleine-koenig@pengutronix.de
From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To: Will Deacon <will@kernel.org>,
=?UTF-8?q?Krzysztof=20Wilczy=C5=84ski?= <kw@linux.com>,
Lorenzo Pieralisi <lpieralisi@kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh@kernel.org>,
linux-pci@vger.kernel.org,
kernel@pengutronix.de,
=?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:57:16 +0000
Message-ID: <20231024185715.10-2-u.kleine-koenig@pengutronix.de>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Result: OK
```
Perhaps there is another variable involved?
>>>
>> This is interesting. Thanks for reporting it. If you are able, could you please try the patches found in the below threads:
>> - https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#mae64003cbb72f015bf5c0c04216524fcb6bb8d09
>
> On main (2e8e77cbac8a) this one is already applied, with that the error
> message reduces to:
>
> $ git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>' -1
> error: unable to extract a valid address from: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
>> - https://public-inbox.org/git/f5c6a72b-f888-4d43-8be8-3ce2c878c669@gmail.com/T/#mca12dc95ccfd3ce2b94e7752ebaae9891201084f
>
> This one doesn't help either. With it applied on top of main I get the
> sams result as on vanilla main.
>
> Best regards
> Uwe
>
^ permalink raw reply
* Re: [PATCH v2 2/2] commit: detect commits that exist in commit-graph but not in the ODB
From: Junio C Hamano @ 2023-10-24 19:10 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak, Taylor Blau, Jeff King
In-Reply-To: <0476d4855562b677ced106a4cc7788b46434cf21.1698060036.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Commit graphs can become stale and contain references to commits that do
> not exist in the object database anymore. Theoretically, this can lead
> to a scenario where we are able to successfully look up any such commit
> via the commit graph even though such a lookup would fail if done via
> the object database directly.
>
> As the commit graph is mostly intended as a sort of cache to speed up
> parsing of commits we do not want to have diverging behaviour in a
> repository with and a repository without commit graphs, no matter
> whether they are stale or not. As commits are otherwise immutable, the
> only thing that we really need to care about is thus the presence or
> absence of a commit.
>
> To address potentially stale commit data that may exist in the graph,
> our `lookup_commit_in_graph()` function will check for the commit's
> existence in both the commit graph, but also in the object database. So
> even if we were able to look up the commit's data in the graph, we would
> still pretend as if the commit didn't exist if it is missing in the
> object database.
>
> We don't have the same safety net in `parse_commit_in_graph_one()`
> though. This function is mostly used internally in "commit-graph.c"
> itself to validate the commit graph, and this usage is fine. We do
> expose its functionality via `parse_commit_in_graph()` though, which
> gets called by `repo_parse_commit_internal()`, and that function is in
> turn used in many places in our codebase.
>
> For all I can see this function is never used to directly turn an object
> ID into a commit object without additional safety checks before or after
> this lookup. What it is being used for though is to walk history via the
> parent chain of commits. So when commits in the parent chain of a graph
> walk are missing it is possible that we wouldn't notice if that missing
> commit was part of the commit graph. Thus, a query like `git rev-parse
> HEAD~2` can succeed even if the intermittent commit is missing.
>
> It's unclear whether there are additional ways in which such stale
> commit graphs can lead to problems. In any case, it feels like this is a
> bigger bug waiting to happen when we gain additional direct or indirect
> callers of `repo_parse_commit_internal()`. So let's fix the inconsistent
> behaviour by checking for object existence via the object database, as
> well.
>
> This check of course comes with a performance penalty. The following
> benchmarks have been executed in a clone of linux.git with stable tags
> added:
>
> Benchmark 1: git -c core.commitGraph=true rev-list --topo-order --all (git = master)
> Time (mean ± σ): 2.913 s ± 0.018 s [User: 2.363 s, System: 0.548 s]
> Range (min … max): 2.894 s … 2.950 s 10 runs
>
> Benchmark 2: git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
> Time (mean ± σ): 3.834 s ± 0.052 s [User: 3.276 s, System: 0.556 s]
> Range (min … max): 3.780 s … 3.961 s 10 runs
>
> Benchmark 3: git -c core.commitGraph=false rev-list --topo-order --all (git = master)
> Time (mean ± σ): 13.841 s ± 0.084 s [User: 13.152 s, System: 0.687 s]
> Range (min … max): 13.714 s … 13.995 s 10 runs
>
> Benchmark 4: git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
> Time (mean ± σ): 13.762 s ± 0.116 s [User: 13.094 s, System: 0.667 s]
> Range (min … max): 13.645 s … 14.038 s 10 runs
>
> Summary
> git -c core.commitGraph=true rev-list --topo-order --all (git = master) ran
> 1.32 ± 0.02 times faster than git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
> 4.72 ± 0.05 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency)
> 4.75 ± 0.04 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = master)
>
> We look at a ~30% regression in general, but in general we're still a
> whole lot faster than without the commit graph. To counteract this, the
> new check can be turned off with the `GIT_COMMIT_GRAPH_PARANOIA` envvar.
Very nicely described. Will queue. I'll go offline for the rest of
the week but if there are no significant issues discovered by the
time I come back, let's declare a victory and merge these two
patches down to 'next'.
Thanks.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> commit.c | 16 +++++++++++++++-
> t/t5318-commit-graph.sh | 27 +++++++++++++++++++++++++++
> 2 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/commit.c b/commit.c
> index b3223478bc..7399e90212 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -28,6 +28,7 @@
> #include "shallow.h"
> #include "tree.h"
> #include "hook.h"
> +#include "parse.h"
>
> static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
>
> @@ -572,8 +573,21 @@ int repo_parse_commit_internal(struct repository *r,
> return -1;
> if (item->object.parsed)
> return 0;
> - if (use_commit_graph && parse_commit_in_graph(r, item))
> + if (use_commit_graph && parse_commit_in_graph(r, item)) {
> + static int object_paranoia = -1;
> +
> + if (object_paranoia == -1)
> + object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
> +
> + if (object_paranoia && !has_object(r, &item->object.oid, 0)) {
> + unparse_commit(r, &item->object.oid);
> + return quiet_on_missing ? -1 :
> + error(_("commit %s exists in commit-graph but not in the object database"),
> + oid_to_hex(&item->object.oid));
> + }
> +
> return 0;
> + }
>
> if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)
> return quiet_on_missing ? -1 :
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index c0cc454538..79467d7926 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -842,4 +842,31 @@ test_expect_success 'stale commit cannot be parsed when given directly' '
> )
> '
>
> +test_expect_success 'stale commit cannot be parsed when traversing graph' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> +
> + test_commit A &&
> + test_commit B &&
> + test_commit C &&
> + git commit-graph write --reachable &&
> +
> + # Corrupt the repository by deleting the intermittent commit
> + # object. Commands should notice that this object is absent and
> + # thus that the repository is corrupt even if the commit graph
> + # exists.
> + oid=$(git rev-parse B) &&
> + rm .git/objects/"$(test_oid_to_path "$oid")" &&
> +
> + # Again, we should be able to parse the commit when not
> + # being paranoid about commit graph staleness...
> + GIT_COMMIT_GRAPH_PARANOIA=false git rev-parse HEAD~2 &&
> + # ... but fail when we are paranoid.
> + test_must_fail git rev-parse HEAD~2 2>error &&
> + grep "error: commit $oid exists in commit-graph but not in the object database" error
> + )
> +'
> +
> test_done
^ permalink raw reply
* Re: [PATCH 00/12] show-ref: introduce mode to check for ref existence
From: Junio C Hamano @ 2023-10-24 19:17 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698152926.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> this patch series introduces a new `--exists` mode to git-show-ref(1) to
> explicitly check for the existence of a reference, only.
I agree that show-ref would be the best place for this feature (not
rev-parse, which is already a kitchen sink). After all, the command
was designed for validating refs in 358ddb62 (Add "git show-ref"
builtin command, 2006-09-15).
Thanks. Hopefully I can take a look before I go offline.
^ permalink raw reply
* Re: [PATCH 09/12] builtin/show-ref: ensure mutual exclusiveness of subcommands
From: Eric Sunshine @ 2023-10-24 19:25 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Han-Wen Nienhuys
In-Reply-To: <d0a991cf4f892e73e4fd62ef3fdae3fa73277321.1698152926.git.ps@pks.im>
On Tue, Oct 24, 2023 at 9:11 AM Patrick Steinhardt <ps@pks.im> wrote:
> 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>
> ---
> diff --git 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)
> + if ((!!exclude_existing_opts.enabled + !!verify) > 1)
> + die(_("only one of --exclude-existing or --verify can be given"));
Somewhat recently, work was done to normalize this sort of message.
The result was to instead use the phrasing "options '%s' and '%s'
cannot be used together". See, for instance, 43ea635c35 (i18n:
refactor "foo and bar are mutually exclusive", 2022-01-05).
^ 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