Skip to content

Conversation

@madanial0
Copy link
Contributor

#119666 adds the -strip-trailing-cr flag to diff which is not supported on AIX switch to use the python implementation of diff instead

@llvmbot
Copy link
Member

llvmbot commented Dec 17, 2024

@llvm/pr-subscribers-clang-format

Author: Mark Danial (madanial0)

Changes

#119666 adds the -strip-trailing-cr flag to diff which is not supported on AIX switch to use the python implementation of diff instead


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

1 Files Affected:

  • (modified) clang/test/Format/lit.local.cfg (+8)
diff --git a/clang/test/Format/lit.local.cfg b/clang/test/Format/lit.local.cfg
index 8acf02725d701b..b060c79226cbda 100644
--- a/clang/test/Format/lit.local.cfg
+++ b/clang/test/Format/lit.local.cfg
@@ -1,3 +1,6 @@
+import platform
+import lit.formats
+
 # Suffixes supported by clang-format.
 config.suffixes = [
     ".c",
@@ -19,3 +22,8 @@ config.suffixes = [
     ".td",
     ".test"
 ]
+
+# AIX 'diff' command doesn't support --strip-trailing-cr, but the internal
+# python implementation does, so use that for cross platform compatibility
+if platform.system() == "AIX":
+    config.test_format = lit.formats.ShTest()

@w2yehia w2yehia requested a review from abhina-sree December 17, 2024 18:11
Copy link
Contributor

@w2yehia w2yehia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@abhina-sree abhina-sree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@madanial0 madanial0 merged commit 2a0091f into llvm:main Dec 17, 2024
9 of 10 checks passed
Comment on lines +28 to +29
if platform.system() == "AIX":
config.test_format = lit.formats.ShTest()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not for all platforms? cc @boomanaiden154

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.

5 participants