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

Update include directory install location, and other updates #2915

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 40 additions & 108 deletions source/How-To-Guides/Ament-CMake-Documentation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,16 @@ The basic outline of the ``CMakeLists.txt`` of an ament package contains:

.. code-block:: cmake

cmake_minimum_required(VERSION 3.5)
cmake_minimum_required(VERSION 3.14)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed because the minimum CMake version in REP 2000 for Humble is 3.14.4

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to add .4 at the end to prevent someone using 3.14.3 for example?

Choose a reason for hiding this comment

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

Why not 3.16 since that is what Ubuntu 20.04 LTS ships with?

Copy link
Contributor

Choose a reason for hiding this comment

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

image
Because then it would exclude mac users on Humble.

Choose a reason for hiding this comment

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

Are there macOS users who aren't using CMake as provided by brew? Does Xcode ship with CMake 3.14.4?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there features in 3.16 you

Are there macOS users who aren't using CMake as provided by brew? Does Xcode ship with CMake 3.14.4?

I'm not a MAC person, but honestly don't think anyone will allow merge that goes against REP2000. If there is a strong reason to bump to 3.16, then it could be proposed. Taking a look at the release notes for 3.15 and 3.16, I don't see any features that are dealbreakers. Do you?

Choose a reason for hiding this comment

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

Every newer CMake release is better than the last so it's always worth increasing the minimum requirement if possible. Our default position should be to use the latest version we can get away with which is almost certainly 3.16.

I'm a macOS developer (for non-ROS things) who has Xcode installed and cannot find an installation of CMake other than what brew provides which is always the latest release, currently 3.24.3. I'm guessing brew shipped 3.14.4 whenever that REP was written. I can't think of any circumstance under which a macOS developer would be locked into a specific version, especially something as old as 3.14.

Choose a reason for hiding this comment

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

Reading through REP2000, it acknowledges how there is no single stable version of CMake on macOS and how Homebrew and Pip are what we should expect users are using to get CMake. That reinforces my opinion that no one using Humble on macOS is actually limited to 3.14 so that's not a requirement we should impose on ourselves.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how to propose a change to the ROS REP, but I'm in alignment with using the newest common denominator; that is what the CMake developers recommend too.

project(my_project)

ament_package()

The argument to ``project`` will be the package name and must be identical to the package name in the ``package.xml``.

The project setup is done by ``ament_package()`` and this call must occur exactly once per package.
``ament_package()`` installs the ``package.xml``, registers the package with the ament index, and installs config (and possibly target) files for CMake so that it can be found by other packages using ``find_package``.
Since ``ament_package()`` gathers a lot of information from the ``CMakeLists.txt`` it should be the last call in your ``CMakeLists.txt``.
Although it is possible to follow calls to ``ament_package()`` by calls to ``install`` functions copying files and directories, it is simpler to just keep ``ament_package()`` the last call.
Copy link
Contributor Author

@sloretz sloretz Aug 4, 2022

Choose a reason for hiding this comment

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

Simplified - I think it's enough to tell a user to make ament_package() be the last call. There are some allowable exceptions, but there's no benefit that I know of to doing so.

``ament_package()`` installs the ``package.xml``, registers the package with the ament index, and installs CMake files so that it can be found by other packages using ``find_package``.
``ament_package()`` should be the last call in your ``CMakeLists.txt``.

``ament_package`` can be given additional arguments:

Expand All @@ -50,80 +49,41 @@ Although it is possible to follow calls to ``ament_package()`` by calls to ``ins
- ``CONFIG_EXTRAS_POST``: same as ``CONFIG_EXTRAS``, but the order in which the files are added differs.
While ``CONFIG_EXTRAS`` files are included before the files generated for the ``ament_export_*`` calls the files from ``CONFIG_EXTRAS_POST`` are included afterwards.

Instead of adding to ``ament_package``, you can also add to the variable ``${PROJECT_NAME}_CONFIG_EXTRAS`` and ``${PROJECT_NAME}_CONFIG_EXTRAS_POST`` with the same effect.
The only difference is again the order in which the files are added with the following total order:

- files added by ``CONFIG_EXTRAS``

- files added by appending to ``${PROJECT_NAME}_CONFIG_EXTRAS``

- files added by appending to ``${PROJECT_NAME}_CONFIG_EXTRAS_POST``

