use CMake rpm macros and unset in-source builds

[christianrauch]

The RPM CMake macros under /usr/lib/rpm/macros.d/macros.cmake are defined differently for RHEL / AlmaLinux 8 and 9. Specifically, under AlmaLinux 8 in-source builds are enabled via %__cmake_in_source_build 1 and AlmaLinux 9 turns of stripping via -DCMAKE_INSTALL_DO_STRIP:BOOL=OFF.

The effect is that the evaluation of the macros generates different spec files. This can be verified via:

# AlmaLinux 8
docker run --rm almalinux:8 bash -c "dnf -y install cmake && rpm --eval '%cmake' && rpm --eval '%cmake_build'"
# AlmaLinux 9
docker run --rm almalinux:9 bash -c "dnf -y install cmake && rpm --eval '%cmake' && rpm --eval '%cmake_build'"

On AlmaLinux 8 this evaluates to:

%if 0 
  %set_build_flags 
%else 
  CFLAGS="${CFLAGS:--O2 -g}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g}" ; export FFLAGS ; 
  FCFLAGS="${FCFLAGS:--O2 -g}" ; export FCFLAGS ; 
   
%endif 
  /usr/bin/cmake \
         \
         \
        -DCMAKE_C_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_CXX_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_Fortran_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON \
        -DCMAKE_INSTALL_PREFIX:PATH=/usr \
        -DINCLUDE_INSTALL_DIR:PATH=/usr/include \
        -DLIB_INSTALL_DIR:PATH=/usr/lib64 \
        -DSYSCONF_INSTALL_DIR:PATH=/etc \
        -DSHARE_INSTALL_PREFIX:PATH=/usr/share \
%if "lib64" == "lib64" 
        -DLIB_SUFFIX=64 \
%endif 
        -DBUILD_SHARED_LIBS:BOOL=ON

  /usr/bin/cmake --build "." -j8 --verbose

On AlmaLinux 9 this evaluates to:

%if 01 
  
  CFLAGS="${CFLAGS:--O2 -g}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g }" ; export FFLAGS ; 
  FCFLAGS="${FCFLAGS:--O2 -g }" ; export FCFLAGS ; 
  LDFLAGS="${LDFLAGS:-}" ; export LDFLAGS 
%else 
  CFLAGS="${CFLAGS:--O2 -g}" ; export CFLAGS ; 
  CXXFLAGS="${CXXFLAGS:--O2 -g}" ; export CXXFLAGS ; 
  FFLAGS="${FFLAGS:--O2 -g}" ; export FFLAGS ; 
  FCFLAGS="${FCFLAGS:--O2 -g}" ; export FCFLAGS ; 
   
%endif 
  /usr/bin/cmake \
        -S "%{_vpath_srcdir}" \
        -B "%{_vpath_builddir}" \
        -DCMAKE_C_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_CXX_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_Fortran_FLAGS_RELEASE:STRING="-DNDEBUG" \
        -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON \
        -DCMAKE_INSTALL_DO_STRIP:BOOL=OFF \
        -DCMAKE_INSTALL_PREFIX:PATH=/usr \
        -DINCLUDE_INSTALL_DIR:PATH=/usr/include \
        -DLIB_INSTALL_DIR:PATH=/usr/lib64 \
        -DSYSCONF_INSTALL_DIR:PATH=/etc \
        -DSHARE_INSTALL_PREFIX:PATH=/usr/share \
%if "lib64" == "lib64" 
        -DLIB_SUFFIX=64 \
%endif 
        -DBUILD_SHARED_LIBS:BOOL=ON

  /usr/bin/cmake --build "%{_vpath_builddir}" -j8 --verbose

Notice the difference in the build path (-B, --build) for the configure and build procedure. On AlmaLinux 8, the build path is the source path due to %__cmake_in_source_build 1, on AlmaLinux 9, there is a dedicated build folder.

This PR fixes the issue by unsetting __cmake_in_source_build and thus reverting back to out-of-source builds for both distributions. I am also replacing the manual calls of the "make" macros with their "cmake" counterpart. See https://docs.fedoraproject.org/en-US/packaging-guidelines/CMake/#_available_macros.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@09e4cee). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #742   +/-   ##
=========================================
  Coverage          ?   54.58%           
