Deal with optional module dependencies - #724
Conversation
|
Polite ping 👼 The current situation does not allow portable use of any modules with optional dependencies on other modules :( |
|
Sorry for the delay, I need to allocate some more time to review it properly. Generally I would to avoid such intrusive changes into the build, but I need to evaluate the benefit |
|
I understand. I tried to think of something cleaner, but in the end this seemed like a decent trade-off. The only other option I can think of is to try to detect these optional dependencies. But Or you could try to detect needed modules based on the namespace of types/constants in items from the generated modules. But this would requires that opencv actually uses sub-namespaces for their modules and strictly adheres to it. I'm pretty sure they don't. Maybe you could ask libclang to figure our which module a type comes from by checking which header declared it. But then you probably run into issues with forward declarations. And a header with the definition may not actually have been parsed by clang, unless you pre-preemptively index all supported modules. I suppose one more option is to detect opencv build-time enabled modules, and have a list of predefined optional dependencies and selectively generate the bindings only for enabled modules that are an optional dependency of one of the modules enabled by a feature flag (recursively). But this also feels rather icky: it will probably end up with a very opencv-version specific list, which is hard to verify and hard to test. And if the feature flags are essentially ignored, this means that the code written by users of this crate don't need to correctly specify the needed features, which also makes their code less portable ("it works on my machine"). (Maybe this could be addressed by generating the bindings, but making them private, so only the other bindings can use them, but not the user.) Anyway, the approach in this PR tries instead makes the crate features take on the function of the build time configuration of the system opencv. It is indeed intrusive to disable a header and take over it's function, but I do think it's actually less error prone (unless you end up building all build-time enabled modules) (until opencv changes the way they manage their Sorry for the wall of text, but if anything here triggers a good idea it will be worth it :) |
This PR is an attempt to deal with optional inter-module dependencies.
For example, the
objdetectmodule optionally depends on thednnmodule. There is aCCheckerDetector::create(const dnn::Net &net)overload which is guarded behind#ifdef HAVE_OPENCV_DNN.The problem is:
HAVE_OPENCV_DNNis defined in a header<opencv2/opencv_modules.hpp>which is shipped with opencv itself. But the binding for thednnmodule (which includes thednn::Nettype) is not generated ifdnnfeature was not enabled.Right now this means that if your system opencv comes with the
dnnmodule, and you enable theobjdetectfeature but notdnn, you get compilation errors inside the opencv crate. On the other hand, if your system opencv doesn't come with thednnmodule, and you enable thednnfeature, you also get a compilation error. So if you want to useobjdetectand you don't care about thednnmodule, you have no good choice.This PR solves this by having all enabled module features become a
-DHAVE_OPENCV_$MODULEdefine for the code generation, and providing an alternative version of<opencv2/opencv_modules.hpp>without any defines in it.The end result: the optional code in
objdetectthat depends on thednnmodule will only be included in the bindings if thednnfeature is enabled \o/Additionally, the
objdetectmodule unconditionally depends on thefeaturesmodule, so I added that dependency inCargo.toml