- files added by ``CONFIG_EXTRAS_POST``
Copy link
Contributor Author

@sloretz sloretz Aug 4, 2022

Choose a reason for hiding this comment

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

The only places I found these features used is once in rviz_rendering and once in rosidl. I don't think they're helpful to explain to a new user.


Adding files and headers
^^^^^^^^^^^^^^^^^^^^^^^^

There are two main targets to build: libraries and executables which are built by ``add_library`` and ``add_executable`` respectively.

With the separation of header files and implementation in C/C++, it is not always necessary to add both files as argument to ``add_library``/ ``add_executable``.

The following best practice is proposed:

- if you are building a library, put all headers which should be usable by clients and therefore must be installed into a subdirectory of the ``include`` folder named like the package, while all other files (``.c/.cpp`` and header files which should not be exported) are inside the ``src`` folder.
There are two main targets to build: libraries and executables.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes are subjective grammar fixes, plus links to CMake docs.

These are built by `add_library() <https://cmake.org/cmake/help/v3.14/command/add_library.html>`__ and `add_executable() <https://cmake.org/cmake/help/v3.14/command/add_executable.html>`__ respectively.

- only cpp files are explicitly referenced in the call to ``add_library`` or ``add_executable``
It's recommended to put all headers which should be usable by clients into a subdirectory of the ``include`` folder (such as ``include/my_project/foo.hpp``, while all other files (``.c/.cpp`` and header files which should not be installed) should be inside a ``src`` folder.

- allow to find headers via
Use `target_include_directories() <https://cmake.org/cmake/help/v3.14/command/target_include_directories.html>`__ to tell libraries and executables where the headers are.

.. code-block:: cmake

target_include_directories(my_target
PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>)

This adds all files in the folder ``${CMAKE_CURRENT_SOURCE_DIR}/include`` to the public interface during build time and all files in the include folder (relative to ``${CMAKE_INSTALL_DIR}``) when being installed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is pretty standard CMake stuff. I think the linked target_include_directories() explanation is enough.


In principle, using generator expressions here is not necessary if both folders are called ``include`` and top-level with respect to ``${CMAKE_CURRENT_SOURCE_DIR}`` and ``${CMAKE_INSTALL_DIR}``, but it is very common.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIUC I think we need generator expressions to make the targets relocatable, so I wouldn't even mention an alternative.

$<INSTALL_INTERFACE:include/${PROJECT_NAME}>)

Adding Dependencies
^^^^^^^^^^^^^^^^^^^

There are two ways to link your packages against a new dependency.

The first and recommended way is to use the ament macro ``ament_target_dependencies``.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With modern CMake targets ament_target_dependencies() doesn't add value over target_linked_libraries() anymore, so I removed it's recommendation.

As an example, suppose we want to link ``my_target`` against the linear algebra library Eigen3.

.. code-block:: cmake

find_package(Eigen3 REQUIRED)
ament_target_dependencies(my_target Eigen3)

It includes the necessary headers and libraries and their dependencies to be correctly found by the project.
It will also ensure that the include directories of all dependencies are ordered correctly when using overlay workspaces.

The second way is to use ``target_link_libraries``.

The recommended way in modern CMake is to only use targets, exporting and linking against them.
CMake targets are namespaced, similar to C++.
For instance, ``Eigen3`` defines the target ``Eigen3::Eigen``.

At least until ``Crystal Clemmys`` target names are not supported in the ``ament_target_dependencies`` macro.
Sometimes it will be necessary to call the ``target_link_libaries`` CMake function.
In the example of Eigen3, the call should then look like
The recommended way is to use `target_link_libraries() <https://cmake.org/cmake/help/v3.14/command/target_link_libraries.html>`__ and modern CMake targets.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you have access to cmake 3.23, then file sets are recommended by Craig Scott in his latest book. Do you think it's worth mentioning that newer feature?

Copy link
Contributor

Choose a reason for hiding this comment

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

The thing is that most of our users won't be on CMake 3.23 yet; Ubuntu Jammy ships with CMake 3.22. So I wouldn't add a mention of it yet.

Here's how to link the target ``my_target`` against the ROS C++ client library ``rclcpp``.

.. code-block:: cmake

find_package(Eigen3 REQUIRED)
target_link_libraries(my_target Eigen3::Eigen)
find_package(rclcpp REQUIRED)
target_link_libraries(my_target PUBLIC rclcpp::rclcpp)

