Some file names that are okay on ext4 and on HFS+ are illegal in
Windows. In order to stay truly platform-independent, Git's source code
must not contain such illegal file names, even if things just happen to
work on Linux.
One such file name was recently introduced into Git's `pu` branch:
t4013/diff.diff_--diff-line-prefix=-->_master_master^_sidt4013/diff.diff_--diff-line-prefix=-->_master_master^_sid
It is illegal because it contains the '>' character that is not part of
a valid filename on Windows.
To catch these issues early, let's introduce a new test-lint-* goal that
fails if such file names were detected and run it as part of a `make
test`.
Signed-off-by: Johannes Schindelin <redacted>
---
Published-As: https://github.com/dscho/git/releases/tag/test-lint-filenames-v1
Fetch-It-Via: git fetch https://github.com/dscho/git test-lint-filenames-v1
This is the more complete fix I talked about.
t/Makefile | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/t/Makefile b/t/Makefile
index 18e2b28..3c0eb48 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:
+ @illegal="$$(git ls-files | grep '["*:<>?\\|]')"; \
+ test -z "$$illegal" || { \
+ echo >&2 "illegal file name(s): " $$illegal; exit 1; }
+
aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean--
2.9.2.691.g78954f3
base-commit: 5e18482599a6cfeb8d4e0ee5a98d30220cbdff72
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 even be checked out.
Make sure that double-quotes, asterisk, colon, greater/less-than,
question-mark, backslash, tab, vertical-bar, as well as any non-ASCII
characters above 0x7f never appear 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 in the
future.
Signed-off-by: Johannes Schindelin <redacted>
---
I re-munged the commit message (I saw that you munged it in `pu`
and did not quite like the wording, so I permitted myself the
indulgence).
And of course I tested this, and it does *not* list all files in
subdirectories of t/ as problematic.
Published-As: https://github.com/dscho/git/releases/tag/test-lint-filenames-v2
Fetch-It-Via: git fetch https://github.com/dscho/git test-lint-filenames-v2
Interdiff vs v1:
diff --git a/t/Makefile b/t/Makefile
index 3c0eb48..bf9cad9 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -69,7 +69,7 @@ test-lint-shell-syntax:
@'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS)
test-lint-filenames:
- @illegal="$$(git ls-files | grep '["*:<>?\\|]')"; \
+ @illegal="$$(git ls-files | grep '[ "*:<>?\\|]')"; \
test -z "$$illegal" || { \
echo >&2 "illegal file name(s): " $$illegal; exit 1; }
t/Makefile | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/t/Makefile b/t/Makefile
index 18e2b28..bf9cad9 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:
+ @illegal="$$(git ls-files | grep '[ "*:<>?\\|]')"; \
+ test -z "$$illegal" || { \
+ echo >&2 "illegal file name(s): " $$illegal; exit 1; }
+
aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean--
2.9.2.691.g78954f3
base-commit: 70dce4aee55ad8a39a53f86f37a4bd400e0cac7d
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 even be checked out.
Make sure that double-quotes, asterisk, colon, greater/less-than,
question-mark, backslash, tab, vertical-bar, as well as any non-ASCII
characters never appear in the pathnames with a new test-lint-* target
as part of a `make test`. To that end, we call `git ls-files` (ensuring
that the paths are quoted properly), relying on the fact that paths
containing non-ASCII characters are quoted within double-quotes.
In case that the source code does not actually live in a Git
repository (e.g. when extracted from a .zip file), or that the `git`
executable cannot be executed, we simply ignore the error for now; In
that case, our trusty Continuous Integration will be the last line of
defense and catch any problematic file name.
Noticed when a topic wanted to add a pathname with '>' in it. A
check like this will prevent a similar problems from happening in the
future.
Signed-off-by: Johannes Schindelin <redacted>
---
Again some touch-ups to the commit message.
Please note that the paths are now quoted within double-quotes,
thanks to the core.quotepaths setting. As such, non-ASCII
characters are no longer caught through the backslash, but through
the double-quote character (i.e. '"').
As previously, tested on Linux and Windows and verified that it
does the job.
Published-As: https://github.com/dscho/git/releases/tag/test-lint-filenames-v3
Fetch-It-Via: git fetch https://github.com/dscho/git test-lint-filenames-v3
Interdiff vs v2:
diff --git a/t/Makefile b/t/Makefile
index bf9cad9..d613935 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -69,9 +69,12 @@ test-lint-shell-syntax:
@'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS)
test-lint-filenames:
- @illegal="$$(git ls-files | grep '[ "*:<>?\\|]')"; \
- test -z "$$illegal" || { \
- echo >&2 "illegal file name(s): " $$illegal; exit 1; }
+ @# We do *not* pass a glob to ls-files but use grep instead, to catch
+ @# non-ASCII characters (which are quoted within double-quotes)
+ @bad="$$(git -c core.quotepath=true ls-files 2>/dev/null | \
+ grep '["*:<>?\\|]')"; \
+ test -z "$$bad" || { \
+ echo >&2 "non-portable file name(s): $$bad"; exit 1; }
aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
t/Makefile | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/t/Makefile b/t/Makefile
index 18e2b28..d613935 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,14 @@ test-lint-executable:
test-lint-shell-syntax:
@'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS)
+test-lint-filenames:
+ @# We do *not* pass a glob to ls-files but use grep instead, to catch
+ @# non-ASCII characters (which are quoted within double-quotes)
+ @bad="$$(git -c core.quotepath=true ls-files 2>/dev/null | \
+ grep '["*:<>?\\|]')"; \
+ test -z "$$bad" || { \
+ echo >&2 "non-portable file name(s): $$bad"; exit 1; }
+
aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean--
2.9.2.691.g78954f3
base-commit: 70dce4aee55ad8a39a53f86f37a4bd400e0cac7d