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

Depend on cli component of ignition-utils #229

Merged
merged 2 commits into from
Mar 24, 2021

Conversation

scpeters
Copy link
Member

🎉 New feature

This is the new dependency part of #216

Summary

Add a dependency on the cli component of ignition-utils. This will be used to refactor the ign tool commands.

Test it

It should pass CI.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

Update migration guide

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters added the beta Targeting beta release of upcoming collection label Mar 24, 2021
@scpeters scpeters requested review from mjcarroll and chapulina March 24, 2021 20:55
@scpeters scpeters requested a review from caguero as a code owner March 24, 2021 20:55
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Mar 24, 2021
Copy link
Collaborator

@caguero caguero left a comment

Choose a reason for hiding this comment

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

Can we also update the installation instructions in the installation tutorial? There's a list of dependencies that we should update with libignition-utils1-cli-dev now.

@caguero
Copy link
Collaborator

caguero commented Mar 24, 2021

Can we also update the installation instructions in the installation tutorial? There's a list of dependencies that we should update with libignition-utils1-cli-dev now.

There's also docker/ign-transport/Dockerfile that needs to be updated with the extra dependency.

@scpeters
Copy link
Member Author

Can we also update the installation instructions in the installation tutorial? There's a list of dependencies that we should update with libignition-utils1-cli-dev now.

There's also docker/ign-transport/Dockerfile that needs to be updated with the extra dependency.

I've updated them in 34fa6c3

it might be useful for them to use packages.apt to reduce duplication

I didn't update the conda instructions because I'm not sure how those work

Copy link
Collaborator

@caguero caguero left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for the updates!

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

I didn't update the conda instructions because I'm not sure how those work

We don't need to worry about that right now

@chapulina chapulina merged commit f1f3447 into gazebosim:main Mar 24, 2021
@scpeters scpeters deleted the depend_utils_cli branch March 25, 2021 00:11
@chapulina
Copy link
Contributor

I think this is causing warnings downstream:

-- Looking for ignition-transport10 -- found version 10.0.0~pre2
-- Searching for dependencies of ignition-transport10
-- Found Protobuf: /usr/lib/x86_64-linux-gnu/libprotobuf.so;-lpthread;-lpthread (found suitable version "3.0.0", minimum required is "3") 
-- Config-file not installed for ZeroMQ -- checking for pkg-config
-- Checking for module 'libzmq >= 4'
--   Found libzmq , version 4.2.5
-- Found ZeroMQ: TRUE (Required is at least version "4") 
-- Looking for ignition-utils1 -- found version 1.0.0~pre1
-- Searching for dependencies of ignition-utils1
-- Searching for <ignition-utils1> component [cli]
CMake Warning at /usr/share/cmake-3.10/Modules/CMakeFindDependencyMacro.cmake:48 (find_package):
  By not providing "Findignition-utils1-cli.cmake" in CMAKE_MODULE_PATH this
  project has asked CMake to find a package configuration file provided by
  "ignition-utils1-cli", but CMake did not find one.

  Could not find a package configuration file provided by
  "ignition-utils1-cli" (requested version 1.0.0) with any of the following
  names:

    ignition-utils1-cliConfig.cmake
    ignition-utils1-cli-config.cmake

  Add the installation prefix of "ignition-utils1-cli" to CMAKE_PREFIX_PATH
  or set "ignition-utils1-cli_DIR" to a directory containing one of the above
  files.  If "ignition-utils1-cli" provides a separate development package or
  SDK, be sure it has been installed.
Call Stack (most recent call first):
  /usr/lib/x86_64-linux-gnu/cmake/ignition-utils1/ignition-utils1-config.cmake:202 (find_dependency)
  /usr/lib/x86_64-linux-gnu/cmake/ignition-transport10/ignition-transport10-config.cmake:96 (find_package)
  /usr/share/cmake/ignition-cmake2/cmake2/IgnUtils.cmake:188 (find_package)
  CMakeLists.txt:58 (ign_find_package)


CMake Warning at /usr/lib/x86_64-linux-gnu/cmake/ignition-transport10/ignition-transport10-config.cmake:96 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/ignition-utils1/ignition-utils1-config.cmake

  but it set ignition-utils1_FOUND to FALSE so package "ignition-utils1" is
  considered to be NOT FOUND.  Reason given by package:

  ignition-utils1 could not be found because dependency ignition-utils1-cli
  could not be found.

