rorth wrote:

> > 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 ;-)


https://github.com/llvm/llvm-project/pull/142353
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to