This will also include necessary headers, libraries and their dependencies, but in contrast to ``ament_target_dependencies`` it might not correctly order the dependencies when using overlay workspaces.
That will make sure your target can include ``rclcpp``'s headers and link against its libraries.

Copy link
Contributor

Choose a reason for hiding this comment

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

While I'm totally onboard with recommending target_link_libraries, I do think we should mention ament_target_dependencies. It is still heavily used inside of the ROS 2 core, in our tutorials, and in the wider ROS ecosystem.

Copy link
Contributor

@RFRIEDM-Trimble RFRIEDM-Trimble Aug 15, 2022

Choose a reason for hiding this comment

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

One thing I would mention is to prefer the alias targets when they exist. Alias targets are not used commonly, but have clear advantages.

Copy link
Contributor

Choose a reason for hiding this comment

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

On this, one of the reasons ament_target_dependencies is convenient is that libraries have adopted a convention to match the project name with the target name, and only generate one target. If we want to support libraries not following that convention, or having multiple targets, I think it would be very helpful to add a note on how you determine what the target names exported by a library actually are. Without any notes, we may find uses resistant to switch off of ament_target_dependencies when they can't find the target name. Thoughts?

.. note::

It should never be necessary to ``find_package`` a library that is not explicitly needed but is a dependency of another dependency that is explicitly needed.
If that is the case, file a bug against the corresponding package.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Subjectively I found this explanation unclear, so I tried to reword it.

It should never be necessary to ``find_package`` a package you do not directly use.
If you find that's the case, then one of your dependencies did not properly ``find_package`` it for you.
File a bug against the offending package.

Building a Library
^^^^^^^^^^^^^^^^^^
Expand All @@ -137,7 +97,7 @@ When building a reusable library, some information needs to be exported for down

install(
DIRECTORY include/
DESTINATION include
DESTINATION include/${PROJECT_NAME}
)

install(
Expand All @@ -146,76 +106,49 @@ When building a reusable library, some information needs to be exported for down
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib
RUNTIME DESTINATION bin
INCLUDES DESTINATION include
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this because target_include_directories() already took care of associating this include with the target.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a recommendation to include GNUInstallDirs before calling install such that users will just use the defaults. There's a few options here that have some advantages.
https://youtu.be/m0DwB4OvDXk?t=1657

Copy link
Contributor

@RFRIEDM-Trimble RFRIEDM-Trimble Nov 16, 2022

Choose a reason for hiding this comment

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

With cmake 3.14 minimum support that we have bumped for humble, a simple call to install(TARGETS my_library EXPORT my_libraryTargets) seems to suffice.

)


Here, we assume that the folder ``include`` contains the headers which need to be exported.
Note that it is not necessary to put all headers into a separate folder, only those that should be included by clients.

Here is what's happening in the snippet above:

- The ``ament_export_targets`` macro exports the targets for CMake.
This is necessary to allow your library's clients to use the ``target_link_libraries(client my_library::my_library)`` syntax.
``ament_export_targets`` can take an arbitrary list of targets named as ``EXPORT`` in an install call and an additional option ``HAS_LIBRARY_TARGET``, which adds potential libraries to environment variables.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confusingly ament_export_targets takes a list of export files to declare, not targets.

Here is what the above snippet does.

- The ``ament_export_targets`` makes sure packages that depend on yours can use your library with ``target_link_libraries(downstream_target my_package::my_library)``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- The ``ament_export_targets`` makes sure packages that depend on yours can use your library with ``target_link_libraries(downstream_target my_package::my_library)``.
- The ``ament_export_targets`` makes sure packages that depend on yours can use your library with ``target_link_libraries(downstream_target my_package::my_library)`` or ``ament_target_dependencies(downstream_target my_package)``.

``ament_export_targets`` takes a name for the export file. You can use any name as long as it matches the name you give to ``EXPORT`` in the the `install() <https://cmake.org/cmake/help/v3.14/command/install.html#installing-targets>`__ command.
The option ``HAS_LIBRARY_TARGET`` makes sure environment variables like ``LD_LIBRARY_PATH`` are set when a workspace containing your package is sourced.
- The ``ament_export_dependencies`` exports dependencies to downstream packages.
This is necessary so that the user of the library does not have to call ``find_package`` for those dependencies, too.

