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

Suppress CMake 'update' step in external projects #8

Merged
merged 1 commit into from Dec 6, 2021
Merged

Suppress CMake 'update' step in external projects #8

merged 1 commit into from Dec 6, 2021

Conversation

cottsay
Copy link
Contributor

@cottsay cottsay commented May 5, 2021

When using a GIT_REPOSITORY source for a CMake external project, CMake performs an "update" step whenever the external project is evaluated. The idea is to update to the latest ref of the branch prior to invoking the configure/build/install commands, ensuring the target is fully up-to-date.

There are two reasons we want to suppress this step here:

  1. You're targeting a specific tag, which means that unless the tag changes, the code won't ever change (unlike the HEAD of a branch might).
  2. The update step invalidates the build target, so the project gets rebuilt during the package's install phase. This isn't always a problem, but the debian builds invoke the install phase with the DESTDIR variable set, which messes up the external project's installation to the staging directory. You end up with a bunch of files in the deb under /tmp/debbuild that shouldn't be there. For RHEL builds, this installing files outside of /opt/ros will fail the build.

This is the same pattern we're using in the various vendor packages under the ros2 org on GitHub, for example: ros2/mimick_vendor#17

The source at that ref will never change, so there is no need to try to
update it. This will avoid unnecessary invalidation of the build and
install targets for the external project.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@wep21 wep21 requested a review from esteve December 3, 2021 04:59
@wep21 wep21 merged commit d3850a7 into tier4:main Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants