Skip to content

[clang][python][test] Check if libclang.so is loadable #142353

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

Closed
wants to merge 1 commit into from

Conversation

rorth
Copy link
Collaborator

@rorth rorth commented Jun 2, 2025

When building a 32-bit clang on a 64-bit system (like i686-pc-linux-gnu on a Linux/x86_64 system), ninja check-all fails:

FAILED: tools/clang/bindings/python/tests/CMakeFiles/check-clang-python tools/clang/bindings/python/tests/CMakeFiles/check-clang-python
cd clang/bindings/python && /usr/bin/cmake -E env CLANG_NO_DEFAULT_CONFIG=1 CLANG_LIBRARY_PATH=lib /usr/bin/python3.11 -m unittest discover
EEEEEEEE

and stops with exit 1.

Further investigation shows that, python3.11, a 64-bit binary, tries to load the freshly build 32-bit libclang.so, which cannot work, thus breaking the build.

Rather than trying to second-guess this situation, which seems very fragile, it's better to actually handle this situation when trying the load, which is what this patch does. The exact error message from cdll.LoadLibrary differs between systems:

  • On Linux, you get clang.cindex.LibclangError: lib/libclang.so: wrong ELF class: ELFCLASS32. while
  • on Solaris, there's clang.cindex.LibclangError: ld.so.1: python3.11: lib/libclang.so: wrong ELF class: ELFCLASS32.

To allow for both cases, this patch just looks for the common "wrong ELF class: ELFCLASS32".

Tested on amd64-pc-solaris2.11, i386-pc-solaris2.11, sparcv9-sun-solaris2.11, sparc-sun-solaris2.11, x86_64-pc-linux-gnu, and i686-pc-linux-gnu.

When building a 32-bit `clang` on a 64-bit system (like `i686-pc-linux-gnu`
on a Linux/x86_64 system), `ninja check-all` fails:

```
FAILED: tools/clang/bindings/python/tests/CMakeFiles/check-clang-python tools/clang/bindings/python/tests/CMakeFiles/check-clang-python
cd clang/bindings/python && /usr/bin/cmake -E env CLANG_NO_DEFAULT_CONFIG=1 CLANG_LIBRARY_PATH=lib /usr/bin/python3.11 -m unittest discover
EEEEEEEE
```
and stops with `exit 1`.

Further investigation shows that, `python3.11`, a 64-bit binary, tries to
load the freshly build 32-bit `libclang.so`, which cannot work, thus
breaking the build.

Rather than trying to second-guess this situation, which seems very
fragile, it's better to actually handle this situation when trying the
load, which is what this patch does.  The exact error message from
`cdll.LoadLibrary` differs between systems:

- On Linux, you get
  ```
  clang.cindex.LibclangError: lib/libclang.so: wrong ELF class: ELFCLASS32.
  ```
  while
- on Solaris, there's
  ```
  clang.cindex.LibclangError: ld.so.1: python3.11: lib/libclang.so: wrong ELF class: ELFCLASS32.
  ```

To allow for both cases, this patch just looks for the common `"wrong ELF class: ELFCLASS32"`.

Tested on `amd64-pc-solaris2.11`, `i386-pc-solaris2.11`,
`sparcv9-sun-solaris2.11`, `sparc-sun-solaris2.11`, `x86_64-pc-linux-gnu`,
and `i686-pc-linux-gnu`.
@rorth rorth requested a review from Endilll June 2, 2025 09:49
@rorth rorth requested a review from DeinAlptraum as a code owner June 2, 2025 09:49
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:as-a-library libclang and C++ API labels Jun 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 2, 2025

@llvm/pr-subscribers-clang

Author: Rainer Orth (rorth)

Changes

When building a 32-bit clang on a 64-bit system (like i686-pc-linux-gnu on a Linux/x86_64 system), ninja check-all fails:

FAILED: tools/clang/bindings/python/tests/CMakeFiles/check-clang-python tools/clang/bindings/python/tests/CMakeFiles/check-clang-python
cd clang/bindings/python && /usr/bin/cmake -E env CLANG_NO_DEFAULT_CONFIG=1 CLANG_LIBRARY_PATH=lib /usr/bin/python3.11 -m unittest discover
EEEEEEEE

and stops with exit 1.

