-
Notifications
You must be signed in to change notification settings - Fork 15k
[llvm-ar] Fix darwin-related tests' XFAIL annotation #130144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The tests updated by this commit are expected to fail when targetting a
darwin platform. As they rely on target detection based on the host,
this is currently checked by using the `XFAIL: system-darwin`
annotation.
This approach becomes a problem when trying to run such tests on a
cross-compiling build of clang on a darwin platform. When no darwin
targets are included in the build, the XFAIL will still apply even
though the target used is not related to the darwin platform, and the
tests will unexpectedly pass.
To fix this issue, this patch updates the condition used by the tests'
XFAIL annotation to `target={{.*}}-darwin{{.*}}`, ensuring they only are
xfailed when the relevant target is used.
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Lucas Duarte Prates (pratlucas) ChangesThe tests updated by this commit are expected to fail when targetting a This approach becomes a problem when trying to run such tests on a To fix this issue, this patch updates the condition used by the tests' Full diff: https://github.com/llvm/llvm-project/pull/130144.diff 2 Files Affected:
diff --git a/llvm/test/tools/llvm-ar/extract.test b/llvm/test/tools/llvm-ar/extract.test
index ccad703f3a149..bf46cc0743c7d 100644
--- a/llvm/test/tools/llvm-ar/extract.test
+++ b/llvm/test/tools/llvm-ar/extract.test
@@ -1,5 +1,5 @@
## Test extract operation.
-# XFAIL: system-darwin
+# XFAIL: target={{.*}}-darwin{{.*}}
# RUN: rm -rf %t && mkdir -p %t/extracted/
diff --git a/llvm/test/tools/llvm-ar/print.test b/llvm/test/tools/llvm-ar/print.test
index a27cab4a227e1..997c05f225a8b 100644
--- a/llvm/test/tools/llvm-ar/print.test
+++ b/llvm/test/tools/llvm-ar/print.test
@@ -1,5 +1,5 @@
## Test Print output
-# XFAIL: system-darwin
+# XFAIL: target={{.*}}-darwin{{.*}}
# RUN: rm -rf %t && mkdir -p %t
# RUN: echo file1 > %t/1.txt
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a Mac to test this on. It seems simple enough and I'm fairly confident it is correct. I assume you have verified that this does indeed XFAIL the tests under the condition you expect it to?
One request: please change the tag in the title to [llvm-ar], since this change has nothing to do with clang. LGTM with that.
|
Thanks for the review! I did verify it on a Mac, both with a regular LLVM build and a cross-compiling downstream project (arm/arm-toolchain). I've updated the PR title, which will be final commit message. Sorry about mixing it up, that's what I get for copy-pasting the commit message from #130138 :) |
This cherry-picks the upstream change from llvm/llvm-project#130144, which landed on the `main` branch, into our 20.x release. The change is included as a patch file for llvm.
This cherry-picks the upstream change from llvm/llvm-project#130144, which landed on the `main` branch, into our 20.x release. The change is included as a patch file for llvm.
|
@pratlucas This change effectively un-XFAILed the tests for LLVM builds through the Swift build script because we use Apple platform names for the LLVM target triple rather than |
The history of these XFAILs is clouded. Initially added as target-based checks in llvm@629273e for somewhat vague reasons, soon after switched to host-based checks in llvm@20b2cd3 "due to the problem being related to the darwin host and not the target", and recently reverted back to target-based in llvm#130144 because, to my understanding, the tests can unexpectedly pass on arm64 macOS with `llvm-ar` cross-compiled to baremetal arm64. Anyway, the problem with the current `target={{.*}}-darwin{{.*}}` is that the Swift build script uses Apple platform names like 'macosx', not 'darwin', for the LLVM target triple, e.g. `arm64-apple-macosx13.0`.
…L for some target triples
The problem with the current `target={{.*}}-darwin{{.*}}` XFAIL
condition (changed in llvm#130144)
is that the Swift build script uses Apple platform names like 'macosx',
not 'darwin', for the LLVM target triple, e.g. `arm64-apple-macosx13.0`.
These tests appear to have been originally XFAILed because the default
format on macOS (darwin) adds newlines as padding to align members.
See: https://github.com/llvm/llvm-project/blob/ee8394d9469a2946ffe2e7d192c593ecf3f93098/llvm/lib/Object/ArchiveWriter.cpp#L897-L904
Use `--format=gnu` to cope with the differences in the output and
reenable the tests.
…L for some target triples (#159796) The problem with the current `target={{.*}}-darwin{{.*}}` XFAIL condition (changed in #130144) is that the Swift build script uses Apple platform names like 'macosx', not 'darwin', for the LLVM target triple, e.g. `arm64-apple-macosx13.0`. These tests appear to have been originally XFAILed because the default format on macOS (darwin) adds newlines as padding to align members. See: https://github.com/llvm/llvm-project/blob/ee8394d9469a2946ffe2e7d192c593ecf3f93098/llvm/lib/Object/ArchiveWriter.cpp#L897-L904 Use `--format=gnu` to cope with the differences in the output and reenable the tests. rdar://157213658
…L for some target triples (llvm#159796) The problem with the current `target={{.*}}-darwin{{.*}}` XFAIL condition (changed in llvm#130144) is that the Swift build script uses Apple platform names like 'macosx', not 'darwin', for the LLVM target triple, e.g. `arm64-apple-macosx13.0`. These tests appear to have been originally XFAILed because the default format on macOS (darwin) adds newlines as padding to align members. See: https://github.com/llvm/llvm-project/blob/ee8394d9469a2946ffe2e7d192c593ecf3f93098/llvm/lib/Object/ArchiveWriter.cpp#L897-L904 Use `--format=gnu` to cope with the differences in the output and reenable the tests. rdar://157213658 (cherry picked from commit 33e82e6)
…tent unXFAIL for some target triples (#159796)
The problem with the current `target={{.*}}-darwin{{.*}}` XFAIL
condition (changed in llvm/llvm-project#130144)
is that the Swift build script uses Apple platform names like 'macosx',
not 'darwin', for the LLVM target triple, e.g. `arm64-apple-macosx13.0`.
These tests appear to have been originally XFAILed because the default
format on macOS (darwin) adds newlines as padding to align members. See:
https://github.com/llvm/llvm-project/blob/ee8394d9469a2946ffe2e7d192c593ecf3f93098/llvm/lib/Object/ArchiveWriter.cpp#L897-L904
Use `--format=gnu` to cope with the differences in the output and
reenable the tests.
rdar://157213658
The tests updated by this commit are expected to fail when targetting a
darwin platform. As they rely on target detection based on the host,
this is currently checked by using the
XFAIL: system-darwinannotation.
This approach becomes a problem when trying to run such tests on a
cross-compiling build of clang on a darwin platform. When no darwin
targets are included in the build, the XFAIL will still apply even
though the target used is not related to the darwin platform, and the
tests will unexpectedly pass.
To fix this issue, this patch updates the condition used by the tests'
XFAIL annotation to
target={{.*}}-darwin{{.*}}, ensuring they only arexfailed when the relevant target is used.