[roottest] Use interpreter in reading step of TFile::MakeProject tests#21081
[roottest] Use interpreter in reading step of TFile::MakeProject tests#21081guitargeek wants to merge 1 commit intoroot-project:masterfrom
Conversation
The reading part of the `TFile::MakeProject` tests were a bit brittle,
because the CMake build system linked against shareded libraries that
are produced at runtime by ROOT with `MakeProject`.
That caused some problems, like:
1. The automatic RPATH that is set by CMake didn't work on macOS,
forcing us to set `DYLD_LIBRARY_PATH`.
2. A linker optimization issue on Windows (see 0ecab8e), which
had to be worked around.
To avoid these workarounds, this commit suggests to use the
auto-generated libraries as they are usually used also by our users:
dynamically by the interpreter at runtime.
Closes root-project#21076 by avoiding overwriting the existing `LD_LIBRARY_PATH`.
guitargeek
force-pushed
the
issue-21076
branch
from
4a67a82 to
c25ce65
Compare
|
|
||
| const auto &event = viewEvent(0); | ||
| // Get values out as plain STL types | ||
| auto event_foo = interpreter_get<std::bitset<16>>("&event.foo"); |
There was a problem hiding this comment.
This is cumbersome and thus once has to wonder whether we would be 'better' serve by switching from an executable to a script ... I suspect that the down side is to have to 'replace' the EXPECT_EQ macro (or maybe gtest works 'fine' in a script?)
There was a problem hiding this comment.
That would also be a good solution! There are many options and we'll discuss next week what to do
There was a problem hiding this comment.
That would also be a good solution! There are many options and we'll discuss next week what to do
vepadulano
left a comment
vepadulano
left a comment
There was a problem hiding this comment.
I discussed this PR on mattermost with @guitargeek . I see the issue it's trying to solve but I do not believe this is the right direction. In general, we cannot assume what way the users may want to use the library produced by MakeProject. If anything we should support all modes of operation (C++ programs, C++ macros, Python scripts). I believe #21083 goes in a better direction, although it is still not ready to be merged. I'm also trying things out myself locally, will give updates in case I find any better way.
Test Results 22 files 22 suites 3d 14h 52m 26s ⏱️ Results for commit c25ce65. |
|
Superseded by #21097 |
4 participants
The reading part of the
TFile::MakeProjecttests were a bit brittle, because the CMake build system linked against shareded libraries that are produced at runtime by ROOT withMakeProject.That caused some problems, like:
DYLD_LIBRARY_PATH.To avoid these workarounds, this commit suggests to use the auto-generated libraries as they are usually used also by our users: dynamically by the interpreter at runtime.
Closes #21076 by avoiding overwriting the existing
LD_LIBRARY_PATH.