Further investigation shows that, python3.11, a 64-bit binary, tries to load the freshly build 32-bit libclang.so, which cannot work, thus breaking the build.

Rather than trying to second-guess this situation, which seems very fragile, it's better to actually handle this situation when trying the load, which is what this patch does. The exact error message from cdll.LoadLibrary differs between systems:

  • On Linux, you get clang.cindex.LibclangError: lib/libclang.so: wrong ELF class: ELFCLASS32. while
  • on Solaris, there's clang.cindex.LibclangError: ld.so.1: python3.11: lib/libclang.so: wrong ELF class: ELFCLASS32.

To allow for both cases, this patch just looks for the common "wrong ELF class: ELFCLASS32".

Tested on amd64-pc-solaris2.11, i386-pc-solaris2.11, sparcv9-sun-solaris2.11, sparc-sun-solaris2.11, x86_64-pc-linux-gnu, and i686-pc-linux-gnu.


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

1 Files Affected:

  • (modified) clang/bindings/python/clang/cindex.py (+11-6)
diff --git a/clang/bindings/python/clang/cindex.py b/clang/bindings/python/clang/cindex.py
index 6f7243cdf80ac..b4e5898223f0f 100644
--- a/clang/bindings/python/clang/cindex.py
+++ b/clang/bindings/python/clang/cindex.py
@@ -4370,12 +4370,17 @@ def get_cindex_library(self) -> CDLL:
         try:
             library = cdll.LoadLibrary(self.get_filename())
         except OSError as e:
-            msg = (
-                str(e) + ". To provide a path to libclang use "
-                "Config.set_library_path() or "
-                "Config.set_library_file()."
-            )
-            raise LibclangError(msg)
+            if "wrong ELF class: ELFCLASS32" in str(e):
+                print("warning: skipping check-clang-python"
+                      " since libclang cannot be loaded", file=sys.stderr)
+                os._exit(0)
+            else:
+                msg = (
+                    str(e) + ". To provide a path to libclang use "
+                    "Config.set_library_path() or "
+                    "Config.set_library_file()."
+                )
+                raise LibclangError(msg)
 
         return library
 

Copy link

github-actions bot commented Jun 2, 2025

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r HEAD~1...HEAD clang/bindings/python/clang/cindex.py
View the diff from darker here.
--- cindex.py	2025-06-02 09:45:33.000000 +0000
+++ cindex.py	2025-06-02 09:52:37.367072 +0000
@@ -4369,12 +4369,15 @@
     def get_cindex_library(self) -> CDLL:
         try:
             library = cdll.LoadLibrary(self.get_filename())
         except OSError as e:
             if "wrong ELF class: ELFCLASS32" in str(e):