- The first ``install`` commands installs the header files which should be available to clients.

.. warning::

Calling ``ament_export_targets``, ``ament_export_dependencies``, or other ament commands from a CMake subdirectory will not work as expected.
This is because the CMake subdirectory has no way of setting necessary variables in the parent scope where ``ament_package`` is called.
The macros ``ament_export_targets``, ``ament_export_dependencies`` must be called in the top level CMakeLists.txt, not a subdirectory.
This is necessary so that the user of the library does not have to call ``find_package`` for those dependencies, too.

- The first ``install`` commands installs the package's public header files.

- The last large install command installs the library.
Archive and library files will be exported to the lib folder, runtime binaries will be installed to the bin folder and the path to installed headers is ``include``.
Archive and library files will be exported to the lib folder, runtime binaries will be installed to the bin folder.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm. While this is strictly true based on the snippet above, it is not what we recommend for things that want to be run with ros2 run. Should we change the wording and/or the code snippet?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree making executables compatible with ros2 run is desireable.


.. note::

Windows DLLs are treated as runtime artifacts and installed into the ``RUNTIME DESTINATION`` folder.
It is therefore advised to not leave out the ``RUNTIME`` install even when developing libraries on Unix based systems.

- Regarding the ``include directory``, the install command only adds information to CMake, it does not actually install the includes folder.
This is done by copying the headers via ``install(DIRECTORY <dir> DESTINATION <dest>)`` as described above.

- The ``EXPORT`` notation of the install call requires additional attention:
It installs the CMake files for the ``my_library`` target.
It is named exactly like the argument in ``ament_export_targets`` and could be named like the library.
However, this will then prohibit using the ``ament_target_dependencies`` way of including your library.
To allow for full flexibility, it is advised to prepend the export target with something like ``<target>Targets``.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this prevents using ament_target_dependencies.

Copy link
Contributor

@RFRIEDM-Trimble RFRIEDM-Trimble Aug 15, 2022

Choose a reason for hiding this comment

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

I've ran into situations where cmake gets mad about calling ament_target_dependencies and target_link_libraries with a mixed use of specifying PUBLIC/PRIVATE/etc or not specifying it. The recommendation by CMake maintainers is to always specify PUBLIC/PRIVATE, hence users in ROS2 should be using target_link_libraries. It might be worthwhile mentioning this as a reason that pacakge maintainers should phase out usage of ament_target_dependencies.


- All install paths are relative to ``CMAKE_INSTALL_PREFIX``, which is already set correctly by colcon/ament
It is therefore advised to not leave out the ``RUNTIME DESTINATION`` option even when developing libraries on Unix based systems.

There are two additional functions which can be used but are superfluous for target based installs:
- The ``EXPORT`` notation installs an export file defined earlier by ``ament_export_targets``.

.. code-block:: cmake

ament_export_include_directories(include)
ament_export_libraries(my_library)
Comment on lines -193 to -196
Copy link
Contributor

Choose a reason for hiding this comment

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

Like my comments on ament_target_dependencies, while I'm on board with not needing this by default, a lot of the ecosystem still uses it. So I think we should document it, even if we label it as "deprecated".


The first macro marks the directory of the exported include directories (this is achieved by ``INCLUDES DESTINATION`` in the target ``install`` call).
The second macro marks the location of the installed library (this is done by the ``HAS_LIBRARY_TARGET`` argument in the call to ``ament_export_targets``).

Some of the macros can take different types of arguments for non-target exports, but since the recommended way for modern Make is to use targets, we will not cover them here.
Documentation of these options can be found in the source code itself.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the explanation of these macros because I wouldn't recommend using them in new packages.

- All install paths are relative to `CMAKE_INSTALL_PREFIX <https://cmake.org/cmake/help/v3.14/variable/CMAKE_INSTALL_PREFIX.html>`__, which you do not need to set when using `colcon <https://colcon.readthedocs.io/en/released/>`__.

Compiler and linker options
^^^^^^^^^^^^^^^^^^^^^^^^^^^

ROS 2 targets compilers which comply with the C++14 and C99 standard until at least ``Crystal Clemmys``.
Newer versions might be targeted in the future and are referenced `here <https://www.ros.org/reps/rep-2000.html>`__.
Therefore it is customary to set the corresponding CMake flags:
ROS 2 releases have minimum C and C++ standards, which cna be found in `REP-2000 <https://www.ros.org/reps/rep-2000.html>`__.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ROS 2 releases have minimum C and C++ standards, which cna be found in `REP-2000 <https://www.ros.org/reps/rep-2000.html>`__.
ROS 2 releases have minimum C and C++ standards, which can be found in `REP-2000 <https://www.ros.org/reps/rep-2000.html>`__.

