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.

Sign up for GitHub

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

Jump to bottom

Compute relative path for cmake extras #362

Merged
merged 2 commits into from
Jul 6, 2023
Merged

Compute relative path for cmake extras #362

merged 2 commits into from
Jul 6, 2023

Conversation

scpeters
Copy link
Member

@scpeters scpeters commented Jul 6, 2023

🦟 Bug fix

Needed by gazebosim/gz-transport#398, follow-up to #360 and gazebosim/gz-msgs#339 (comment)

Summary

As noted in gazebosim/gz-msgs#339 (comment), packages that install extra cmake files may need the relative path from the folder to which the extra cmake files are installed and the install prefix. In order to account for arch-specific lib folders (needed for debian installs to /usr, see GNUInstallDirs), some code is rearranged so that the relative path is computed after the _gz_setup_packages() macro, since that is where GNUInstallDirs is included from, and before the extra cmake files are configured.

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 added 2 commits July 5, 2023 16:42
@scpeters
Handle cmake extras after _gz_setup_packages()
3cd1d60 
This is needed by a subsequent step that depends on
variables defined by GNUInstallDirs.

Signed-off-by: Steve Peters <[email protected]>
@scpeters
cmake extras: compute relative path to prefix
8df53dd 
Define a variable in gz_configure_project with the
relative path from the extras install folder to the
install prefix. This is needed by downstream packages.

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters requested review from mxgrey and j-rivero as code owners July 6, 2023 05:34
@github-actions github-actions bot added 🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic labels Jul 6, 2023
@scpeters scpeters mentioned this pull request Jul 6, 2023
Closed
@j-rivero
Copy link
Contributor

j-rivero commented Jul 6, 2023

Testing the change Build Status

mjcarroll
mjcarroll approved these changes Jul 6, 2023
View reviewed changes
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Good catch! Changes make sense to me, and looks good with green CI.

@scpeters
Copy link
Member Author

scpeters commented Jul 6, 2023

Testing the change Build Status

+ wget -O orig_tarball gz-cmake3
--2023-07-06 14:52:04--  http://gz-cmake3/
Resolving gz-cmake3 (gz-cmake3)... failed: Name or service not known.
wget: unable to resolve host address ‘gz-cmake3’
Build step 'Execute shell' marked build as failure

@scpeters
Copy link
Member Author

scpeters commented Jul 6, 2023

I'd like to merge this and then make a nightly build of gz-cmake3 to continue testing before making a patch release with this change

@mjcarroll
Copy link
Contributor

I'd like to merge this and then make a nightly build of gz-cmake3 to continue testing before making a patch release with this change

Sounds good to me.

@j-rivero
Copy link
Contributor

j-rivero commented Jul 6, 2023

Testing the change Build Status

Dah, need more coffee, sorry.

@scpeters scpeters merged commit 6dfa4be into gz-cmake3 Jul 6, 2023
@scpeters scpeters deleted the ci_matching_branch/cmake_extras_install branch July 6, 2023 16:59
@scpeters
Copy link
Member Author

scpeters commented Jul 6, 2023

@scpeters scpeters mentioned this pull request Jul 6, 2023
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjcarroll mjcarroll mjcarroll approved these changes

@mxgrey mxgrey Awaiting requested review from mxgrey

@j-rivero j-rivero Awaiting requested review from j-rivero j-rivero is a code owner

Assignees
No one assigned
Labels
🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@scpeters @j-rivero @mjcarroll