Avoid for binaries that depend on HPX to directly link against internal modules #3828
Conversation
| # integrate the hpx modules with the main library | ||
| set(_modules hpx_preprocessor) | ||
| foreach(_module ${_modules}) | ||
| # add module binaries as PRIVATE dependencies to the core hpx library to |
There was a problem hiding this comment.
You could use HPX_LIBS instead of _modules here (
Lines 8 to 11 in 18ab489
There was a problem hiding this comment.
By the way, while the hpx target should link to all modules, hpx_init and hpx_wrap might not need to once we have more of them.
| # add module include directories as PRIVATE to the hpx_init library to | ||
| # enable its compilation against indirectly included headers | ||
| get_target_property(_module_includes ${_module} INTERFACE_INCLUDE_DIRECTORIES) | ||
| target_include_directories(hpx_init PRIVATE ${_module_includes}) |
There was a problem hiding this comment.
Do we explicitly not want to link against the modules? Same for hpx_wrap.
There was a problem hiding this comment.
While compiling hpx_init (and hpx_wrap) the module include directories are needed. Applications using those libraries will see the include directories through the dependency on HPX itself.
There was a problem hiding this comment.
I meant why would target_link_libraries(hpx_init PRIVATE ${_module}) not be enough?
There was a problem hiding this comment.
Fair point. That should do the trick as well. The rationale for my change was that hpx_init and friends do not need to link against the modules, they only need to see the includes.
There was a problem hiding this comment.
Ok. Either is good, just wanted to make sure I understand since this is more code.
|
Looks good in general, but I'm wondering if you could give a bit more detail about where this becomes a problem? It must be in the build tree only, since we don't expose the modules from the installed config files. So tests and examples link unnecessarily to the internal modules? |
That was exactly the problem. The libraries where listed as (public and transitive) dependencies on the HPX libraries. Thus any binary that depends on HPX wanted to link against the modules as well. |
Ok, sounds reasonable. Just to clarify, this didn't cause any compilation problems (on Windows?), it was just unnecessary? In other words, did we miss anything with our testing? |
I did see problems on CircleCI while building Phylanx. There ninja was complaining about not being able to find |
Currently we propagate link dependencies on the HPX module to any binary that depends on HPX. This is not necessary and not desirable. External binaries should depend on the HPX core binaries (libhpx and friends) only. At the same time however, external modules need to be able to find the header files of the modules as those might be indirectly included through HPX API headers. Thus the the corresponding include directories must be exported publicly.