Skip to content

Conversation

dyung
Copy link
Collaborator

@dyung dyung commented Sep 12, 2025

#157765 added some additional tests for split-file and attempted to diff the output files to make sure they matched what was expected. The problem was that on Windows the files being created had Windows line endings '\r\n' which was causing the diff to fail because the comparison files were presumably created on unix and did not have the '\r' character in the line endings.

This fixes it by passing --strip-trailing-cr to diff to tell it to ignore that character when doing the diff.

Should fix bot: https://lab.llvm.org/buildbot/#/builders/46/builds/23206

@llvmbot
Copy link
Member

llvmbot commented Sep 12, 2025

@llvm/pr-subscribers-testing-tools

Author: None (dyung)

Changes

#157765 added some additional tests for split-file and attempted to diff the output files to make sure they matched what was expected. The problem was that on Windows the files being created had Windows line endings '\r\n' which was causing the diff to fail because the comparison files were presumably created on unix and did not have the '\r' character in the line endings.

This fixes it by passing --strip-trailing-cr to diff to tell it to ignore that character when doing the diff.

Should fix bot: https://lab.llvm.org/buildbot/#/builders/46/builds/23206


Full diff: https://github.com/llvm/llvm-project/pull/158297.diff

1 Files Affected:

  • (modified) llvm/utils/lit/tests/diff-test-update.py (+7-7)
diff --git a/llvm/utils/lit/tests/diff-test-update.py b/llvm/utils/lit/tests/diff-test-update.py
index ad14034a85a17..8b9f4610f7f95 100644
--- a/llvm/utils/lit/tests/diff-test-update.py
+++ b/llvm/utils/lit/tests/diff-test-update.py
@@ -8,13 +8,13 @@
 
 # RUN: not %{lit} --update-tests -v %S/Inputs/diff-test-update | FileCheck %s
 
-# RUN: diff %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file.test
-# RUN: diff %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file-populated.test
-# RUN: diff %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file.test
-# RUN: diff %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file-populated.test
-# RUN: diff %S/Inputs/diff-test-update/single-split-file-no-expected.out %S/Inputs/diff-test-update/single-split-file-no-expected.test
-# RUN: diff %S/Inputs/diff-test-update/split-c-comments.out %S/Inputs/diff-test-update/split-c-comments.test
-# RUN: diff %S/Inputs/diff-test-update/split-whitespace.out "%S/Inputs/diff-test-update/split whitespace.test"
+# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file.test
+# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/single-split-file.out %S/Inputs/diff-test-update/single-split-file-populated.test
+# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file.test
+# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/multiple-split-file.out %S/Inputs/diff-test-update/multiple-split-file-populated.test
+# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/single-split-file-no-expected.out %S/Inputs/diff-test-update/single-split-file-no-expected.test
+# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/split-c-comments.out %S/Inputs/diff-test-update/split-c-comments.test
+# RUN: diff --strip-trailing-cr %S/Inputs/diff-test-update/split-whitespace.out "%S/Inputs/diff-test-update/split whitespace.test"
 
 
 # CHECK: # update-diff-test: could not deduce source and target from {{.*}}1.in and {{.*}}2.in

@dyung dyung enabled auto-merge (squash) September 12, 2025 14:09
@dyung
Copy link
Collaborator Author

dyung commented Sep 12, 2025

The precommit failure is also present in at least one build bot, so is unrelated to this change: https://lab.llvm.org/buildbot/#/builders/195/builds/14511

@dyung dyung disabled auto-merge September 12, 2025 14:52
@dyung dyung merged commit b0cb4e1 into llvm:main Sep 12, 2025
11 of 12 checks passed
@dyung dyung deleted the fix-diff-on-windows branch September 12, 2025 14:55
hnrklssn pushed a commit to hnrklssn/llvm-project that referenced this pull request Sep 12, 2025
hnrklssn pushed a commit to hnrklssn/llvm-project that referenced this pull request Sep 12, 2025
kateinoigakukun pushed a commit to kateinoigakukun/llvm-project that referenced this pull request Sep 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants