Skip to content

Disable CLI11 vendoring by default #167

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

Merged
merged 5 commits into from
Apr 21, 2025
Merged

Disable CLI11 vendoring by default #167

merged 5 commits into from
Apr 21, 2025

Conversation

scpeters
Copy link
Member

@scpeters scpeters commented Jan 8, 2025

🦟 Bug fix

Part of #135

Summary

CLI11 is available upstream, so don't vendor by default.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@scpeters
Copy link
Member Author

scpeters commented Jan 8, 2025

we may need to update the package metadata to ensure the cli11 dependency is installed properly in each platform. I don't see it in the gz-utils3 homebrew formula for example

@scpeters
Copy link
Member Author

scpeters commented Jan 8, 2025

we may need to update the package metadata to ensure the cli11 dependency is installed properly in each platform. I don't see it in the gz-utils3 homebrew formula for example

it's missing from both noble and homebrew, though homebrew doesn't report cmake warnings so its build status looks fine

@scpeters
Copy link
Member Author

scpeters commented Jan 8, 2025

we may need to update the package metadata to ensure the cli11 dependency is installed properly in each platform. I don't see it in the gz-utils3 homebrew formula for example

it's missing from both noble and homebrew, though homebrew doesn't report cmake warnings so its build status looks fine

updated metadata for homebrew in osrf/homebrew-simulation#2928 and Ubuntu in 6e8fead

the windows metadata still needs to be updated

scpeters added a commit to gazebo-tooling/release-tools that referenced this pull request Jan 10, 2025
@scpeters
Copy link
Member Author

the windows metadata still needs to be updated

adding cli11 to list of vcpkg dependencies to install in gazebo-tooling/release-tools#1230

I'm not sure what to do for conda (cc @traversaro)

@traversaro
Copy link
Contributor

I'm not sure what to do for conda (cc @traversaro)

At the conda-forge level, cli11 was already added as a dependency (and that was the reason why I added this option). For the gazebo internal conda-based Windows CI, I am not sure where dependencies are stored, perhaps @j-rivero may know more.

@traversaro
Copy link
Contributor

For the gazebo internal conda-based Windows CI, I am not sure where dependencies are stored, perhaps @j-rivero may know more.

Perhaps we need to add cli11 in https://github.com/gazebo-tooling/release-tools/blob/master/conda/envs/legacy/pixi.toml and re-generated the pixi.lock ?

@j-rivero
Copy link
Contributor

I've hijacked gazebo-tooling/release-tools#1230 to include the conda update.

@scpeters
Copy link
Member Author

I've hijacked gazebo-tooling/release-tools#1230 to include the conda update.

thanks! I'll mark this ready for review once we've merged gazebo-tooling/release-tools#1230

scpeters added a commit to gazebo-tooling/release-tools that referenced this pull request Jan 21, 2025
* Needed by gazebosim/gz-utils#167.
* Add conda-forge cli11 pixi-2.1.2

Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Jose Luis Rivero <[email protected]>
@scpeters scpeters marked this pull request as ready for review January 21, 2025 19:51
@scpeters scpeters requested a review from azeey as a code owner January 21, 2025 19:51
@scpeters
Copy link
Member Author

@osrf-jenkins run tests please

1 similar comment
@scpeters
Copy link
Member Author

@osrf-jenkins run tests please

@scpeters
Copy link
Member Author

this is ready for review

@scpeters scpeters requested a review from caguero April 17, 2025 22:30
@j-rivero
Copy link
Contributor

Support is not found in Windows.

CMake Warning at C:/J/workspace/gz_utils-pr-cnlwin/ws/install/gz-cmake4/share/cmake/gz-cmake4/cmake4/GzFindPackage.cmake:201 (find_package):
  By not providing "FindCLI11.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "CLI11", but
  CMake did not find one.

  Could not find a package configuration file provided by "CLI11" with any of
  the following names:

    CLI11Config.cmake
    cli11-config.cmake

  Add the installation prefix of "CLI11" to CMAKE_PREFIX_PATH or set
  "CLI11_DIR" to a directory containing one of the above files.  If "CLI11"
  provides a separate development package or SDK, be sure it has been
  installed.
Call Stack (most recent call first):
  CMakeLists.txt:37 (gz_find_package)


-- Looking for CLI11 - not found

@scpeters
Copy link
Member Author

Support is not found in Windows.

can you take a look at gazebo-tooling/release-tools#1307?

@scpeters scpeters requested a review from j-rivero April 18, 2025 18:54
@j-rivero
Copy link
Contributor

@osrf-jenkins run tests

Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working, thanks for the fix!

@github-project-automation github-project-automation bot moved this from In progress to In review in Core development Apr 21, 2025
Signed-off-by: Steve Peters <[email protected]>
@scpeters
Copy link
Member Author

I updated the migration guide in a4d96d6

@scpeters scpeters merged commit 572620d into main Apr 21, 2025
11 checks passed
@scpeters scpeters deleted the scpeters/unvendor_cli11 branch April 21, 2025 17:35
@github-project-automation github-project-automation bot moved this from In review to Done in Core development Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants