search

iLCSoft / LCIO

Linear Collider I/O
BSD 3-Clause "New" or "Revised" License
17 stars 34 forks source link

t_pyLCIO_import test broken on some systems #134

Closed tmadlener closed 3 years ago

tmadlener commented 3 years ago

The t_pyLCIO_import test seems to be broken (at least partially). See, e.g. the CI workflow outputs of #132: e.g. centos

I can reproduce this locally on Ubuntu as well

tmadlener commented 3 years ago

This is because of the following loading logic: https://github.com/iLCSoft/LCIO/blob/8f9e86b93b7d5d83221fabb872ed7e82f1638476/src/python/pyLCIO/base/SetupLcioDictionary.py#L16-L31

This logic only works if LCIO is installed back into its source directory, because it implicitly assumes where the libraries should be located and tries to load them via a full path. However, this is not strictly necessary, as ROOT will look on LD_LIBRARY_PATH in any case if just the library name is provided.

gaede commented 3 years ago

Yes, we have in principle always made in-source installations for LCIO and iLCSoft releases. This is exactly the kind of stuff, we'd have to fix across all packages when modernizing the cmake...

tmadlener commented 3 years ago

But even with the in-source installation the LD_LIBRARY_PATH points to the correct lib (or lib64) directory, right? So the lookup would not need to depend on the way things are installed as long as LD_LIBRARY_PATH is set correctly.

gaede commented 3 years ago

Yes, this was just meant as an explanation, why it was done that way. Please feel free to change as needed...

tmadlener commented 3 years ago

OK. Very good, I just wanted to make sure my assumption about the LD_LIBRARY_PATH is correct. In #136 I have made the lookup such that we first try to use ROOT and its automatic loading via LD_LIBRARY_PATH, before we fall back to the in-source installation lookup logic.

As a nice side-effect this makes this test independent of the way LCIO is installed (or would be installed) and now all tests can be run without installing first.