search

rock-core / base-types

C/C++ and Ruby definition of the base types in Rock
6 stars 40 forks source link

Make SISL truely optional #134

Closed 2maz closed 5 years ago

2maz commented 5 years ago

@saarnold

should be applied in combination with: rock-core/tools-service_discovery#9 base-orogen-types also requires adaption which is WIP: rock-core/base-orogen-types#29

doudou commented 5 years ago

Please don't make it dependent on the presence or not of SISL, which will only cause pain for people that for whatever reason won't have SISL available but expect it to be there.

Just add a USE_SISL option that people can turn to OFF explicitely if they don't want SISL.

2maz commented 5 years ago

Updated

planthaber commented 5 years ago

Please don't make it dependent on the presence or not of SISL, which will only cause pain for people that for whatever reason won't have SISL available but expect it to be there.

Isn't this the default way ./configure scripts and any other installation of software with optional dependencies work?

Even in most CMake libs, when a lib is not found I get a warning and have to provide e.g. SISL_PATH manually when it is not found.

What about a WITH_SISL define in Trajectory.hpp header that removed the code and produces an error "base-types was compiled without SISL, base::Trajectory is not available" on compile time when the header is used?

2maz commented 5 years ago

What about a WITH_SISL define in Trajectory.hpp header that removed the code and produces an error "base-types was compiled without SISL, base::Trajectory is not available" on compile time when the header is used?

Regarding the discussion on base/orogen/types then SISL is rather a dependency for Rock, while it should still be optional in base/types to facilitate the external use. Thus I agree with @doudou to have the option and thereby an explicit way of controlling the need for SISL, while having a error reported at configuration of base/types if SISL is missing.