Remove export of vendored project. #10
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
This was referenced Oct 7, 2022
Git is being used to fetch the vendored project so it should be available at build time. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
ament's dependency export system relies on adding corresponding exported dependencies in the `package.xml` file. It actually would be possible to use this export if an additional `buildtool_export_depend` on `ament_cmake` was used so that all downstream packages required `ament_cmake` to build using this package. However, that's a bit heavy handed for a vendor package in particular and it is a recommended practice that vendor packages be as transparent as possible so that they can be removed once deprecated. This change will require updates to packages consuming this vendor package. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
nuclearsandwich
force-pushed
the
nuclearsandwich/dependencies
branch
from
59d97a8
to
0e6ef2e
Compare
orensbruli
approved these changes
Oct 9, 2022
orensbruli
pushed a commit
that referenced
this pull request
Nov 2, 2022
* Add buildtool dependency on git. Git is being used to fetch the vendored project so it should be available at build time. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Do not export dependencies on the vendored project. ament's dependency export system relies on adding corresponding exported dependencies in the `package.xml` file. It actually would be possible to use this export if an additional `buildtool_export_depend` on `ament_cmake` was used so that all downstream packages required `ament_cmake` to build using this package. However, that's a bit heavy handed for a vendor package in particular and it is a recommended practice that vendor packages be as transparent as possible so that they can be removed once deprecated. This change will require updates to packages consuming this vendor package. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
orensbruli
pushed a commit
that referenced
this pull request
Nov 11, 2022
* Add buildtool dependency on git. Git is being used to fetch the vendored project so it should be available at build time. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Do not export dependencies on the vendored project. ament's dependency export system relies on adding corresponding exported dependencies in the `package.xml` file. It actually would be possible to use this export if an additional `buildtool_export_depend` on `ament_cmake` was used so that all downstream packages required `ament_cmake` to build using this package. However, that's a bit heavy handed for a vendor package in particular and it is a recommended practice that vendor packages be as transparent as possible so that they can be removed once deprecated. This change will require updates to packages consuming this vendor package. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
orensbruli
added a commit
that referenced
this pull request
Nov 11, 2022
* 0.1.1 Signed-off-by: Esteban Martinena <orensbruli@gmail.com> * Added ament_cmake_libraries dependence Signed-off-by: Esteban Martinena <orensbruli@gmail.com> * 0.1.2 Signed-off-by: Esteban Martinena <orensbruli@gmail.com> * Revert "Change install behavior (#7)" This reverts commit b3bbc75. Signed-off-by: Michael X. Grey <grey@openrobotics.org> * testing cmake vendoring Signed-off-by: Esteban Martinena <orensbruli@gmail.com> * Updated changelog Signed-off-by: Esteban Martinena <orensbruli@gmail.com> * 0.1.3 Signed-off-by: Esteban Martinena <orensbruli@gmail.com> * Update vendor package build. (#9) * Remove extra arguments that aren't used. According to the ExternalProject docs (1) `PREFIX` is used as the prefix for the project directory generator expressions but is not otherwise passed to the external project's build directly. Similarly the docs for the `INSTALL_DIR` option state: > Installation prefix to be placed in the <INSTALL_DIR> placeholder. > This does not actually configure the external project to install to the given prefix. > That must be done by passing appropriate arguments to the external project configuration step, e.g. using <INSTALL_DIR>. So both of these arguments are unused and sources for potential confusion. (1) https://cmake.org/cmake/help/latest/module/ExternalProject.html * Pin specific commit of vendored project. Even when targeting an upstream project's branch rather than a specific release vendor packages should pin to a specific commit so that changes to the upstream package are only introduced intentionally and not due to an unrelated rebuild on the build farm. * Remove unneeded BUILD_ALWAYS argument. This argument does not do anything for this external project. Build farm builds will always build this package when required since they run in a clean workspace and since this external project is configured via git CMake has the ability to detect changes and know that a rebuild is required when working locally. From the documentation for this option: > Enabling this option forces the build step to always be run. > This can be the easiest way to robustly ensure that the external project's own build dependencies are evaluated rather than relying on the default success timestamp-based method. > This option is not normally needed unless developers are expected to modify something the external project's build depends on in a way that is not detectable via the step target dependencies (e.g. SOURCE_DIR is used without a download method and developers might modify the sources in SOURCE_DIR). * Fix formatting and update build options against changes in main. This package does not use the standard `BUILD_TESTING` CMake argument but it has namespaced arguments for building tests and examples. While updating these I also fixed some inconsistent whitespace and use the CMake values ON/OFF for all boolean values for consistency. * Remove CMake patch which is not required for relocatability. This patch is adapted from an earlier version of CMakeLists.txt and does not appear to be required to build and find the vendored package in a local colcon workspace. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * 0.2.0 Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Remove export of vendored project. (#10) * Add buildtool dependency on git. Git is being used to fetch the vendored project so it should be available at build time. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Do not export dependencies on the vendored project. ament's dependency export system relies on adding corresponding exported dependencies in the `package.xml` file. It actually would be possible to use this export if an additional `buildtool_export_depend` on `ament_cmake` was used so that all downstream packages required `ament_cmake` to build using this package. However, that's a bit heavy handed for a vendor package in particular and it is a recommended practice that vendor packages be as transparent as possible so that they can be removed once deprecated. This change will require updates to packages consuming this vendor package. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * 0.2.1 Signed-off-by: Esteban Martinena <orensbruli@gmail.com> Signed-off-by: Michael X. Grey <grey@openrobotics.org> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
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.
ament's dependency export system relies on adding corresponding exported dependencies in the
package.xmlfile.It actually would be possible to use this export if an additional
buildtool_export_dependonament_cmakewas used so that all downstream packages requiredament_cmaketo build using this package.However, that's a bit heavy handed for a vendor package in particular and it is a recommended practice that vendor packages be as transparent as possible so that they can be removed once deprecated.
This change will require updates to packages consuming this vendor package.
An additional change updates the package.xml to reflect that
gitis a required build tool when fetching git repositories usingexternalproject_add.