-                print("warning: skipping check-clang-python"
-                      " since libclang cannot be loaded", file=sys.stderr)
+                print(
+                    "warning: skipping check-clang-python"
+                    " since libclang cannot be loaded",
+                    file=sys.stderr,
+                )
                 os._exit(0)
             else:
                 msg = (
                     str(e) + ". To provide a path to libclang use "
                     "Config.set_library_path() or "

Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. Several points:

  1. What happens on Windows?
  2. check-clang-python is just one of the ways to run into this. cindex.py is a user-facing interface, so we shouldn't emit messages that make sense only during development.
  3. Given the fact that OSError already contains the message that indicates the problem, I think we should explore why it's not properly emitted in the first place, before introducing bespoke ways of delivering error messages. @DeinAlptraum any thoughts here?

@rorth
Copy link
Collaborator Author

rorth commented Jun 2, 2025

Thank you for the PR. Several points:

1. What happens on Windows?

I have no idea, knowing nothing at all about Windows. I cannot even tell if a similar situation can arise there, i.e. if you can execute 32-bit programs on a 64-bit Windows and, if you can, what the message would be if you tried. However, fixing an issue on one set of targets shouldn't force me to fix it everywhere: if/when such a situation arises elsewhere, any interested party can augment the code as necessary. Btw., Darwin could be similarly affected, but the last Darwin versions that could build and execute 32-bit programs are so old that I have no idea if they are still supported in LLVM.

2. `check-clang-python` is just one of the ways to run into this. `cindex.py` is a user-facing interface, so we shouldn't emit messages that make sense only during development.

I tried several different routes:

  • Add a check if cdll.LoadLibrary works to tests/CMakeLists.txt, setting RUN_PYTHON_TESTS to FALSE if not. However, this cannot work since libclang.so doesn't even exist at cmake time.
  • Alternatively (although I haven't tried that yet), one could a a similar small python script to the check-clang-python rule, not running the actual test if the library cannot be loaded.

Besides, one could just change the wording of the warning message: trying to load a 32-bit libclang.so into a 64-bit python is always an error, testsuite or no.

3. Given the fact that `OSError` already contains the message that indicates the problem, I think we should explore why it's not properly emitted in the first place, before introducing bespoke ways of delivering error messages. @DeinAlptraum any thoughts here?

Even if the error message were emitted, this doesn't help because ninja check-all is still broken, there being no way to disable check-clang-python even manually.

IMO the testsuite integration of check-clang-python is broken in various ways:

  • Any failure in this target aborts all of the testsuite run, as has been seen several times in the past, even without the issue at hand.
  • The result is reported in a very non-standard way: instead of emitting (say) PASS: libclang :: python bindings or whatever, you just get Ran 165 tests in ... unlike any other test. Instead, the tests should be reported like the PASS above (or UNSUPPORTED, XFAIL, ...). A failing test must not abort the whole testsuite, just report an appropriate result.

@DeinAlptraum
Copy link
Contributor

Given the fact that OSError already contains the message that indicates the problem, I think we should explore why it's not properly emitted in the first place, before introducing bespoke ways of delivering error messages. @DeinAlptraum any thoughts here?

This has been bothering me for a while but I didn't think too much about it. Took this opportunity to finally take a look and opened a fix PR #142371 .
TLDR; one of our tests captures stderr to keep the test output clean, but if the test crashes before it is released (usually when loading libclang.so raises as is the case here), the remaining tests in the run are effectively silenced. So this is an issue that only affected the tests.

@DeinAlptraum
Copy link
Contributor

DeinAlptraum commented Jun 2, 2025

trying to load a 32-bit libclang.so into a 64-bit python is always an error, testsuite or no.

That's why you shouldn't return an exit code of 0 when this error occurs.
Moreover, this seems like a workaround for that one specific issue you encountered. To connect this with:

Even if the error message were emitted, this doesn't help because ninja check-all is still broken, there being no way to disable check-clang-python even manually.

Wouldn't it be a better solution to wrap the check-clang-python target somehow as to change the exit code to 0? E.g. just pipe to true or echo or something like that. (is there an issue with that approach? I'm not that familiar with cmake)

Probably not relevant then, but is there a reason you called os._exit instead of sys.exit? os._exit skips exit handlers etc. so sys.exit is usually preferred (though I doubt it makes a difference here)

@rorth
Copy link
Collaborator Author

rorth commented Jun 2, 2025

trying to load a 32-bit libclang.so into a 64-bit python is always an error, testsuite or no.

That's why you shouldn't return an exit code of 0 when this error occurs. Moreover, this seems like a workaround for that one specific issue you encountered. To connect this with:

Even if the error message were emitted, this doesn't help because ninja check-all is still broken, there being no way to disable check-clang-python even manually.

Wouldn't it be a better solution to wrap the check-clang-python target somehow as to change the exit code to 0? E.g. just pipe to true or echo or something like that. (is there an issue with that approach? I'm not that familiar with cmake)

That might work, though I'm not sure if this also works on Windows or non-Unix systems in general. Alternatively, as I suggested, one could wrap the actual python -m unittest discover invocation with a check if libclang.so is loadable at all, only then running the actual test. I had something like

import os
from clang.cindex import Config
conf = Config()
Config.set_library_path(os.environ["CLANG_LIBRARY_PATH"])
conf.lib

initially until I realized that doing this at cmake time would disable the test everywhere (no libclang.so exists at that point).

One problem with all these approaches, as I'd already mentioned, is that the test outcome isn't properly reported, nor can one use the usual machinery to XFAIL the test or declare it UNSUPPORTED. That's all hardcoded inside test/CMakeLists.txt.

Probably not relevant then, but is there a reason you called os._exit instead of sys.exit? os._exit skips exit handlers etc. so sys.exit is usually preferred (though I doubt it makes a difference here)

