[clang][python] Don't add check-clang-python to check-all if cross-compiling #111657
Conversation
…mpiling Consistent with other cases for these tests, we opt not to add the target to check-all if they're known to fail. The tests fail when cross compiling for a different architecture because the host Python3_EXECUTABLE is used to run them, and FFI calls will of course fail against the libraries compiled for the target. This is an alternate take on <llvm#111367>.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Alex Bradbury (asb) ChangesConsistent with other cases for these tests, we opt not to add the target to check-all if they're known to fail. The tests fail when cross compiling for a different architecture because the host Python3_EXECUTABLE is used to run them, and FFI calls will of course fail against the libraries compiled for the target. This is an alternate take on Full diff: https://github.com/llvm/llvm-project/pull/111657.diff 1 Files Affected:
diff --git a/clang/bindings/python/tests/CMakeLists.txt b/clang/bindings/python/tests/CMakeLists.txt
index 2543cf739463d9..de75d303aca7bb 100644
--- a/clang/bindings/python/tests/CMakeLists.txt
+++ b/clang/bindings/python/tests/CMakeLists.txt
@@ -47,6 +47,14 @@ if(${LLVM_NATIVE_ARCH} MATCHES "^(AArch64|Hexagon|Sparc|SystemZ)$")
set(RUN_PYTHON_TESTS FALSE)
endif()
+# Tests will fail if cross-compiling for a different target, as tests will try
+# to use the host Python3_EXECUTABLE and make FFI calls to functions in target
+# libraries.
+if(CMAKE_CROSS_COMPILING)
+ message(WARNING "check-clang-python-tests not added to check-all as they fail in a cross-build setup")
+ set(RUN_PYTHON_TESTS FALSE)
+endif()
+
if(RUN_PYTHON_TESTS)
set_property(GLOBAL APPEND PROPERTY
LLVM_ALL_ADDITIONAL_TEST_TARGETS check-clang-python)
|
| # Tests will fail if cross-compiling for a different target, as tests will try | ||
| # to use the host Python3_EXECUTABLE and make FFI calls to functions in target | ||
| # libraries. | ||
| if(CMAKE_CROSS_COMPILING) |
There was a problem hiding this comment.
If there's nothing fundamental preventing this from working, I'd like to left a FIXME, too.
There was a problem hiding this comment.
I think fundamentally it can't really work. We could add a search for a python interpreter built for the target in the sysroot, and add logic to use it if we know we can execute it with qemu-user...but I'm not sure it's worth that kind of complexity and I personally prefer to minimise dependency on the sysroot as much as possible other than providing some needed libraries and headers. A FIXME could suggest adding some logic for specifying a guest python3_executable, but I'm not sure that's a path we'd want to commit to. So I'm a bit torn.
There was a problem hiding this comment.
Do I understand correctly that you don't have this kind of problem with "regular" tests in clang/test? If so, I think it'd be novel to say that tests for entire components are second-class citizens simply because said components are written in Python. As far as commitments go, I think our default here is to support all kinds of tests.
That said, I understand that complexity is there (it appears that nothing is easy in cross-compiling), and what you described doesn't seem to be too convoluted, given the inherent complexity of the problem. We can leave this out of scope of this PR, but I'd expect that claiming official support for cross-compiling environments is going to be blocked on this.
There was a problem hiding this comment.
Yes, I'm trying for now to just make changes that remove the small bits of friction for cross-compiling and then testing using qemu-user as a litmus test rather than (for now) make it "officially supported".
It is indeed correct that there's no problem with the regular tests in clang/test (other than clang/test/Driver/env.c if you need QEMU_LD_PREFIX passed through, as the test strips envvars - but of course for the lit-orchestrated tests we already have a mechanism to skip tests downstream by configuring the LIT_ARGS).
There was a problem hiding this comment.
If I understand correctly, I think we probably do want a FIXME for clarity because we'd ultimately like the python tests to work in a cross-compile situation because I don't think there should be a distinction between running the C binding tests and the Python binding tests. Potentially we might be able to do this by hooking them in to the lit-orchestrated tests and using LIT_ARGS the same as the Clang tests?
Regardless, nothing has to be done for now beyond adding a FIXME comment; and if we decide the complexity is too great when digging into doing that effort, we can always walk back the comment.
There was a problem hiding this comment.
Thank you both for the comments, I've added a FIXME now that notes the issue without indicating a particular solution has been chosen.
…mpiling (llvm#111657) Consistent with other cases for these tests, we opt not to add the target to check-all if they're known to fail. The tests fail when cross compiling for a different architecture because the host Python3_EXECUTABLE is used to run them, and FFI calls will of course fail against the libraries compiled for the target. Do note that CMAKE_CROSSCOMPILING is set to true whenever CMAKE_SYSTEM_NAME was set manually <https://cmake.org/cmake/help/latest/variable/CMAKE_CROSSCOMPILING.html> so in some circumstances it may be set even when not cross-compiling. However, it's the best way of checking that CMake has right now, and we use it elsewhere in LLVM's build system.
…mpiling (llvm#111657) Consistent with other cases for these tests, we opt not to add the target to check-all if they're known to fail. The tests fail when cross compiling for a different architecture because the host Python3_EXECUTABLE is used to run them, and FFI calls will of course fail against the libraries compiled for the target. Do note that CMAKE_CROSSCOMPILING is set to true whenever CMAKE_SYSTEM_NAME was set manually <https://cmake.org/cmake/help/latest/variable/CMAKE_CROSSCOMPILING.html> so in some circumstances it may be set even when not cross-compiling. However, it's the best way of checking that CMake has right now, and we use it elsewhere in LLVM's build system.
…mpiling (llvm#111657) Consistent with other cases for these tests, we opt not to add the target to check-all if they're known to fail. The tests fail when cross compiling for a different architecture because the host Python3_EXECUTABLE is used to run them, and FFI calls will of course fail against the libraries compiled for the target. Do note that CMAKE_CROSSCOMPILING is set to true whenever CMAKE_SYSTEM_NAME was set manually <https://cmake.org/cmake/help/latest/variable/CMAKE_CROSSCOMPILING.html> so in some circumstances it may be set even when not cross-compiling. However, it's the best way of checking that CMake has right now, and we use it elsewhere in LLVM's build system.
Consistent with other cases for these tests, we opt not to add the target to check-all if they're known to fail. The tests fail when cross compiling for a different architecture because the host Python3_EXECUTABLE is used to run them, and FFI calls will of course fail against the libraries compiled for the target.
This is an alternate take on
#111367.