poelmanc created this revision.
poelmanc added a reviewer: clang-tools-extra.
poelmanc added a project: clang-tools-extra.
Herald added a subscriber: jdoerfert.
poelmanc requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Running `check-clang-tools` on Windows produces 5 test failures:
Failed Tests (5):
Clang Tools :: clang-apply-replacements/ClangRenameClassReplacements.cpp
Clang Tools :: clang-apply-replacements/basic.cpp
Clang Tools :: clang-apply-replacements/format.cpp
Clang Tools :: clang-move/move-used-helper-decls.cpp
Clang Tools :: clang-tidy/infrastructure/export-diagnostics.cpp
Four of these failures are simply due to fixed character position offsets
differing on Windows versus Linux, since Windows line endings take up two
characters instead of one:
- clang-apply-replacements/ClangRenameClassReplacements.cpp runs `clang-rename
-offset=254`
- clang-apply-replacements/Inputs/basic/file[12].yaml specify e.g. `FileOffset:
148` and `Offset: 298`
- clang-apply-replacements/Inputs/format/{no,yes}.yaml specify e.g.
`FileOffset: 94` and `Offset: 94`
- clang-tidy/infrastructure/export-diagnostics.cpp specifies e.g.
`CHECK-YAML-NEXT: FileOffset: 30`
(The move-used-helper-decls.cpp failure seems more complex; clang-move adds a
blank line after `void HelperFun1() {}` when
clang-move/Inputs/helper_decls_test.cpp has LF line endings, but does //not//
add a blank line when the input files has CRLF line endings. That difference in
behavior seems like it may be an actual bug, but I have yet to track it down.)
Do other folks developing on Windows see this? Performing a git checkout with
Linux (LF) or "as-is" line endings would make the tests pass, but the
underlying clang tools need to work on on input files with various line ending
types, so it seems best to test the code against various line ending types. For
example, last year I found a clang-tidy fix that //chomped// a CRLF line ending
in half by implicitly assuming a line ending to be one byte, and the above
clang-move issue could be similar. Ideally the Windows BuildKite job should
test input files with CRLF line endings; since the above tests pass, I'd guess
it's checking out test files only with LF line endings?
- I considered changing export-diagnostics.cpp to e.g. `CHECK-YAML-NEXT:
FileOffset: 3[01]` to accept either, but that would relax the test on Linux -
we really want exactly 30 for LF line endings, and exactly 31 for CRLF line
endings.
- An ideal fix might somehow enable different offset numbers depending on the
input file line endings, but that seems like a big change.
- Instead this patch adds a .gitattributes file to specify Linux LF line
endings only for the above files that use `-offset`, `FileOffset` and `Offset`
in their tests, CRLF line endings for the one crlf.cpp test, and auto for
everything else.
I'm open to any other thoughts, ideas, or hints about testing clang tools on
inputs with various line ending types.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97625
Files:
clang-tools-extra/test/.gitattributes
Index: clang-tools-extra/test/.gitattributes
===================================================================
--- /dev/null
+++ clang-tools-extra/test/.gitattributes
@@ -0,0 +1,19 @@
+# CRLF (Windows) line endings take two bytes instead of one, so any tests that
+# rely on or check fixed character -offset, Offset: or FileOffset: locations
+# will fail when run on input files checked out with different line endings.
+
+# Most test input files should use native line endings, to ensure that we run
+# tests against both line ending types.
+* text=auto
+
+# These test input files rely on one-byte Unix (LF) line-endings, as they use
+# fixed -offset, FileOffset:, or Offset: numbers in their tests.
+clang-apply-replacements/ClangRenameClassReplacements.cpp text eol=lf
+clang-apply-replacements/Inputs/basic/basic.h text eol=lf
+clang-apply-replacements/Inputs/format/no.cpp text eol=lf
+clang-apply-replacements/Inputs/format/yes.cpp text eol=lf
+clang-tidy/infrastructure/export-diagnostics.cpp text eol=lf
+
+# These test input files rely on two-byte Windows (CRLF) line endings.
+clang-apply-replacements/Inputs/crlf/crlf.cpp text eol=crlf
+clang-apply-replacements/Inputs/crlf/crlf.cpp.expected text eol=crlf
Index: clang-tools-extra/test/.gitattributes
===================================================================
--- /dev/null
+++ clang-tools-extra/test/.gitattributes
@@ -0,0 +1,19 @@
+# CRLF (Windows) line endings take two bytes instead of one, so any tests that
+# rely on or check fixed character -offset, Offset: or FileOffset: locations
+# will fail when run on input files checked out with different line endings.
+
+# Most test input files should use native line endings, to ensure that we run
+# tests against both line ending types.
+* text=auto
+
+# These test input files rely on one-byte Unix (LF) line-endings, as they use
+# fixed -offset, FileOffset:, or Offset: numbers in their tests.
+clang-apply-replacements/ClangRenameClassReplacements.cpp text eol=lf
+clang-apply-replacements/Inputs/basic/basic.h text eol=lf
+clang-apply-replacements/Inputs/format/no.cpp text eol=lf
+clang-apply-replacements/Inputs/format/yes.cpp text eol=lf
+clang-tidy/infrastructure/export-diagnostics.cpp text eol=lf
+
+# These test input files rely on two-byte Windows (CRLF) line endings.
+clang-apply-replacements/Inputs/crlf/crlf.cpp text eol=crlf
+clang-apply-replacements/Inputs/crlf/crlf.cpp.expected text eol=crlf
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