I tried that at first, but got the warning: many times for every single subtest, which seems excessively verbose to me ;-)

@Endilll
Copy link
Contributor

Endilll commented Jun 2, 2025

@rorth Now that #142371 is merged, can you re-evaluate this PR and update the description?

@DeinAlptraum
Copy link
Contributor

DeinAlptraum commented Jun 4, 2025

I am unfortunately not at all familiar with how exactly the other tests are integrated and produce the outputs you described:

One problem with all these approaches, as I'd already mentioned, is that the test outcome isn't properly reported, nor can one use the usual machinery to XFAIL the test or declare it UNSUPPORTED. That's all hardcoded inside test/CMakeLists.txt.

and couldn't find much on it either, but that sounds like

Alternatively, as I suggested, one could wrap the actual python -m unittest discover invocation with a check if libclang.so is loadable at all, only then running the actual test.

is pretty much the way to go here: wrap the unittest call in an additional script that adapts the exit code and output format to what is used by the other tests, and then also add additional platform checks into that script.

You mentioned the problem of checking loadability at cmake time, but is this really necessary? Such a wrapper script could still parse the failure exceptions and emit an XFAIL at run time if wrong ELF class: ELFCLASS32 is encountered. But also, wouldn't you be able to check this at cmake time just by probing if we are on a 64bit system that's building 32bit?

@rorth
Copy link
Collaborator Author

rorth commented Jun 4, 2025

@rorth Now that #142371 is merged, can you re-evaluate this PR and update the description?

This PR changed the output from completely silent to excessively verbose: I now get

