git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Jeff King <peff@peff.net>,
	 Han Young <hanyang.tony@bytedance.com>,
	 Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: [PATCH v2] t4126: make sure a directory with SP at the end is usable
Date: Thu, 28 Mar 2024 14:08:47 -0700	[thread overview]
Message-ID: <xmqqh6gqt674.fsf_-_@gitster.g> (raw)
In-Reply-To: <xmqqa5miuutd.fsf@gitster.g> (Junio C. Hamano's message of "Thu, 28 Mar 2024 10:31:42 -0700")

As afb31ad9 (t1010: fix unnoticed failure on Windows, 2021-12-11)
said:

    On Microsoft Windows, a directory name should never end with a period.
    Quoting from Microsoft documentation[1]:

	Do not end a file or directory name with a space or a period.
	Although the underlying file system may support such names, the
	Windows shell and user interface does not.

    [1]: https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file

and the condition addressed by this change is exactly that.  If the
platform is unable to properly create these sample patches about a
file that lives in a directory whose name ends with a SP, there is
no point testing how "git apply" behaves there on the filesystem.

Even though the ultimate purpose of "git apply" is to apply a patch
and to update the filesystem entities, this particular test is
mainly about parsing a patch on a funny pathname correctly, and even
on a system that is incapable of checking out the resulting state
correctly on its filesystem, at least the parsing can and should work
fine.  Rewrite the test to work inside the index without touching the
filesystem.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

    Junio C Hamano <gitster@pobox.com> writes:

    > As this test _is_, unlike the cited patch that was not about a
    > directory with a funny name, about parsing a patch and applying it
    > to a path with a directory with a funny name, I am tempted to keep
    > the test with the filesystem, instead of replacing it with the one
    > using the "--cached" that Peff suggested.  I am _also_ tempted to
    > add that "--cached" thing (instead of replacing), though.

    So, I changed my mind and just took Peff's "--cached" approach
    with no filesystem-based test.  format-patch --range-diff just
    didn't understand that the single patch corresponds to the only
    one patch in the older "series", and I had to force it to match
    them with --creation-factor=999 in a separate invocation.  The
    patch text has changed too much so it is useless, but the log
    message change may be easier to see in the range-diff.

1:  7e84d0f64f ! 1:  a107f21ea2 t4126: make sure a directory with SP at the end is usable
    @@ Commit message
         and the condition addressed by this change is exactly that.  If the
         platform is unable to properly create these sample patches about a
         file that lives in a directory whose name ends with a SP, there is
    -    no point testing how "git apply" behaves there.
    +    no point testing how "git apply" behaves there on the filesystem.
     
    -    Protect the test that involves the filesystem access with a
    -    prerequisite, and perform the same test only within the index
    -    everywhere.
    +    Even though the ultimate purpose of "git apply" is to apply a patch
    +    and to update the filesystem entities, this particular test is
    +    mainly about parsing a patch on a funny pathname correctly, and even
    +    on a system that is incapable of checking out the resulting state
    +    correctly on its filesystem, at least the parsing can and should work
    +    fine.  Rewrite the test to work inside the index without touching the
    +    filesystem.
     
         Helped-by: Jeff King <peff@peff.net>
         Helped-by: Eric Sunshine <sunshine@sunshineco.com>
    @@ t/t4126-apply-empty.sh: test_expect_success 'apply --index create' '
      '
      
     -test_expect_success 'apply with no-contents and a funny pathname' '
    -+test_expect_success 'setup patches in dir ending in SP' '
    -+	test_when_finished "rm -fr \"funny \"" &&
    - 	mkdir "funny " &&
    - 	>"funny /empty" &&
    - 	git add "funny /empty" &&
    +-	mkdir "funny " &&
    +-	>"funny /empty" &&
    +-	git add "funny /empty" &&
     -	git diff HEAD "funny /" >sample.patch &&
     -	git diff -R HEAD "funny /" >elpmas.patch &&
    -+	git diff HEAD -- "funny /" >sample.patch &&
    -+	git diff -R HEAD -- "funny /" >elpmas.patch &&
    ++test_expect_success 'parsing a patch with no-contents and a funny pathname' '
      	git reset --hard &&
     -	rm -fr "funny " &&
    -+
    -+	if  grep "a/funny /empty b/funny /empty" sample.patch &&
    -+	    grep "b/funny /empty a/funny /empty" elpmas.patch
    -+	then
    -+		test_set_prereq DIR_ENDS_WITH_SP
    -+	else
    -+		# Win test???
    -+		ls -l
    -+	fi
    -+'
    -+
    -+test_expect_success DIR_ENDS_WITH_SP 'apply with no-contents and a funny pathname' '
    -+	test_when_finished "rm -fr \"funny \"" &&
    - 
    - 	git apply --stat --check --apply sample.patch &&
    - 	test_must_be_empty "funny /empty" &&
    -@@ t/t4126-apply-empty.sh: test_expect_success 'apply with no-contents and a funny pathname' '
    - 	test_path_is_missing "funny /empty"
    - '
    - 
    -+test_expect_success 'parsing a patch with no-contents and a funny pathname' '
    -+	git reset --hard &&
    -+
     +	empty_blob=$(test_oid empty_blob) &&
    -+	echo $empty_blob >expect &&
    -+
    ++	echo "$empty_blob" >expect &&
    + 
    +-	git apply --stat --check --apply sample.patch &&
    +-	test_must_be_empty "funny /empty" &&
     +	git update-index --add --cacheinfo "100644,$empty_blob,funny /empty" &&
     +	git diff --cached HEAD -- "funny /" >sample.patch &&
     +	git diff --cached -R HEAD -- "funny /" >elpmas.patch &&
     +	git reset &&
    -+
    + 
    +-	git apply --stat --check --apply elpmas.patch &&
    +-	test_path_is_missing "funny /empty" &&
     +	git apply --cached --stat --check --apply sample.patch &&
     +	git rev-parse --verify ":funny /empty" >actual &&
     +	test_cmp expect actual &&
    -+
    + 
    +-	git apply -R --stat --check --apply elpmas.patch &&
    +-	test_must_be_empty "funny /empty" &&
     +	git apply --cached --stat --check --apply elpmas.patch &&
     +	test_must_fail git rev-parse --verify ":funny /empty" &&
    -+
    + 
    +-	git apply -R --stat --check --apply sample.patch &&
    +-	test_path_is_missing "funny /empty"
     +	git apply -R --cached --stat --check --apply elpmas.patch &&
     +	git rev-parse --verify ":funny /empty" >actual &&
     +	test_cmp expect actual &&
     +
     +	git apply -R --cached --stat --check --apply sample.patch &&
     +	test_must_fail git rev-parse --verify ":funny /empty"
    -+'
    -+
    + '
    + 
      test_done

 t/t4126-apply-empty.sh | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh
index eaf0c5304a..2462cdf904 100755
--- a/t/t4126-apply-empty.sh
+++ b/t/t4126-apply-empty.sh
@@ -66,26 +66,29 @@ test_expect_success 'apply --index create' '
 	git diff --exit-code
 '
 
-test_expect_success 'apply with no-contents and a funny pathname' '
-	mkdir "funny " &&
-	>"funny /empty" &&
-	git add "funny /empty" &&
-	git diff HEAD "funny /" >sample.patch &&
-	git diff -R HEAD "funny /" >elpmas.patch &&
+test_expect_success 'parsing a patch with no-contents and a funny pathname' '
 	git reset --hard &&
-	rm -fr "funny " &&
+	empty_blob=$(test_oid empty_blob) &&
+	echo "$empty_blob" >expect &&
 
-	git apply --stat --check --apply sample.patch &&
-	test_must_be_empty "funny /empty" &&
+	git update-index --add --cacheinfo "100644,$empty_blob,funny /empty" &&
+	git diff --cached HEAD -- "funny /" >sample.patch &&
+	git diff --cached -R HEAD -- "funny /" >elpmas.patch &&
+	git reset &&
 
-	git apply --stat --check --apply elpmas.patch &&
-	test_path_is_missing "funny /empty" &&
+	git apply --cached --stat --check --apply sample.patch &&
+	git rev-parse --verify ":funny /empty" >actual &&
+	test_cmp expect actual &&
 
-	git apply -R --stat --check --apply elpmas.patch &&
-	test_must_be_empty "funny /empty" &&
+	git apply --cached --stat --check --apply elpmas.patch &&
+	test_must_fail git rev-parse --verify ":funny /empty" &&
 
-	git apply -R --stat --check --apply sample.patch &&
-	test_path_is_missing "funny /empty"
+	git apply -R --cached --stat --check --apply elpmas.patch &&
+	git rev-parse --verify ":funny /empty" >actual &&
+	test_cmp expect actual &&
+
+	git apply -R --cached --stat --check --apply sample.patch &&
+	test_must_fail git rev-parse --verify ":funny /empty"
 '
 
 test_done
-- 
2.44.0-368-gc75fd8d815


  reply	other threads:[~2024-03-28 21:09 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-19  9:52 [PATCH 0/1] quote: quote space Han Young
2024-03-19  9:52 ` [PATCH 1/1] " Han Young
2024-03-19  9:59   ` Kristoffer Haugsbakk
2024-03-19 15:15 ` [PATCH 0/1] " Junio C Hamano
2024-03-19 22:56   ` Junio C Hamano
2024-03-26 21:41     ` Junio C Hamano
2024-03-27  9:17     ` Jeff King
2024-03-27 14:59       ` Junio C Hamano
2024-03-27 22:11     ` Junio C Hamano
2024-03-28 10:32       ` Jeff King
2024-03-28 11:40         ` Jeff King
2024-03-28 17:05           ` Eric Sunshine
2024-03-28 17:31             ` Junio C Hamano
2024-03-28 21:08               ` Junio C Hamano [this message]
2024-03-29  2:18                 ` [PATCH v2] t4126: make sure a directory with SP at the end is usable Junio C Hamano
2024-03-29  5:37                   ` [PATCH] t4126: fix "funny directory name" test on Windows (again) Junio C Hamano
2024-03-29 12:00                     ` Jeff King
2024-03-29 17:21                     ` [PATCH v2] " Junio C Hamano
2024-03-29 18:34                       ` Jeff King
2024-03-29 11:27                   ` [PATCH v2] t4126: make sure a directory with SP at the end is usable Jeff King
2024-03-29 17:01                     ` Junio C Hamano
2024-04-27 14:47                       ` Johannes Schindelin
2024-04-27 17:20                         ` Junio C Hamano
2024-03-28 16:19         ` [PATCH 0/1] quote: quote space Junio C Hamano
2024-03-28 16:30           ` Jeff King
2024-03-28 16:53             ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh6gqt674.fsf_-_@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=hanyang.tony@bytedance.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).