Re: [PATCH] t/Makefile: make sure that file names are truly platform-independent

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 15, 2016, 21:03 UTC
Message-ID: <xmqqoa4tg2yi.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqk2fihv90.fsf@gitster.mtv.corp.google.com>

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

Show 21 quoted lines

> Good thinking.
>
> Some tests may have to be skipped on platforms that cannot express
> certain paths, but even then they shouldn't ship a file with
> pathname that cannot even be checked out (they should instead create
> and use such a path, protected behind filesystem specific test
> prerequisite).
>
>> +test-lint-filenames:
>> +	@illegal="$$(git ls-files | grep '["*:<>?\\|]')"; \
>
> This pattern must exclude questionables on either NTFS or HFS+; it
> is ironic that it is not even sufficient to limit ourselves to the
> Portable Character Set [*1*], but such is life.
>
> By the way, doesn't ls-files take pathspec glob, saving one extra
> process to run grep?
>
>     master$ git ls-files '*["*:<>?\\|]*'
>     pu$ git ls-files '*["*:<>?\\|]*'
>     t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side

One more thing you may want to exclude is HT. Here is a suggested reroll. I reworded to avoid a subjective "truly platform-independent", which is not what we intend to aim for anyway (we only try to support the platforms we care about).

-- >8 --
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 15 Aug 2016 16:08:41 +0200
Subject: [PATCH] t/Makefile: make sure that paths can be checked out on
 platforms we care

Some pathnames that are okay on ext4 and on HFS+ cannot be checked out on Windows. Tests that want to see operations on such paths on filesystems that support them must do so behind appropriate test prerequisites, and must not include them in the source tree (instead they should create them when they run). Otherwise, the source tree cannot be even checked out.

Make sure that double-quotes, asterisk, colon, greater/less-than, question-mark, backslash, tab and vertical-bar never appears in the pathnames with a new test-lint-* target as part of a `make test`.

Noticed when a topic wanted to add a pathname with '>' in it. A check like this will prevent a similar problems from happening.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/t/Makefile b/t/Makefile
index 18e2b28..d4b2a50 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -52,7 +52,8 @@ clean-except-prove-cache:
 clean: clean-except-prove-cache
 	$(RM) .prove
 
-test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
+test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
+	test-lint-filenames
 
 test-lint-duplicates:
 	@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
@@ -67,6 +68,11 @@ test-lint-executable:
 test-lint-shell-syntax:
 	@'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS)
 
+test-lint-filenames:
+	@bad=$$(git ls-files '*[	"*:<>?\\|]*'); \
+		test -z "$$bad" || { \
+		echo >&2 "do not use non-portable file name(s): $$bad"; exit 1; }
+
 aggregate-results-and-cleanup: $(T)
 	$(MAKE) aggregate-results
 	$(MAKE) clean

-- 
2.10.0-rc0-132-gce76bc9