FAILED: tools/clang/bindings/python/tests/CMakeFiles/check-clang-python /var/llvm/local-i386-release-stage2-A-openmp/tools/clang/stage2-bins/tools/clang/bindings/python/tests/CMakeFiles/check-clang-python
cd /vol/llvm/src/llvm-project/local/clang/bindings/python && /usr/bin/cmake -E env CLANG_NO_DEFAULT_CONFIG=1 CLANG_LIBRARY_PATH=/var/llvm/local-i386-release-stage2-A-openmp/tools/clang/stage2-bins/lib /usr/bin/python3.11 -m unittest discover
EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE.E.EEEEEEEEE..EEEEEE.EEEEEEEEEEEEEE...EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE
======================================================================
ERROR: test_access_specifiers (tests.cindex.test_access_specifiers.TestAccessSpecifiers.test_access_specifiers)
Ensure that C++ access specifiers are available on cursors
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/clang/cindex.py", line 4371, in get_cindex_library
    library = cdll.LoadLibrary(self.get_filename())
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/ctypes/__init__.py", line 454, in LoadLibrary
    return self._dlltype(name)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/ctypes/__init__.py", line 376, in __init__
    self._handle = _dlopen(self._name, mode)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: ld.so.1: python3.11: /var/llvm/local-i386-release-stage2-A-openmp/tools/clang/stage2-bins/lib/libclang.so: wrong ELF class: ELFCLASS32

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/tests/cindex/test_access_specifiers.py", line 17, in test_access_specifiers
    tu = get_tu(
         ^^^^^^^
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/tests/cindex/util.py", line 30, in get_tu
    return TranslationUnit.from_source(name, args, unsaved_files=[(name, source)])
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/clang/cindex.py", line 3317, in from_source
    index = Index.create()
            ^^^^^^^^^^^^^^
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/clang/cindex.py", line 3189, in create
    return Index(conf.lib.clang_createIndex(excludeDecls, 0))
                 ^^^^^^^^
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/clang/cindex.py", line 250, in __get__
    value = self.wrapped(instance)
            ^^^^^^^^^^^^^^^^^^^^^^
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/clang/cindex.py", line 4343, in lib
    lib = self.get_cindex_library()
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/vol/llvm/src/llvm-project/local/clang/bindings/python/clang/cindex.py", line 4378, in get_cindex_library
    raise LibclangError(msg)
clang.cindex.LibclangError: ld.so.1: python3.11: /var/llvm/local-i386-release-stage2-A-openmp/tools/clang/stage2-bins/lib/libclang.so: wrong ELF class: ELFCLASS32. To provide a path to libclang use Config.set_library_path() or Config.set_library_file().

158 times total. While one can see what's going on, there's no way to avoid the issue.

@rorth
Copy link
Collaborator Author

rorth commented Jun 4, 2025

and couldn't find much on it either, but that sounds like

Alternatively, as I suggested, one could wrap the actual python -m unittest discover invocation with a check if libclang.so is loadable at all, only then running the actual test.

Indeed: one should move the test directory to (say) clang/test/bindings/python and add a script using the proper lit syntax. See e.g. llvm/utils/lit/tests/*.py. This way one can use the full set of XFAIL: etc. directives and automatically gets the expected output. This way one wouldn't even have to write the necessary lit.* scripts: it's all handled by the preexisting clang test framework.

is pretty much the way to go here: wrap the unittest call in an additional script that adapts the exit code and output format to what is used by the other tests, and then also add additional platform checks into that script.

You mentioned the problem of checking loadability at cmake time, but is this really necessary? Such a wrapper script could still parse the failure exceptions and emit an XFAIL at run time if wrong ELF class: ELFCLASS32 is encountered. But also, wouldn't you be able to check this at cmake time just by probing if we are on a 64bit system that's building 32bit?

Not at all: I just mentioned that attempt to describe why something along the lines of RUN_PYTHON_TESTS cannot be used here. It would even be easier (as I described) to just run a minimal script as described above first and only if that works run the actual tests. I'm pretty certain that it's way more reliable to just try loading libclang.so and react to failures, rather than trying to deduce the situation indirectly. This is quite similar to software that tries to reason if some filesystem access is possible rather than just trying it and let the kernel do its work (accept or reject the request). As in that case, there are so many factors that could affect this (like using a 32-bit python even on a 64-bit system) that would render all guesswork moot.

@rorth
Copy link
Collaborator Author

rorth commented Jun 4, 2025

FWIW I now have a prototype of the python tests within the lit framework. The basics (PASS if python and libclang.so match, FAIL if not) work, but there's more to be done:

  • The test should be UNSUPPORTED when libclang.so cannot be loaded.
  • The various ways RUN_PYTHON_TESTS can be set to FALSE need to be done with XFAIL: or UNSUPPORTED: in the new lit test.

@Endilll
Copy link
Contributor

Endilll commented Jun 4, 2025

We generally avoid XFAIL tests, instead we do normal tests where we expect the wrong behavior and leave a TODO.
In this case UNSUPPORTED makes more sense.

rorth added a commit to rorth/llvm-project that referenced this pull request Jun 5, 2025
As discussed in PR llvm#142353, the current testsuite of the `clang` Python
bindings has several issues:

- It `libclang.so` cannot be loaded into `python` to run the testsuite, the
  whole `ninja check-all` aborts.
- The result of running the testsuite isn't report like the `lit`-based
  tests, rendering them almost invisible.
- The testsuite is disabled in a non-obvious way (`RUN_PYTHON_TESTS`) in
  `tests/CMakeLists.txt`, which again doesn't show up in the test results.

All these issues can be avoided by integrating the Python bindings tests
with `lit`, which is what this patch does:

- The actual test lives in `clang/test/bindings/python/bindings.sh` and is
  run by `lit`.
- The current `clang/bindings/python/tests` directory (minus the
  now-subperfluous `CMakeLists.txt`) is moved into the same directory.
- The check if `libclang` is loadable (originally from PR llvm#142353) is now
  handled via a new `lit` feature, `libclang-loadable`.
- The various ways to disable the tests have been turned into `XFAIL`s as
  appropriate.  This isn't complete and not completely tested yet.

Tested on `sparc-sun-solaris2.11`, `sparcv9-sun-solaris2.11`,
`i386-pc-solaris2.11`, `amd64-pc-solaris2.11`, `i686-pc-linux-gnu`, and
`x86_64-pc-linux-gnu`.
@rorth
Copy link
Collaborator Author

rorth commented Jun 5, 2025

I've now created PR #142948 with an initial implementation of lit integration of the clang Python bindings tests.

@rorth
Copy link
Collaborator Author

rorth commented Jun 10, 2025

Superceded by PR #142948.

@rorth rorth closed this Jun 10, 2025
DeinAlptraum pushed a commit to DeinAlptraum/llvm-project that referenced this pull request Jun 22, 2025
As discussed in PR llvm#142353, the current testsuite of the `clang` Python
bindings has several issues:

- It `libclang.so` cannot be loaded into `python` to run the testsuite, the
  whole `ninja check-all` aborts.
- The result of running the testsuite isn't report like the `lit`-based
  tests, rendering them almost invisible.
- The testsuite is disabled in a non-obvious way (`RUN_PYTHON_TESTS`) in
  `tests/CMakeLists.txt`, which again doesn't show up in the test results.

All these issues can be avoided by integrating the Python bindings tests
with `lit`, which is what this patch does:

- The actual test lives in `clang/test/bindings/python/bindings.sh` and is
  run by `lit`.
- The current `clang/bindings/python/tests` directory (minus the
  now-subperfluous `CMakeLists.txt`) is moved into the same directory.
- The check if `libclang` is loadable (originally from PR llvm#142353) is now
  handled via a new `lit` feature, `libclang-loadable`.
- The various ways to disable the tests have been turned into `XFAIL`s as
  appropriate.  This isn't complete and not completely tested yet.

Tested on `sparc-sun-solaris2.11`, `sparcv9-sun-solaris2.11`,
`i386-pc-solaris2.11`, `amd64-pc-solaris2.11`, `i686-pc-linux-gnu`, and
`x86_64-pc-linux-gnu`.
rorth added a commit to rorth/llvm-project that referenced this pull request Jun 25, 2025
As discussed in PR llvm#142353, the current testsuite of the `clang` Python
bindings has several issues:

- It `libclang.so` cannot be loaded into `python` to run the testsuite, the
  whole `ninja check-all` aborts.
- The result of running the testsuite isn't report like the `lit`-based
  tests, rendering them almost invisible.
- The testsuite is disabled in a non-obvious way (`RUN_PYTHON_TESTS`) in
  `tests/CMakeLists.txt`, which again doesn't show up in the test results.

All these issues can be avoided by integrating the Python bindings tests
with `lit`, which is what this patch does:

- The actual test lives in `clang/test/bindings/python/bindings.sh` and is
  run by `lit`.
- The current `clang/bindings/python/tests` directory (minus the
  now-subperfluous `CMakeLists.txt`) is moved into the same directory.
- The check if `libclang` is loadable (originally from PR llvm#142353) is now
  handled via a new `lit` feature, `libclang-loadable`.
- The various ways to disable the tests have been turned into `XFAIL`s as
  appropriate.  This isn't complete and not completely tested yet.

Tested on `sparc-sun-solaris2.11`, `sparcv9-sun-solaris2.11`,
`i386-pc-solaris2.11`, `amd64-pc-solaris2.11`, `i686-pc-linux-gnu`, and
`x86_64-pc-linux-gnu`.
rorth added a commit that referenced this pull request Jun 25, 2025
…2948)

As discussed in PR #142353, the current testsuite of the `clang` Python
bindings has several issues:

- If `libclang.so` cannot be loaded into `python` to run the testsuite,
the whole `ninja check-all` aborts.
- The result of running the testsuite isn't report like the `lit`-based
tests, rendering them almost invisible.
- The testsuite is disabled in a non-obvious way (`RUN_PYTHON_TESTS`) in
`tests/CMakeLists.txt`, which again doesn't show up in the test results.

All these issues can be avoided by integrating the Python bindings tests
with `lit`, which is what this patch does:

- The actual test lives in `clang/test/bindings/python/bindings.sh` and
is run by `lit`.
- The current `clang/bindings/python/tests` directory (minus the
now-superfluous `CMakeLists.txt`) is moved into the same directory.
- The check if `libclang` is loadable (originally from PR #142353) is
now handled via a new `lit` feature, `libclang-loadable`.
- The various ways to disable the tests have been turned into `XFAIL`s
as appropriate. This isn't complete and not completely tested yet.
- It keeps the `check-clang-python` target for use by the Clang Python
CI.

Tested on `sparc-sun-solaris2.11`, `sparcv9-sun-solaris2.11`,
`i386-pc-solaris2.11`, `amd64-pc-solaris2.11`, `i686-pc-linux-gnu`, and
`x86_64-pc-linux-gnu`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:as-a-library libclang and C++ API clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants