modify the “foo" file path to "$PWD/bad-clone/sub/foo". #2022
+1
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/submit |
|
Submitted as pull.2022.git.git.1753860300588.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this): On Wed, Jul 30, 2025, at 09:25, On Wed, Jul 30, 2025, at 09:25, 陈建虎 via GitGitGadget wrote:
> From: =?UTF-8?q?=E9=99=88=E5=BB=BA=E8=99=8E?= <chenjianhu@kylinos.cn>
> ...
> Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
The author name and signoff name should match. Either “陈建虎” or the
(I’m guessing) romanization “chenjianhu” for both.
--
Kristoffer Haugsbakk
|
|
User |
Yes, I've updated the profile settings on GitHub to consistently use "chenjianhu" as both the author name and sign-off name. |
@cjhxmx unfortunately, Kristoffer won't see this reply unless you follow the instructions how to reply on the Git mailing list. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> On Wed, Jul 30, 2025, at 09:25, On Wed, Jul 30, 2025, at 09:25, 陈建虎 via GitGitGadget wrote:
>> From: =?UTF-8?q?=E9=99=88=E5=BB=BA=E8=99=8E?= <chenjianhu@kylinos.cn>
>> ...
>> Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
>
> The author name and signoff name should match. Either “陈建虎” or the
> (I’m guessing) romanization “chenjianhu” for both.
Yup, either is fine but it would be nicer to non-east-asian friends
to use the Anglicized form.
The title is what needs more polish than the name though. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "陈建虎 via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: =?UTF-8?q?=E9=99=88=E5=BB=BA=E8=99=8E?= <chenjianhu@kylinos.cn>
>
> In the t7450-bad-git-dotfiles.sh, when post-checkout
> is executed, the actual path where the foo file
> is created should be "$PWD/bad-clone/sub/foo".
"is created" is a bit iffy thing to say, as the test actually
expects the path _not_ to exist.
Also, pay special attention to what you say on your Subject: line.
Can I tell what area the change touches by only looking at the
Subject: line, especially when it is mixed with dozens of other
patch e-mails?
is the question any author of a patch e-mail should be asking.
$ git log --no-merges --format=%s -100 | sort
may give us some inspirations. For this one, perhaps I would have
written
Subject: t7450: inspect the correct path a broken code would write to
Prior to 05e9cd64 (config: quote values containing CR character,
2025-05-19), a repository can trick "clone --recurse-submodules"
into running a post-checkout hook shipped with the project. The
test was written to make sure the trick would no longer run the
hook with the fix in the commit.
However, the test did not check for the path the hook would
create; correct the path to the expected one if the bug were
still with us.
or something like that.
Justin, who wrote the test originally, Cc'ed for comments.
Thanks.
> diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
> index 14b5743b962..f512eed278c 100755
> --- a/t/t7450-bad-git-dotfiles.sh
> +++ b/t/t7450-bad-git-dotfiles.sh
> @@ -401,7 +401,7 @@ test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into d
> git -C repo commit -m submodule &&
>
> git -c protocol.file.allow=always clone --recurse-submodules repo bad-clone &&
> - ! test -f "$PWD/foo" &&
> + ! test -f "$PWD/bad-clone/sub/foo" &&
> test -f $(printf "bad-clone/sub\r/post-checkout")
> ' |
|
@cjhxmx also please remember not to close old, and open new, PRs left and right, but instead to simply force-push the PR branch before |
|
On the Git mailing list, Justin Tobler wrote (reply to this): On 25/07/30 08:47AM, Junio C Hamano wrote:
> "陈建虎 via GitGitGadget" <gitgitgadget@gmail.com> writes:
> > From: =?UTF-8?q?=E9=99=88=E5=BB=BA=E8=99=8E?= <chenjianhu@kylinos.cn>
>
> Justin, who wrote the test originally, Cc'ed for comments.
>
> > diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
> > index 14b5743b962..f512eed278c 100755
> > --- a/t/t7450-bad-git-dotfiles.sh
> > +++ b/t/t7450-bad-git-dotfiles.sh
> > @@ -401,7 +401,7 @@ test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into d
> > git -C repo commit -m submodule &&
> >
> > git -c protocol.file.allow=always clone --recurse-submodules repo bad-clone &&
> > - ! test -f "$PWD/foo" &&
> > + ! test -f "$PWD/bad-clone/sub/foo" &&
This assertion is supposed to validate that the post-checkout hook did
not execute. The path here is incorrect indeed. I've tested the fix here
and it works as expected. Thanks
> > test -f $(printf "bad-clone/sub\r/post-checkout")
The second assertion here still demonstrates that a file was not checked
out into an arbitrary location, which is the root of the problem, and
likely why I missed this. Apologies.
-Justin |
|
User |
Prior to 05e9cd6 (config: quote values containing CR character, 2025-05-19), a repository can trick "clone --recurse-submodules" into running a post-checkout hook shipped with the project. The test was written to make sure the trick would no longer run the hook with the fix in the commit. However, the test did not check for the path the hook would create; correct the path to the expected one if the bug were still with us. Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
|
/submit |
|
Submitted as pull.2022.v2.git.git.1753933780883.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "chenjianhu via GitGitGadget" <gitgitgadget@gmail.com> writes:
Somehow Justin and Kristoffer are missing from the Cc list.
> From: chenjianhu <chenjianh@kylinos.cn>
>
> Prior to 05e9cd64 (config: quote values containing CR character,
> 2025-05-19), a repository can trick "clone --recurse-submodules"
> into running a post-checkout hook shipped with the project. The
> test was written to make sure the trick would no longer run the
> hook with the fix in the commit.
>
> However, the test did not check for the path the hook would
> create; correct the path to the expected one if the bug were
> still with us.
>
> Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
> ---
> modify the “foo" file path to "$PWD/bad-clone/sub/foo".
>
> cc: "Kristoffer Haugsbakk" kristofferhaugsbakk@fastmail.com cc: Justin
> Tobler jltobler@gmail.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2022%2Fcjhxmx%2Fcjhxmx-git-test-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2022/cjhxmx/cjhxmx-git-test-v2
> Pull-Request: https://github.com/git/git/pull/2022
>
> Range-diff vs v1:
>
> 1: c2d1d8fe884 ! 1: 6434587a075 modify the “foo" file path to "$PWD/bad-clone/sub/foo".
> @@
> ## Metadata ##
> -Author: 陈建虎 <chenjianhu@kylinos.cn>
> +Author: chenjianhu <chenjianh@kylinos.cn>
>
> ## Commit message ##
> - modify the “foo" file path to "$PWD/bad-clone/sub/foo".
> + t7450: inspect the correct path a broken code would write to
>
> - In the t7450-bad-git-dotfiles.sh, when post-checkout
> - is executed, the actual path where the foo file
> - is created should be "$PWD/bad-clone/sub/foo".
> + Prior to 05e9cd64 (config: quote values containing CR character,
> + 2025-05-19), a repository can trick "clone --recurse-submodules"
> + into running a post-checkout hook shipped with the project. The
> + test was written to make sure the trick would no longer run the
> + hook with the fix in the commit.
> +
> + However, the test did not check for the path the hook would
> + create; correct the path to the expected one if the bug were
> + still with us.
>
> Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
>
>
>
> t/t7450-bad-git-dotfiles.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
> index 14b5743b962..f512eed278c 100755
> --- a/t/t7450-bad-git-dotfiles.sh
> +++ b/t/t7450-bad-git-dotfiles.sh
> @@ -401,7 +401,7 @@ test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into d
> git -C repo commit -m submodule &&
>
> git -c protocol.file.allow=always clone --recurse-submodules repo bad-clone &&
> - ! test -f "$PWD/foo" &&
> + ! test -f "$PWD/bad-clone/sub/foo" &&
> test -f $(printf "bad-clone/sub\r/post-checkout")
> '
>
>
> base-commit: e813a0200a7121b97fec535f0d0b460b0a33356c |
|
This patch series was integrated into seen via 45d530d. |
|
CC: Kristoffer Haugsbakkkristofferhaugsbakk@fastmail.com,Justin Tobler jltobler@gmail.com |
|
On the Git mailing list, Justin Tobler wrote (reply to this): On 25/07/31 03:49AM, chenjianhu via GitGitGadget wrote:
> From: chenjianhu <chenjianh@kylinos.cn>
>
> Prior to 05e9cd64 (config: quote values containing CR character,
> 2025-05-19), a repository can trick "clone --recurse-submodules"
> into running a post-checkout hook shipped with the project. The
> test was written to make sure the trick would no longer run the
> hook with the fix in the commit.
Yep the first assertion in the test exists to ensure that the
post-checkout hook in the submodule is not executed. The test also
validates via its second assertion that the sumodule cannot be tricked
into being checked-out into a symlinked directory.
> However, the test did not check for the path the hook would
> create; correct the path to the expected one if the bug were
> still with us.
>
> Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
> ---
[snip]
> diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
> index 14b5743b962..f512eed278c 100755
> --- a/t/t7450-bad-git-dotfiles.sh
> +++ b/t/t7450-bad-git-dotfiles.sh
> @@ -401,7 +401,7 @@ test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into d
> git -C repo commit -m submodule &&
>
> git -c protocol.file.allow=always clone --recurse-submodules repo bad-clone &&
> - ! test -f "$PWD/foo" &&
> + ! test -f "$PWD/bad-clone/sub/foo" &&
Yep, this is the correct path now.
This patch looks good to me. Thanks for fixing :)
-Justin
> test -f $(printf "bad-clone/sub\r/post-checkout")
> '
>
>
> base-commit: e813a0200a7121b97fec535f0d0b460b0a33356c
> --
> gitgitgadget
> |
|
User |
|
This branch is now known as |
|
This patch series was integrated into seen via 83b7426. |
|
This patch series was integrated into next via 478a84a. |
|
This patch series was integrated into seen via 8744dad. |
|
There was a status update in the "Cooking" section about the branch Test fix. Will merge to 'master'. source: <pull.2022.v2.git.git.1753933780883.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via d8f795e. |
|
This patch series was integrated into master via d8f795e. |
|
This patch series was integrated into next via d8f795e. |
|
Closed via d8f795e. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc: Kristoffer Haugsbakk kristofferhaugsbakk@fastmail.com
cc: Justin Tobler jltobler@gmail.com
cc: Justin Tobler jltobler@gmail.com