arpg / HAL

Hardware Abstraction Layer library.
Other
36 stars 24 forks source link

HAL CMake build system - redundant libraries #116

Closed Algomorph closed 9 years ago

Algomorph commented 9 years ago

The way HAL's build system is structured, each driver finds it's own dependencies via calls to find_package(...), then adds them to some internal variables via add_to_hal_libraries and add_to_hal_include_dirs. After all drivers are processed, these internal variables are read off and added to HAL_LIBS and HAL_INCLUDES, which are then passed to the install_package command.

This results in an unhealthy amount of redundant libraries in the linker path for projects using HAL. For instance, the Convert driver and the OpenCV driver both add the same opencv libraries. Later, opencv is added a few more times by other dependencies. 1) Should add_to_hal_libraries and add_to_hal_include_dirs be altered to avoid re-adding things that are already there? 2) Why does "add_to_hal_libraries" expand the paths of "imported" libraries even when the paths are in the system library path, i.e. /user/lib and /user/local lib? Can this be avoided?

crheckman commented 9 years ago

Hi there,

Thanks for bringing this issue up; these macros are needed because of the directory structure of HAL, which (in theory) allows external parties to create new drivers without having to change CMakeLists.txt files in the upper level directories and including an extra call to add_subdirectory. However as you've indicated, there is some ambivalence to what other drivers are doing that results from this.

To respond to issue 1): Currently we use a command in the install_package cmake function to remove duplicate entries that have been added to an include dir or library dir list, which should prune the list considerably. Is this not the behavior that you see?

As for issue 2): Using add_to_hal_libraries in the way you propose would be challenging since a priori it is not clear which directories are "system library paths" once they're expanded by cmake. Is this causing bad behavior on your end? For example, is it adding libraries that are in the system path and then trying to re-install them when passed to install_package?

Algomorph commented 9 years ago

Thanks for looking into this. For (1), it seems to be working. I guess I first started looking into it when I saw the absolute path to libopencv_core and simple "opencv_core" both in the compile path. I wasn't aware of the subsequent cleanup, so I checked the contents of the HAL_LIBS variable and had an "OH MY GOD!" reaction :-) For (2) , no, it really is not causing any bad behavior. I suppose linking -l/usr/local/lib/libopencv_core.so and -lopencv_core doesn't upset the linker.