Conversation
|
With ogdf/ogdf#292 it should now also be possible to use non-vendored dependencies without editing source files , but if the current approach works there's also no big need to change it |
|
@N-Coder Hello! Thank you for your comments! Is there any chance of having a new release soon? So we could update the recipe to use external dependencies without needing extra changes. Regards! |
uilianries
left a comment
There was a problem hiding this comment.
@hedtke Hello! Thank you for your PR! I reviewed it with a few suggestions. Regards!
|
@uilianries Thank you for the feedback! Changes applied |
uilianries
left a comment
There was a problem hiding this comment.
@hedtke Thank you for applying those suggestions! 😄
I did a second review round with a few new suggestions 🙏
recipes/ogdf/all/conanfile.py
Outdated
There was a problem hiding this comment.
| required_conan_version = ">=2.1" |
@uilianries we don't have any scheduled yet as there aren't really any other new things yet, so for now it probably makes the most sense to stay with the current approach - sorry for bringing this up. 😅 |
hedtke
left a comment
There was a problem hiding this comment.
Ready for next round
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
|
@hedtke Hello! Thank you again for updating this PR! To avoid delay with this PR and wasting your time again in a new round, I made a few changes, including:
Regards! |
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
If we no longer include the cmake config file, how do dependent packages usually link against this package with conan? Some of our includes are now in non-default locations (to allow parallel debug and release installations that use different headers; see ogdf/ogdf#253) that are only picked up automatically when using cmake. |
|
@N-Coder Thank you again for following this PR changes 😄
Conan supports generators like CMakeDeps, which mimics the upstream cmake config files. This is the regular way to consume Conan packages, using those CMake config files guarantees all dependencies will be found correctly and will follow package_info() specification. People will run
We can handle different custom header folders using I'll double-check the result CMake files from the project and update the |
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
|
@hedtke @N-Coder I just addressed a few changes regarding Debug support. It was a present bug in the recipe, because the library Locally, I can build on Linux without errors: ogdf-2025.10-linux-amd64-gcc13-debug-static.log I did a small change, following @N-Coder point regarding the header folder, according the build type. I also checked the project produced cmake files, including |
|
LGTM |
Signed-off-by: Uilian Ries <uilianr@jfrog.com>
uilianries
left a comment
There was a problem hiding this comment.
LGTM Thank you @hedtke and @N-Coder for your great support and review 👏
@N-Coder please feel free to review/comment again regarding those last changes.
Reviewer Comments
- The PR ogdf/ogdf#253 not only shows the expected CMake targets, as the correct library name and include folder. Fixed the Debug build to use the expected suffix.
- Avoid using hardcoded copy, use CMake install with correct includedir in the package_info
- Using version range for pugixml as it should be safe according to the project: https://pugixml.org/docs/manual.html#overview.feedback
- Updated test package to validate the expected CMake target and headers subfolder according to the build type
- Added missing license file to the package
|
Does conan support debug/release versions of the same package out of the box or should we create a separate |
It supports both, depending on the config of the project downloads or compiles whatever the user requests. That's why my approach of copying headers has worked. You can have 4 or more projects on your machine, each requesting something different, like debug/release, shared/static, ... Conan downloads the source and compiles it This recipe supports debug/release, all operating systems, etc |
Summary
Changes to recipe: ogdf/2025.10
Motivation
New release happened in October 2025. Release on conan was requested here: ogdf/ogdf#214
Details
Add a 👍 reaction to pull requests you find important to help the team prioritize, thanks!