Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature-Request] Add options to avoid installing some dependencies with Hunter #1021

Open
traversaro opened this issue May 13, 2024 · 8 comments
Labels
enhancement New feature or request priority: low

Comments

@traversaro
Copy link
Contributor

Start with the why:

I am trying to integrated depthai-core in an existing C++ code base in which many dependencies (such as opencv and pcl) are managed via the conda package manager, installing packages from conda-forge. In this context, I would like to minimize the chance of packages installed by conda to conflict with packages installed via Hunter. As discussed in #411, just setting HUNTER_ENABLE to OFF is not an option, as some dependencies are patched to work correctly with depthai-core, so you can't use the stock version. However, I would like to be able to use the conda version of dependencies that should work out of the box and I already used in the rest of the codebase, such as nlohmann_json, bzip2, spdlog and jsoncpp

Move to the what:

I would like to have a set of granular options to specify if hunter_add_package should be called or not for a package. Currently there is HUNTER_ENABLE to enable/disable hunter_add_package for all packages, but this can't work for the reason described in the previous section.

Move to the how:

My idea was to have options such as DEPTHAI_USE_EXTERNAL_<pkgname> for each pkgname . These variable will be mark_as_advanced and set to FALSE by default, to not interfere with existing workflow. If a given DEPTHAI_USE_EXTERNAL_<pkgname> option will be enabled, hunter_add_package(<pkgname>) will not be called, and the "normal" find_package(<pkgname>) will be used instead.

@traversaro traversaro added the enhancement New feature or request label May 13, 2024
@traversaro
Copy link
Contributor Author

I will be happy to work on a PR for this proposal, but I wanted first to check with maintainers if this is something you would be open to.

@themarpe
Copy link
Collaborator

Hi @traversaro

Thanks for bringing this up - and yes, it is something that I do view would be beneficial.

I lean towards Hunter actually, as it seems like the best place where this would be managed (& properly propagated to subdependencies as well)

See the (original) proposal: cpp-pm/hunter#557

I will be happy to work on a PR for this proposal, but I wanted first to check with maintainers if this is something you would be open to.

We'd be very open for this - so if its in form of Hunter, we'd gladly switch to that build of Hunter with this functionality in.


WRT the depthai-ros - there is a branch, ros-release which "locally" already makes an attempt at handling this for nlohmann_json (but note, its just change on DAI level - if there was a dependency that was still under Hunter and it relied on the disabled nlohmann json, it would still be pulled, as it doesn't propagate - which is why I brought up the change in Hunter instead)

@traversaro
Copy link
Contributor Author

Thanks for the quick reply! I see the point of transitive hunter dependencies, however in cpp-pm/hunter#557 it does not seem that upstream is really interested in that feature, or to have Hunter interact with any other package manager from what I understand.

To be completely honest, I never used Hunter, so clearly for me it is tricky to estimate the effort to actually get a fully fledged feature in Hunter itself, and I am not sure I will be able to look into that in the near future. On the other side, I see why you would like to have this in Hunter and not as local options in depthai-core.

@themarpe
Copy link
Collaborator

@traversaro

however in cpp-pm/hunter#557 it does not seem that upstream is really interested in that feature, or to have Hunter interact with any other package manager from what I understand.

I don't think they'd be against it per-se. Also, it functions similarly to HUNTER_ENABLE=OFF, but specifically.

IMO the approach would be much simpler going from Hunter side than from DAI side and we can rely on this fork of Hunter directly.

Let me know if you'll happen to check it out - would be interesting for a couple of integration paths (Yocto, ROS, ...)

@traversaro
Copy link
Contributor Author

traversaro commented May 14, 2024

I don't think they'd be against it per-se.

To keep it simple I just asked this to the mantainers in cpp-pm/hunter#557 (comment) .

@themarpe
Copy link
Collaborator

To keep it simple I just asked this to the mantainers

Even better! Thanks!

@themarpe
Copy link
Collaborator

@traversaro any updates from the Hunter's side of approach? If you fork works as expected, we could potentially utilize that as well, if upstream won't be too fast on merging

@traversaro
Copy link
Contributor Author

@traversaro any updates from the Hunter's side of approach? If you fork works as expected, we could potentially utilize that as well, if upstream won't be too fast on merging

Sorry for the radio silence. As mentioned in cpp-pm/hunter#557 (comment), I tried to impement the feature in way that handled corrected caching, but after spending a few hours on this, I could not find a solution, probably also due to my limited knowledge of Hunter's internals. Instead, I quickly implemented the solution with a modification of depthai-core side, and it has been working fine (see traversaro@649841a), so I am currently using that as a local patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: low
Projects
None yet
Development

No branches or pull requests

2 participants