Skip to content

Deal with optional module dependencies - #724

Closed
de-vri-es wants to merge 3 commits into
twistedfall:masterfrom
de-vri-es:deal-with-optional-module-dependencies
Closed

de-vri-es wants to merge 3 commits into
twistedfall:masterfrom
de-vri-es:deal-with-optional-module-dependencies

Conversation

@de-vri-es

@de-vri-es de-vri-es commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

This PR is an attempt to deal with optional inter-module dependencies.

For example, the objdetect module optionally depends on the dnn module. There is a CCheckerDetector::create(const dnn::Net &net) overload which is guarded behind #ifdef HAVE_OPENCV_DNN.

The problem is: HAVE_OPENCV_DNN is defined in a header <opencv2/opencv_modules.hpp> which is shipped with opencv itself. But the binding for the dnn module (which includes the dnn::Net type) is not generated if dnn feature was not enabled.

Right now this means that if your system opencv comes with the dnn module, and you enable the objdetect feature but not dnn, you get compilation errors inside the opencv crate. On the other hand, if your system opencv doesn't come with the dnn module, and you enable the dnn feature, you also get a compilation error. So if you want to use objdetect and you don't care about the dnn module, you have no good choice.


This PR solves this by having all enabled module features become a -DHAVE_OPENCV_$MODULE define 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 objdetect that depends on the dnn module will only be included in the bindings if the dnn feature is enabled \o/

Additionally, the objdetect module unconditionally depends on the features module, so I added that dependency in Cargo.toml

@de-vri-es

Copy link
Copy Markdown
Contributor Author

Polite ping 👼

The current situation does not allow portable use of any modules with optional dependencies on other modules :(

@twistedfall

Copy link
Copy Markdown
Owner

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

@de-vri-es

Copy link
Copy Markdown
Contributor Author

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 libclang doesn't tell you about #ifdef directives. So the only thing you could do is detect the #define HAVE_OPENCV_* directives (which it does report). And then you will end up generating the bindings for all modules in the system opencv, making the crate features kinda useless.

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 #define HAVE_OPENCV_... list 🙄 ).

Sorry for the wall of text, but if anything here triggers a good idea it will be worth it :)

@de-vri-es

Copy link
Copy Markdown
Contributor Author

Did you perhaps have a change to think about the best way to deal with these optional module dependencies?

@twistedfall

Copy link
Copy Markdown
Owner

On the other hand, if your system opencv doesn't come with the dnn module, and you enable the dnn feature, you also get a compilation error.

@de-vri-es Can you please tell me if you encountered this error scenario in the actual code? In theory (unless there is a bug) the actual set of modules that the crate generates the bindings for is an intersection of what is chosen via cargo features and what actual modules OpenCV is compiled with. So for this scenario even if you enable DNN, the bindings for it won't be generated because OpenCV doesn't contain the dnn.hpp file. Again, in theory. If you can reproduce this scenario, can you post the actual error you are getting?

@twistedfall

Copy link
Copy Markdown
Owner

The problem is: HAVE_OPENCV_DNN is defined in a header <opencv2/opencv_modules.hpp> which is shipped with opencv itself. But the binding for the dnn module (which includes the dnn::Net type) is not generated if dnn feature was not enabled.

Previously I solved these kind of problems with cargo feature dependencies like this. This way enabling one module will automatically enable the dependency. You lose some flexibility this way, e.g. you can't drop "dnn" even if you don't need the additional functionality, but it does help prevent the compilation errors in particular configurations.

@de-vri-es

Copy link
Copy Markdown
Contributor Author

Sure, reproducing this is easy. You don't even need to write any code to trigger the build failure, since the compile error is in the opencv crate itself.

It depends on your opencv build though, but with Arch Linux:

$ cargo init
$ cargo add opencv --no-default-features --features objdetect,features
$ cargo build

Gives:

   Compiling opencv v0.100.1
error[E0433]: cannot find `dnn` in `crate`
    --> /tmp/2026-09-16-20-48-18/foo/target/debug/build/opencv-e65d366b809fc8fe/out/opencv/objdetect.rs:6123:37
     |
6123 | ...   pub fn create_1(net: &impl crate::dnn::NetTraitConst) -> Result<core::Ptr<crate::objdetect...
     |                                         ^^^ could not find `dnn` in the crate root

For more information about this error, try `rustc --explain E0433`.
error: could not compile `opencv` (lib) due to 1 previous error
✗ - status code 101

Enabling the "dnn" feature would create a different problem. If I do that, the project will fail to compile if opencv was built without "dnn". So either way, the project isn't portable, even though it doesn't use "dnn" at all.

That why I really think these optional module dependencies need special handling.

@de-vri-es

de-vri-es commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Oh, hmm, I just read this bit:

They are all enabled by default, but if a corresponding module is not found then it will silently be ignored.

🤔

I was assuming asking for a module that's not available would be an error. If not, enabling the feature does work.

Then the only advantage of this PR is that it lets you build less bindings, which saves a bit of time. But then I'm not convinced it's worth it mysel 😄

So... should we just add make the objdetect feature enable the features and dnn features then?

/edit: See #725. And then I guess this can be closed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants