git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo".
@ 2025-07-30  7:25 陈建虎 via GitGitGadget
  2025-07-30  7:38 ` Kristoffer Haugsbakk
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: 陈建虎 via GitGitGadget @ 2025-07-30  7:25 UTC (permalink / raw)
  To: git; +Cc: 陈建虎, 陈建虎

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".

Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
---
    modify the “foo" file path to "$PWD/bad-clone/sub/foo".

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2022%2Fcjhxmx%2Fcjhxmx-git-test-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2022/cjhxmx/cjhxmx-git-test-v1
Pull-Request: https://github.com/git/git/pull/2022

 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
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo".
  2025-07-30  7:25 [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo" 陈建虎 via GitGitGadget
@ 2025-07-30  7:38 ` Kristoffer Haugsbakk
  2025-07-30 15:09   ` Junio C Hamano
  2025-07-30 15:47 ` Junio C Hamano
  2025-07-31  3:49 ` [PATCH v2] t7450: inspect the correct path a broken code would write to chenjianhu via GitGitGadget
  2 siblings, 1 reply; 8+ messages in thread
From: Kristoffer Haugsbakk @ 2025-07-30  7:38 UTC (permalink / raw)
  To: Josh Soref, git; +Cc: 陈建虎, 陈建虎

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo".
  2025-07-30  7:38 ` Kristoffer Haugsbakk
@ 2025-07-30 15:09   ` Junio C Hamano
  0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2025-07-30 15:09 UTC (permalink / raw)
  To: Kristoffer Haugsbakk
  Cc: Josh Soref, git, 陈建虎,
	陈建虎

"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.  

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo".
  2025-07-30  7:25 [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo" 陈建虎 via GitGitGadget
  2025-07-30  7:38 ` Kristoffer Haugsbakk
@ 2025-07-30 15:47 ` Junio C Hamano
  2025-07-30 18:08   ` Justin Tobler
  2025-07-31  3:49 ` [PATCH v2] t7450: inspect the correct path a broken code would write to chenjianhu via GitGitGadget
  2 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2025-07-30 15:47 UTC (permalink / raw)
  To: 陈建虎 via GitGitGadget, Justin Tobler
  Cc: git, 陈建虎, 陈建虎

"陈建虎 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")
>  '

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo".
  2025-07-30 15:47 ` Junio C Hamano
@ 2025-07-30 18:08   ` Justin Tobler
  0 siblings, 0 replies; 8+ messages in thread
From: Justin Tobler @ 2025-07-30 18:08 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: 陈建虎 via GitGitGadget, git,
	陈建虎, 陈建虎

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] t7450: inspect the correct path a broken code would write to
  2025-07-30  7:25 [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo" 陈建虎 via GitGitGadget
  2025-07-30  7:38 ` Kristoffer Haugsbakk
  2025-07-30 15:47 ` Junio C Hamano
@ 2025-07-31  3:49 ` chenjianhu via GitGitGadget
  2025-07-31 18:45   ` Junio C Hamano
  2025-08-01 16:48   ` Justin Tobler
  2 siblings, 2 replies; 8+ messages in thread
From: chenjianhu via GitGitGadget @ 2025-07-31  3:49 UTC (permalink / raw)
  To: git; +Cc: chenjianhu, chenjianhu

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
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] t7450: inspect the correct path a broken code would write to
  2025-07-31  3:49 ` [PATCH v2] t7450: inspect the correct path a broken code would write to chenjianhu via GitGitGadget
@ 2025-07-31 18:45   ` Junio C Hamano
  2025-08-01 16:48   ` Justin Tobler
  1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2025-07-31 18:45 UTC (permalink / raw)
  To: chenjianhu via GitGitGadget
  Cc: Justin Tobler, Kristoffer Haugsbakk, git, chenjianhu, chenjianhu

"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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] t7450: inspect the correct path a broken code would write to
  2025-07-31  3:49 ` [PATCH v2] t7450: inspect the correct path a broken code would write to chenjianhu via GitGitGadget
  2025-07-31 18:45   ` Junio C Hamano
@ 2025-08-01 16:48   ` Justin Tobler
  1 sibling, 0 replies; 8+ messages in thread
From: Justin Tobler @ 2025-08-01 16:48 UTC (permalink / raw)
  To: chenjianhu via GitGitGadget; +Cc: git, chenjianhu, chenjianhu

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
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-08-01 16:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-30  7:25 [PATCH] modify the “foo" file path to "$PWD/bad-clone/sub/foo" 陈建虎 via GitGitGadget
2025-07-30  7:38 ` Kristoffer Haugsbakk
2025-07-30 15:09   ` Junio C Hamano
2025-07-30 15:47 ` Junio C Hamano
2025-07-30 18:08   ` Justin Tobler
2025-07-31  3:49 ` [PATCH v2] t7450: inspect the correct path a broken code would write to chenjianhu via GitGitGadget
2025-07-31 18:45   ` Junio C Hamano
2025-08-01 16:48   ` Justin Tobler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).