osqp 0.6.2 and 0.6.3 with Ninja generator + osqp build options #23539
Conversation
tkhyn
changed the title
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Since only either static or shared libraries get packaged, ultimately, I would recommend simply disabling the other one based on BUILD_SHARED_LIBS instead.
There was a problem hiding this comment.
I like this approach better too
recipes/osqp/all/conanfile.py
Outdated
| "ctrlc": [True, False], | ||
| "dfloat": [True, False], | ||
| "dlong": [True, False], | ||
| "coverage": [True, False], |
There was a problem hiding this comment.
coverage does not really make much sense as a package option. It only has an effect when unit tests are being run.
There was a problem hiding this comment.
Yes, as a library user I don't really care about coverage so happy to skip (done in commits I just pushed), but as the UNITTEST option was handled by the recipe possibly the COVERAGE could be too.
| "dfloat": False, | ||
| "dlong": True, | ||
| "coverage": False, | ||
| "mkl_paradisio": True | ||
| } |
There was a problem hiding this comment.
An options_description = {...} field would be nice, since the meaning of the options isn't super obvious.
recipes/osqp/all/conanfile.py
Outdated
| "printing": [True, False], | ||
| "profiling": [True, False], | ||
| "ctrlc": [True, False], | ||
| "dfloat": [True, False], | ||
| "dlong": [True, False], | ||
| "coverage": [True, False], | ||
| "mkl_paradisio": [True, False] |
There was a problem hiding this comment.
I would suggest skipping the mkl_paradisio option as it is being replaced by OSQP_ALGEBRA_BACKEND in the upcoming v1 release and would have to be deprecated. https://github.com/osqp/osqp/blob/master/CMakeLists.txt#L77-L79
recipes/osqp/all/conanfile.py
Outdated
| } | ||
| default_options = { | ||
| "shared": False, | ||
| "fPIC": True, | ||
| "printing": True, | ||
| "profiling": True, | ||
| "ctrlc": True, |
There was a problem hiding this comment.
Has been renamed to interrupt in upcoming 1.0 version. Might be worth renaming here as well. https://github.com/osqp/osqp/blob/master/CMakeLists.txt#L83
|
Thank you @valgur and @RubenRBS for the feedback. I have implemented the requested changes and learnt a few things on the way! Note: it doesn't look like |
|
Let's validate out the shared case for windows until 1.0 yes :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
No symbols are exported so the DLL cannot be linked against. Leaving the switches in package() method as this should be fixed in future versions
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 6 (
Conan v2 pipeline ✔️
All green in build 6 ( |
Specify library name and version: osqp/0.6.2 and osqp/0.6.3
This PR fixes issue #16821 by patching OSQP's cmake file to generate the shared and static libraries in different folders.
It also exposes the OSQP build options in the recipe.