Call Stack (most recent call first):
  /usr/share/cmake/ignition-cmake2/cmake2/IgnUtils.cmake:188 (find_package)
  CMakeLists.txt:58 (ign_find_package)

https://build.osrfoundation.org/job/ignition_sensors-ci-pr_any-ubuntu_auto-amd64/656/consoleFull

@scpeters
Copy link
Member Author

I think this is causing warnings downstream:

-- Looking for ignition-transport10 -- found version 10.0.0~pre2
-- Searching for dependencies of ignition-transport10
-- Found Protobuf: /usr/lib/x86_64-linux-gnu/libprotobuf.so;-lpthread;-lpthread (found suitable version "3.0.0", minimum required is "3") 
-- Config-file not installed for ZeroMQ -- checking for pkg-config
-- Checking for module 'libzmq >= 4'
--   Found libzmq , version 4.2.5
-- Found ZeroMQ: TRUE (Required is at least version "4") 
-- Looking for ignition-utils1 -- found version 1.0.0~pre1
-- Searching for dependencies of ignition-utils1
-- Searching for <ignition-utils1> component [cli]
CMake Warning at /usr/share/cmake-3.10/Modules/CMakeFindDependencyMacro.cmake:48 (find_package):
  By not providing "Findignition-utils1-cli.cmake" in CMAKE_MODULE_PATH this
  project has asked CMake to find a package configuration file provided by
  "ignition-utils1-cli", but CMake did not find one.

  Could not find a package configuration file provided by
  "ignition-utils1-cli" (requested version 1.0.0) with any of the following
  names:

    ignition-utils1-cliConfig.cmake
    ignition-utils1-cli-config.cmake

  Add the installation prefix of "ignition-utils1-cli" to CMAKE_PREFIX_PATH
  or set "ignition-utils1-cli_DIR" to a directory containing one of the above
  files.  If "ignition-utils1-cli" provides a separate development package or
  SDK, be sure it has been installed.
Call Stack (most recent call first):
  /usr/lib/x86_64-linux-gnu/cmake/ignition-utils1/ignition-utils1-config.cmake:202 (find_dependency)
  /usr/lib/x86_64-linux-gnu/cmake/ignition-transport10/ignition-transport10-config.cmake:96 (find_package)
  /usr/share/cmake/ignition-cmake2/cmake2/IgnUtils.cmake:188 (find_package)
  CMakeLists.txt:58 (ign_find_package)


CMake Warning at /usr/lib/x86_64-linux-gnu/cmake/ignition-transport10/ignition-transport10-config.cmake:96 (find_package):
  Found package configuration file:

    /usr/lib/x86_64-linux-gnu/cmake/ignition-utils1/ignition-utils1-config.cmake

  but it set ignition-utils1_FOUND to FALSE so package "ignition-utils1" is
  considered to be NOT FOUND.  Reason given by package:

  ignition-utils1 could not be found because dependency ignition-utils1-cli
  could not be found.

Call Stack (most recent call first):
  /usr/share/cmake/ignition-cmake2/cmake2/IgnUtils.cmake:188 (find_package)
  CMakeLists.txt:58 (ign_find_package)

https://build.osrfoundation.org/job/ignition_sensors-ci-pr_any-ubuntu_auto-amd64/656/consoleFull

I had a related comment in #216 (comment) about finding ignition-utils1 as a private dependency. I think that would fix the warning at this time, but if at a later time we wanted to use ImplPtr or warning suppressions in a public header file, we would need to to export that dependency, and I'm not sure we could find the core component as PUBLIC and the CLI component as PRIVATE

the simplest workaround would be to add the -cli-dev as a runtime dependency of the -transport10-dev package. I was trying to avoid that, but it may be easier just to include it. It probably doesn't hurt anything

@chapulina
Copy link
Contributor

the simplest workaround would be to add the -cli-dev as a runtime dependency

Thanks, I'll do this for ign-transport and also ign-gazebo. Gazebo's examples are failing to compile unless users have libignition-utils1-cli-dev installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants