Add recipe for gz-sim-yarp-plugins package #25910
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CC @traversaro |
|
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/gz-sim-yarp-plugins:
|
I confirm I am ok in being a mantainer of this package. |
|
@traversaro the CI is failing for missing openGL: do you think I have to add other libraries to the |
Yes, mesa-libGL-devel, not sure it is not necessary for gazebo-yarp-plugins . |
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
traversaro
reviewed
Mar 29, 2024
traversaro
reviewed
Mar 29, 2024
traversaro
reviewed
Mar 29, 2024
|
Unfortunately the issue with openGL is persisting. Is the |
The Something else is going on. I guess the real error is: in gz-sim we are adding |
5 tasks
Tried using |
|
Sorry, I was confused. cdt are kind of special packages, and the yum_requirements.txt are only relevant for what regards loading those libraries, for example for running tests or similar, for compilation or Cmake configuration, we need to add CDT dependencies to the meta.yaml, as documented in https://conda-forge.org/docs/maintainer/knowledge_base/#libgl . |
|
@traversaro now the openGL issue seems to be solved. We are getting different errors on the CI for different OS: Linux:WindowsError on DetailsmacOSSome strange errors with Details |
|
For the linux failure tests, we have our own upstream problems (see robotology/gz-sim-yarp-plugins#75), so I would just manually enable in ctest (via the For Windows, it seems that somehow we are using mingw compilers instead of msvc? Let me check. macOS are probably some problem of old SDK and/or old deployment target being used, see conda-forge/protozfits-feedstock#24 (comment) for a related error. You can add a conda_build_config.yaml file to specify a greater value, see for example https://github.com/conda-forge/gazebo-yarp-plugins-feedstock/blob/4258eb6eda82a40d07840f1573ada25303a25143/recipe/conda_build_config.yaml and https://conda-forge.org/docs/maintainer/knowledge_base/#requiring-newer-macos-sdks for generic docs. |
I am not sure what is happening, but I guess that is just easier to install gtest as any other dependency: robotology/gz-sim-yarp-plugins#139 . |
Thanks! I put the |
|
With the last commits, the issue mentioned in #25910 (comment) are gone. Now I'm getting the same errors for all OSs:
Details |
You can safely ignore those, basically the conda-build machinery is automatically handling those. Just FYI you could add
Ah, that is indeed a problem, those are public dependencies of gz-sim, so we actually depend at runtime on the specific versions of those libraries, so we need to manually list them as depenendencies in the host section of the recipe. See conda-forge/gazebo-feedstock#107 for a generic discussion on the problem.
This is a valid warning. Basically we only depend on gtest for testing, so we should not depend at runtime on it. We can ignore the run_exports on it with a |
xela-95
force-pushed
the
add-gz-sim-yarp-plugins-pkg
branch
from
fe52999 to
7308cdb
Compare
|
@traversaro finally the CI is passing. The branch has also been rebased on main. |
Can you squash the history to a single commit? Thanks! |
xela-95
force-pushed
the
add-gz-sim-yarp-plugins-pkg
branch
from
7308cdb to
c24cddb
Compare
Done |
traversaro
reviewed
Apr 4, 2024
traversaro
reviewed
Apr 4, 2024
traversaro
reviewed
Apr 4, 2024
|
Ok, as the last step before the review, we need to provide a description in the original post of the PR, to introduce the package and explain a bunch of design decision to the reviewers, so to simplify their work, I suggest something like (inspired by packages I added in the past, see https://github.com/conda-forge/staged-recipes/pulls/traversaro): gz-sim-yarp-plugins are a set of plugins that expose the functionality of the gz-sim simulator via YARP devices. It is a spiritual successor to For consistency with
The feedstock is named |
|
To clarify @xela-95 you need to copy the description yourself, I can't modify your original post. |
|
Thank you @traversaro, I updated the PR description as suggested. |
|
@conda-forge/staged-recipes @conda-forge/help-c-cpp The recipe is ready for review, thanks a lot in advance! |
traversaro
approved these changes
Apr 4, 2024
|
To help direct your pull request to the best reviewers, please mention a topic-specifc team if your recipe matches any of the following: conda-forge/help-c-cpp, conda-forge/help-cdts, conda-forge/help-go, conda-forge/help-java, conda-forge/help-julia, conda-forge/help-nodejs, conda-forge/help-perl, conda-forge/help-python, conda-forge/help-python-c, conda-forge/help-r, conda-forge/help-ruby,or conda-forge/help-rust. Thanks! |
1 task
xhochy
requested changes
Apr 8, 2024
| - name: {{ cxx_name }} | ||
| script: build_cxx.sh # [unix] | ||
| script: bld_cxx.bat # [win] | ||
| build: |
There was a problem hiding this comment.
This probably should have a run_exports?
There was a problem hiding this comment.
Yes, it will have it eventually. However, at the moment the package does not install any header file, so it can't be actually consumed as a C++ library by downstream users (it can only be used at runtime by loading it via dlopen). Furthermore, we still need to define an ABI stability policy, so we did not add any run_exports. As soon as we do a release which install headers and define an ABI stability policy, we will add run_exports . See also the original post: #25910 (comment) .
Co-authored-by: Uwe L. Korn <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gz-sim-yarp-plugins are a set of plugins that expose the functionality of the gz-sim simulator via YARP devices. It is a spiritual successor to gazebo-yarp-plugins package, already packaged in conda-forge (see https://github.com/conda-forge/gazebo-yarp-plugins-feedstock).
For consistency with gazebo-yarp-plugins feedstock, the proposed recipe adds two different packages:
libgz-sim-yarp-plugins: This is the package that is meant to be used if the package is consumed as a C/C++ library, and linked to a downstream project. At the moment the package does not install any header, but as soon as it will do it, this package will get arun_exportssection. This package name contains thelibprefix as requested in Add recipe for libmatio-cpp #19764 (review) for C/C++ libraries.gz-sim-yarp-plugins: This is the package is an empty package that depends onlibgz-sim-yarp-pluginsand that is meant to be used if the plugins are just loaded at runtime in a downstream project, but not linked at compile time to any downstream project.The feedstock is named
gz-sim-yarp-pluginsto match the repo name and support different package outputs names in the future.Checklist
url) rather than a repo (e.g.git_url) is used in your recipe (see here for more details).