=========================================
  Files             ?       52           
  Lines             ?     6363           
  Branches          ?     1128           
=========================================
  Hits              ?     3473           
  Misses            ?     2551           
  Partials          ?      339           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christianrauch]

@nuclearsandwich @cottsay Can you have a look at this?

[cottsay]

We specifically disable the automatic out-of-source build mechanism on the official buildfarm: https://github.com/ros-infrastructure/ros_buildfarm/blob/a1ebac65f42a3019b5b248fb15b8959097520a09/ros_buildfarm/templates/release/rpm/mock_config.cfg.em#L39-L41

What is the motivation for this change?

[christianrauch]

The motivation is to align the bloom behaviour for RHEL 8 and 9, specifically when you use bloom to generate rpms (bloom-generate rosrpm) outside the build farm. Also, using the cmake macros over make macros makes this easier to maintain IMHO.

[cottsay]

Also, using the cmake macros over make macros makes this easier to maintain IMHO.

I completely agree. This code predates those macros, as well as the automatic out-of-source mechanisms.

I'm extremely hesitant to change this. It's working fine on the official buildfarm today, and I have plans to migrate away from the current spec file generators in Bloom for ROS 2 L-turtle on RHEL 10. Fixing bugs in the templates is really tough because it requires package maintainers to re-release their packages, so I'd rather not rock the boat for RHEL 8 and 9 if we can avoid it.

You can set __cmake3_in_source_build in your ~/.rpmmacros to align with the buildfarm. In the future I hope to avoid these sorts of environmental dependencies, but here we are.

[christianrauch]

You can set __cmake3_in_source_build in your ~/.rpmmacros to align with the buildfarm. In the future I hope to avoid these sorts of environmental dependencies, but here we are.

I found that I can set this on-the-fly via rpmbuild --define "%__cmake_in_source_build 1" -bb --build-in-place rpm/template.spec, and so, can make this work on RHEL 9 as expected. But it does not seem to be possible to undefine the __cmake_in_source_build macro this way.

I'm extremely hesitant to change this. It's working fine on the official buildfarm today, and I have plans to migrate away from the current spec file generators in Bloom for ROS 2 L-turtle on RHEL 10. Fixing bugs in the templates is really tough because it requires package maintainers to re-release their packages, so I'd rather not rock the boat for RHEL 8 and 9 if we can avoid it.

I can understand that you do not want to have updates to packages released with a different spec file. Since this PR contains two changes, one for changing from in-source to out-of-source builds for RHEL 8, and one for using the cmake macros instead of the make macros. Would you be willing to merge at least the "in/out source build" change in order to make bloom work again outside the build farm?

At the moment, bloom itself is broken "by default" for RHEL 9, and only works on the build farm because of workarounds, and this RHEL version still has to be supported by another 2 years. That means users won't be able to generate RPM packages for at least 2 years without looking up these workarounds. I think this is unfortunate for a tool central to ROS.

Would a test on the CI, let's say building RPMs of some core packages, convince you to release changes? I don't see tests for building RPM or DEB packages in this repo and I somehow feel that this may be the reason that many PRs stalled and are not getting reviewed. Would a DEB/RPM test change that?

[cottsay]

Would it be acceptable if we just set %global __cmake_in_source_build 1 in these spec file templates? It probably should have been done like that to begin with instead of setting it in the buildfarm configuration. We're clearly performing out-of-source builds manually anyway. That would make the buildfarm's config redundant and allow local builds to behave like the buildfarm does today (i.e. actually work).

[christianrauch]

I just added %global __cmake_in_source_build 1 to the spec templates to fix the original issue mentioned in the beginning. I can confirm that I can run something like:

bloom-generate rosrpm --ros-distro ${{ matrix.ros_distribution }}
dnf builddep -y rpm/template.spec
rpmbuild -bb --build-in-place rpm/template.spec

on AlmaLinux 8 and 9 with this PR branch. This is now only using -bb --build-in-place without manually overriding macros via --define.

[christianrauch]

@cottsay Does the new change work for you?

[cottsay]

Does the new change work for you?

Yes, thank you.