It's recommended to set them using `target_compile_features() <https://cmake.org/cmake/help/v3.14/command/target_compile_features.html>`__

.. code-block:: cmake

if(NOT CMAKE_C_STANDARD)
set(CMAKE_C_STANDARD 99)
endif()
if(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 14)
endif()
# If using C
target_compile_features(my_library PUBLIC c_std_99)
# If using C++
target_compile_features(my_library PUBLIC cxx_std_17)
Comment on lines -213 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here. We don't actually do this in any packages that I know of, so while we can mention it we should also mention the old ways of doing things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a little subjective, but it is helpful to set the C++ standard this way. Just last week I encountered a package where it set it's C++ version to 14, but it failed to compile because it had a dependency on a package using a C++ 17 feature in its header. If it had used target_compile_features() then it wouldn't have encountered that problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Modern cmake guidelines also recommend target-based properties, rather than globally setting them.


To keep the code clean, compilers should throw warnings for questionable code and these warnings should be fixed.

Expand All @@ -225,23 +158,22 @@ It is recommended to at least cover the following warning levels:

- For GCC and Clang: ``-Wall -Wextra -Wpedantic`` are required and ``-Wshadow -Werror`` are advisable (the latter makes warnings errors).

Although modern CMake advises to add compiler flags on a target basis, i.e. call
Modern CMake advises to add compiler flags on a target basis with `target_compile_options() <https://cmake.org/cmake/help/v3.14/command/target_compile_options.html>`__

.. code-block:: cmake

target_compile_options(my_target PRIVATE -Wall)

it is at the moment recommended to use the directory level function ``add_compile_options(-Wall)`` to not clutter the code with target-based compile options for all executables and tests.

Building libraries on Windows
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Since Linux, Mac and Windows are all officially supported platforms, to have maximum impact any package should also build on Windows.
Windows is an officially supported platform, so we recommend making sure your packages can build on it.

The Windows library format enforces symbol visibility:
Every symbol which should be used from a client has to be explicitly exported by the library (and data symbols need to be implicitly imported).

To keep this compatible with Clang and GCC builds, it is advised to use the logic in `the GCC wiki <https://gcc.gnu.org/wiki/Visibility>`__.
To use it for a package called ``my_library``:
Use the logic in `the GCC wiki <https://gcc.gnu.org/wiki/Visibility>`__ to keep this compatible with Clang and GCC builds.
Say your package creates a library named ``my_library``:

- Copy the logic in the link into a header file called ``visibility_control.hpp``.

Comment on lines -239 to 179
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I think we should remove this entire section and just leave in the link to the Windows_Symbol_Visibility document below.

Choose a reason for hiding this comment

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

Perhaps we instead refer to the GenerateExportHeader documentation instead of using users to copy-paste some preprocessor code from the web?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we instead refer to the GenerateExportHeader documentation instead of using users to copy-paste some preprocessor code from the web?

@wjwwood and I have talked about GenerateExportHeader before. I used to be in favor of using it, but his argument was users who build ROS in other build systems will have to create that header manually. I just recently had to do this myself to build a ROS 2 package with bazel.

Choose a reason for hiding this comment

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

Does ROS2 officially support users who want to use alternative build systems? If not then we shouldn't make our projects more cumbersome to maintain because of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does ROS2 officially support users who want to use alternative build systems? If not then we shouldn't make our projects more cumbersome to maintain because of them.

AFAIK it's not officially supported. That's a good thought about the maintenance burden, but our experience has been they're rarely touched: see rmw's whose last change was 5 years ago.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I'm with @wjwwood; I think the status quo is fine. Once written, these files never change, so the maintenance burden is zero. And when people do want to use build systems other than the official ones, they don't need to do anything special.

Copy link
Member

Choose a reason for hiding this comment

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

At the end of the day both approaches are ok, so it's up to the maintainers. As a maintainer, I personally prefer the static file, but either way can work.

If we're talking about documentation and what we recommend, I think we can just refer to both as options. 🤷

Expand Down