HoboVR-Labs / issue_tracker

General issues to fix or features to be implemented
3 stars 0 forks source link

[FEATURE REQ]: Better dependency handling #6

Open okawo80085 opened 2 years ago

okawo80085 commented 2 years ago

Is your feature request related to a problem? Please describe. Our driver starts to grow in the number of external libraries it depends on, right now all that is handled in it's CMakeLists file and each library needs to be added by hand, each of them has different paths, weird source trees (looking at you openvr), etc.

Describe the solution you'd like Since we have forks of all of them external libs we're using, we should format them all to follow a single convention for source trees and build instructions, so that our driver's CMakeLists doesn't grow larger then the driver itself...

Describe alternatives you've considered I considered other build systems like bazel and a few python based ones, but the amount of effort needed to transition to those is too much imo.

Additional context If we don't come up with some standard to follow early on, it will really bite us in the ass later on.

okawo80085 commented 2 years ago

Almost done with this one actually, it was easier than i expected.

okawo80085 commented 2 years ago

So the solution i found, that is more or less painless, is: all dependencies must use CMake and be setup for tool chains, while also having a repo tree like this:

# lets say the name of the dep is "example"
# so the repo structure will look like this
example/
├── CMakeLists.txt
├── LICENSE
├── README.md
├── src
│   ├── example.h
│   └── # and other src files
└── test
    ├── CMakeLists.txt
    └── # test src files

The main dep build recipe should not set up any global variables or declarations, only target specific commands. It needs to have tests, but they need to be off by default (i'm not building dep tests in production, just no, our driver takes ages to compile as is). The main include header for the dep needs to be located (and named) in <dep_repo_root>/src/<dep_name>.h and the dep needs to declare only a single target named <dep_name>.

All of that to achieve this behavior when using the dep.

CMakeLists.txt in the main project:


...

# setup toolchain
add_subdirectory("<dep_name>")
set(<dep_name>_INCLUDE_DIR "<dep_repo_root>/src/")

...

# add dep to include path
target_include_directories(main_target
    ...
    ${<dep_name>_INCLUDE_DIR}
    ...
)

...

# link dep target
target_link_libraries(main_target
    <dep_name>
    ...
    ${CMAKE_DL_LIBS}
)

...
okawo80085 commented 2 years ago

It'll take some time to move all of our deps to this mental model (cough cough openvr cough cough), but it's worth it and most of our deps are already pretty much setup for this, requiring only a few tweaks (like disabling test builds or changing some compile options).