From 7a266a54f54a56e3737989c9b680c88a1fe74c51 Mon Sep 17 00:00:00 2001 From: Mirko Ferrati <mirko.ferrati@canonical.com> Date: Wed, 3 Jan 2024 11:15:14 +0100 Subject: [PATCH] Colcon plugin: use release build by default --- snapcraft/parts/plugins/colcon_plugin.py | 11 +- snapcraft_legacy/plugins/v2/colcon.py | 7 +- tests/legacy/unit/plugins/v2/test_colcon.py | 135 +++++++++++++++++- tests/unit/parts/plugins/test_colcon.py | 143 +++++++++++++++++++- 4 files changed, 292 insertions(+), 4 deletions(-) diff --git a/snapcraft/parts/plugins/colcon_plugin.py b/snapcraft/parts/plugins/colcon_plugin.py index 4b8a740ca7..553f6bd61a 100644 --- a/snapcraft/parts/plugins/colcon_plugin.py +++ b/snapcraft/parts/plugins/colcon_plugin.py @@ -197,7 +197,16 @@ def _get_build_commands(self) -> List[str]: if options.colcon_packages: build_command.extend(["--packages-select", *options.colcon_packages]) - if options.colcon_cmake_args: + # compile in release only if user did not set the build type in cmake-args + if not any("-DCMAKE_BUILD_TYPE=" in s for s in options.colcon_cmake_args): + build_command.extend( + [ + "--cmake-args", + "-DCMAKE_BUILD_TYPE=Release", + *options.colcon_cmake_args, + ] + ) + elif len(options.colcon_cmake_args) > 0: build_command.extend(["--cmake-args", *options.colcon_cmake_args]) if options.colcon_ament_cmake_args: diff --git a/snapcraft_legacy/plugins/v2/colcon.py b/snapcraft_legacy/plugins/v2/colcon.py index 4ed2e93752..b4e9d57882 100644 --- a/snapcraft_legacy/plugins/v2/colcon.py +++ b/snapcraft_legacy/plugins/v2/colcon.py @@ -199,7 +199,12 @@ def _get_build_commands(self) -> List[str]: if self.options.colcon_packages: build_command.extend(["--packages-select", *self.options.colcon_packages]) - if self.options.colcon_cmake_args: + # compile in release only if user did not set the build type in cmake-args + if not any("-DCMAKE_BUILD_TYPE=" in s for s in self.options.colcon_cmake_args): + build_command.extend(["--cmake-args", "-DCMAKE_BUILD_TYPE=Release", + *self.options.colcon_cmake_args + ]) + elif len(self.options.colcon_cmake_args)>0: build_command.extend(["--cmake-args", *self.options.colcon_cmake_args]) if self.options.colcon_ament_cmake_args: diff --git a/tests/legacy/unit/plugins/v2/test_colcon.py b/tests/legacy/unit/plugins/v2/test_colcon.py index 1267afbbb5..b1f164cf46 100644 --- a/tests/legacy/unit/plugins/v2/test_colcon.py +++ b/tests/legacy/unit/plugins/v2/test_colcon.py @@ -182,6 +182,7 @@ class Options: "colcon build " '--base-paths "${SNAPCRAFT_PART_SRC_WORK}" --build-base "${SNAPCRAFT_PART_BUILD}" ' '--merge-install --install-base "${SNAPCRAFT_PART_INSTALL}"/opt/ros/snap ' + "--cmake-args -DCMAKE_BUILD_TYPE=Release " '--parallel-workers "${SNAPCRAFT_PARALLEL_BUILD_COUNT}"', "## Post build command", 'if [ -f "${SNAPCRAFT_PART_INSTALL}"/opt/ros/snap/COLCON_IGNORE ]; then', @@ -310,7 +311,139 @@ class Options: '--base-paths "${SNAPCRAFT_PART_SRC_WORK}" --build-base "${SNAPCRAFT_PART_BUILD}" ' '--merge-install --install-base "${SNAPCRAFT_PART_INSTALL}"/opt/ros/snap ' "--packages-ignore ipackage1 ipackage2... --packages-select package1 " - "package2... --cmake-args cmake args... " + "package2... --cmake-args -DCMAKE_BUILD_TYPE=Release cmake args... " + "--ament-cmake-args ament args... --catkin-cmake-args catkin " + 'args... --parallel-workers "${SNAPCRAFT_PARALLEL_BUILD_COUNT}"', + "## Post build command", + 'if [ -f "${SNAPCRAFT_PART_INSTALL}"/opt/ros/snap/COLCON_IGNORE ]; then', + 'rm "${SNAPCRAFT_PART_INSTALL}"/opt/ros/snap/COLCON_IGNORE', + "fi", + "env -i LANG=C.UTF-8 LC_ALL=C.UTF-8 PATH=/bin:/test SNAP=TESTSNAP " + "SNAP_ARCH=TESTARCH SNAP_NAME=TESTSNAPNAME SNAP_VERSION=TESTV1 " + "http_proxy=http://foo https_proxy=https://bar " + "/test/python3 -I /test/_ros.py " + 'stage-runtime-dependencies --part-src "${SNAPCRAFT_PART_SRC_WORK}" --part-install "${SNAPCRAFT_PART_INSTALL}" ' + '--ros-version "${ROS_VERSION}" --ros-distro "${ROS_DISTRO}" --target-arch "${SNAPCRAFT_TARGET_ARCH}"', + ] + + +def test_get_build_commands_with_cmake_debug(monkeypatch): + class Options: + colcon_ament_cmake_args = ["ament", "args..."] + colcon_catkin_cmake_args = ["catkin", "args..."] + colcon_cmake_args = ["-DCMAKE_BUILD_TYPE=Debug", "args..."] + colcon_packages = ["package1", "package2..."] + colcon_packages_ignore = ["ipackage1", "ipackage2..."] + ros_build_snaps = ["foo"] + + plugin = colcon.ColconPlugin(part_name="my-part", options=Options()) + + monkeypatch.setattr(sys, "path", ["", "/test"]) + monkeypatch.setattr(sys, "executable", "/test/python3") + monkeypatch.setattr(_ros, "__file__", "/test/_ros.py") + monkeypatch.setattr( + os, + "environ", + dict( + FOO="baR", + PATH="/bin:/test", + SNAP="TESTSNAP", + SNAP_ARCH="TESTARCH", + SNAP_NAME="TESTSNAPNAME", + SNAP_VERSION="TESTV1", + http_proxy="http://foo", + https_proxy="https://bar", + ), + ) + + assert plugin.get_build_commands() == [ + "if [ ! -f /etc/ros/rosdep/sources.list.d/20-default.list ]; then", + "sudo --preserve-env=http_proxy,https_proxy rosdep init; fi", + 'rosdep update --include-eol-distros --rosdistro "${ROS_DISTRO}"', + 'state="$(set +o); set -$-"', + "set +u", + "", + "## Sourcing ROS ws in build snaps", + 'if [ -f "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/snap/foo/current/opt/ros/${ROS_DISTRO}" . "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/snap/foo/current/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/snap/foo/current/opt/ros/snap" . "/snap/foo/current/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in stage snaps", + 'if [ -f "${SNAPCRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="${SNAPCRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}" . "${SNAPCRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "${SNAPCRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="${SNAPCRAFT_PART_INSTALL}/opt/ros/snap" . "${SNAPCRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in system", + 'if [ -f "/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/opt/ros/${ROS_DISTRO}" . "/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/opt/ros/snap" . "/opt/ros/snap/local_setup.sh"', + "fi", + "", + 'eval "${state}"', + 'rm -f "${SNAPCRAFT_PART_INSTALL}/.installed_packages.txt"', + 'rm -f "${SNAPCRAFT_PART_INSTALL}/.build_snaps.txt"', + "if [ -d /snap/foo/current/opt/ros ]; then", + "ROS_PACKAGE_PATH=/snap/foo/current/opt/ros rospack list-names | (xargs " + 'rosdep resolve --rosdistro "${ROS_DISTRO}" || echo "") | awk ' + '"/#apt/{getline;print;}" >> ' + '"${SNAPCRAFT_PART_INSTALL}/.installed_packages.txt"', + "fi", + 'if [ -d "/snap/foo/current/opt/ros/${ROS_DISTRO}/" ]; then', + 'rosdep keys --rosdistro "${ROS_DISTRO}" --from-paths ' + '"/snap/foo/current/opt/ros/${ROS_DISTRO}" --ignore-packages-from-source | ' + '(xargs rosdep resolve --rosdistro "${ROS_DISTRO}" || echo "") | grep -v "#" ' + '>> "${SNAPCRAFT_PART_INSTALL}"/.installed_packages.txt', + "fi", + 'if [ -d "/snap/foo/current/opt/ros/snap/" ]; then', + 'rosdep keys --rosdistro "${ROS_DISTRO}" --from-paths ' + '"/snap/foo/current/opt/ros/snap" --ignore-packages-from-source | (xargs ' + 'rosdep resolve --rosdistro "${ROS_DISTRO}" || echo "") | grep -v "#" >> ' + '"${SNAPCRAFT_PART_INSTALL}"/.installed_packages.txt', + "fi", + "", + 'rosdep install --default-yes --ignore-packages-from-source --from-paths "${SNAPCRAFT_PART_SRC_WORK}"', + 'state="$(set +o); set -$-"', + "set +u", + "", + "## Sourcing ROS ws in build snaps", + 'if [ -f "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/snap/foo/current/opt/ros/${ROS_DISTRO}" . "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/snap/foo/current/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/snap/foo/current/opt/ros/snap" . "/snap/foo/current/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in stage snaps", + 'if [ -f "${SNAPCRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="${SNAPCRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}" . "${SNAPCRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "${SNAPCRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="${SNAPCRAFT_PART_INSTALL}/opt/ros/snap" . "${SNAPCRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in system", + 'if [ -f "/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/opt/ros/${ROS_DISTRO}" . "/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/opt/ros/snap" . "/opt/ros/snap/local_setup.sh"', + "fi", + "", + 'eval "${state}"', + "## Build command", + "colcon build " + '--base-paths "${SNAPCRAFT_PART_SRC_WORK}" --build-base "${SNAPCRAFT_PART_BUILD}" ' + '--merge-install --install-base "${SNAPCRAFT_PART_INSTALL}"/opt/ros/snap ' + "--packages-ignore ipackage1 ipackage2... --packages-select package1 " + "package2... --cmake-args -DCMAKE_BUILD_TYPE=Debug args... " "--ament-cmake-args ament args... --catkin-cmake-args catkin " 'args... --parallel-workers "${SNAPCRAFT_PARALLEL_BUILD_COUNT}"', "## Post build command", diff --git a/tests/unit/parts/plugins/test_colcon.py b/tests/unit/parts/plugins/test_colcon.py index 2e27a13c4c..a117b12ae2 100644 --- a/tests/unit/parts/plugins/test_colcon.py +++ b/tests/unit/parts/plugins/test_colcon.py @@ -216,6 +216,7 @@ def test_get_build_commands(self, setup_method_fixture, new_dir, monkeypatch): "colcon build " '--base-paths "${CRAFT_PART_SRC_WORK}" --build-base "${CRAFT_PART_BUILD}" ' '--merge-install --install-base "${CRAFT_PART_INSTALL}/opt/ros/snap" ' + "--cmake-args -DCMAKE_BUILD_TYPE=Release " '--parallel-workers "${CRAFT_PARALLEL_BUILD_COUNT}"', "## Post build command", 'if [ -f "${CRAFT_PART_INSTALL}"/opt/ros/snap/COLCON_IGNORE ]; then', @@ -352,7 +353,147 @@ def test_get_build_commands_with_all_properties( '--base-paths "${CRAFT_PART_SRC_WORK}" --build-base "${CRAFT_PART_BUILD}" ' '--merge-install --install-base "${CRAFT_PART_INSTALL}/opt/ros/snap" ' "--packages-ignore ipackage1 ipackage2... --packages-select package1 " - "package2... --cmake-args cmake args... " + "package2... --cmake-args -DCMAKE_BUILD_TYPE=Release cmake args... " + "--ament-cmake-args ament args... --catkin-cmake-args catkin " + 'args... --parallel-workers "${CRAFT_PARALLEL_BUILD_COUNT}"', + "## Post build command", + 'if [ -f "${CRAFT_PART_INSTALL}"/opt/ros/snap/COLCON_IGNORE ]; then', + 'rm "${CRAFT_PART_INSTALL}"/opt/ros/snap/COLCON_IGNORE', + "fi", + "env -i LANG=C.UTF-8 LC_ALL=C.UTF-8 PATH=/bin:/test SNAP=TESTSNAP " + "SNAP_ARCH=TESTARCH SNAP_NAME=TESTSNAPNAME SNAP_VERSION=TESTV1 " + "http_proxy=http://foo https_proxy=https://bar " + "/test/python3 -I /test/_ros.py " + 'stage-runtime-dependencies --part-src "${CRAFT_PART_SRC_WORK}" ' + '--part-install "${CRAFT_PART_INSTALL}" ' + '--ros-version "${ROS_VERSION}" --ros-distro "${ROS_DISTRO}" ' + '--target-arch "${CRAFT_TARGET_ARCH}" ' + f"--stage-cache-dir {new_dir} --base core22", + ] + + def test_get_build_commands_with_cmake_debug( + self, setup_method_fixture, new_dir, monkeypatch + ): + # pylint: disable=line-too-long + plugin = setup_method_fixture( + new_dir, + properties={ + "source": ".", + "colcon-ament-cmake-args": ["ament", "args..."], + "colcon-catkin-cmake-args": ["catkin", "args..."], + "colcon-cmake-args": ["-DCMAKE_BUILD_TYPE=Debug", "args..."], + "colcon-packages": ["package1", "package2..."], + "colcon-packages-ignore": ["ipackage1", "ipackage2..."], + "colcon-ros-build-snaps": ["foo"], + }, + ) + + monkeypatch.setattr(sys, "path", ["", "/test"]) + monkeypatch.setattr(sys, "executable", "/test/python3") + monkeypatch.setattr(_ros, "__file__", "/test/_ros.py") + monkeypatch.setattr( + os, + "environ", + { + "FOO": "baR", + "PATH": "/bin:/test", + "SNAP": "TESTSNAP", + "SNAP_ARCH": "TESTARCH", + "SNAP_NAME": "TESTSNAPNAME", + "SNAP_VERSION": "TESTV1", + "http_proxy": "http://foo", + "https_proxy": "https://bar", + }, + ) + + assert plugin.get_build_commands() == [ + "if [ ! -f /etc/ros/rosdep/sources.list.d/20-default.list ]; then", + "sudo --preserve-env=http_proxy,https_proxy rosdep init; fi", + 'rosdep update --include-eol-distros --rosdistro "${ROS_DISTRO}"', + 'state="$(set +o); set -$-"', + "set +u", + "", + "## Sourcing ROS ws in build snaps", + 'if [ -f "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/snap/foo/current/opt/ros/${ROS_DISTRO}" . "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/snap/foo/current/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/snap/foo/current/opt/ros/snap" . "/snap/foo/current/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in stage snaps", + 'if [ -f "${CRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="${CRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}" . "${CRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "${CRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="${CRAFT_PART_INSTALL}/opt/ros/snap" . "${CRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in system", + 'if [ -f "/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/opt/ros/${ROS_DISTRO}" . "/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/opt/ros/snap" . "/opt/ros/snap/local_setup.sh"', + "fi", + "", + 'eval "${state}"', + 'rm -f "${CRAFT_PART_INSTALL}/.installed_packages.txt"', + 'rm -f "${CRAFT_PART_INSTALL}/.build_snaps.txt"', + "if [ -d /snap/foo/current/opt/ros ]; then", + "ROS_PACKAGE_PATH=/snap/foo/current/opt/ros rospack list-names | (xargs " + 'rosdep resolve --rosdistro "${ROS_DISTRO}" || echo "") | awk ' + '"/#apt/{getline;print;}" >> ' + '"${CRAFT_PART_INSTALL}/.installed_packages.txt"', + "fi", + 'if [ -d "/snap/foo/current/opt/ros/${ROS_DISTRO}/" ]; then', + 'rosdep keys --rosdistro "${ROS_DISTRO}" --from-paths ' + '"/snap/foo/current/opt/ros/${ROS_DISTRO}" --ignore-packages-from-source | ' + '(xargs rosdep resolve --rosdistro "${ROS_DISTRO}" || echo "") | grep -v "#" ' + '>> "${CRAFT_PART_INSTALL}"/.installed_packages.txt', + "fi", + 'if [ -d "/snap/foo/current/opt/ros/snap/" ]; then', + 'rosdep keys --rosdistro "${ROS_DISTRO}" --from-paths ' + '"/snap/foo/current/opt/ros/snap" --ignore-packages-from-source | (xargs ' + 'rosdep resolve --rosdistro "${ROS_DISTRO}" || echo "") | grep -v "#" >> ' + '"${CRAFT_PART_INSTALL}"/.installed_packages.txt', + "fi", + "", + 'rosdep install --default-yes --ignore-packages-from-source --from-paths "${CRAFT_PART_SRC_WORK}"', + 'state="$(set +o); set -$-"', + "set +u", + "", + "## Sourcing ROS ws in build snaps", + 'if [ -f "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/snap/foo/current/opt/ros/${ROS_DISTRO}" . "/snap/foo/current/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/snap/foo/current/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/snap/foo/current/opt/ros/snap" . "/snap/foo/current/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in stage snaps", + 'if [ -f "${CRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="${CRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}" . "${CRAFT_PART_INSTALL}/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "${CRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="${CRAFT_PART_INSTALL}/opt/ros/snap" . "${CRAFT_PART_INSTALL}/opt/ros/snap/local_setup.sh"', + "fi", + "", + "## Sourcing ROS ws in system", + 'if [ -f "/opt/ros/${ROS_DISTRO}/local_setup.sh" ]; then', + 'AMENT_CURRENT_PREFIX="/opt/ros/${ROS_DISTRO}" . "/opt/ros/${ROS_DISTRO}/local_setup.sh"', + "fi", + 'if [ -f "/opt/ros/snap/local_setup.sh" ]; then', + 'COLCON_CURRENT_PREFIX="/opt/ros/snap" . "/opt/ros/snap/local_setup.sh"', + "fi", + "", + 'eval "${state}"', + "## Build command", + "colcon build " + '--base-paths "${CRAFT_PART_SRC_WORK}" --build-base "${CRAFT_PART_BUILD}" ' + '--merge-install --install-base "${CRAFT_PART_INSTALL}/opt/ros/snap" ' + "--packages-ignore ipackage1 ipackage2... --packages-select package1 " + "package2... --cmake-args -DCMAKE_BUILD_TYPE=Debug args... " "--ament-cmake-args ament args... --catkin-cmake-args catkin " 'args... --parallel-workers "${CRAFT_PARALLEL_BUILD_COUNT}"', "## Post build command",