[PATCH] D97625: fix check-clang-tools tests that fail due to Windows CRLF line endings

[Conrad Poelman via Phabricator via cfe-commits]

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97625.326956.patch
Type: text/x-patch
Size: 1220 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210228/cb2a7eb4/attachment.bin>