feat(colcon_plugin): use release build by default (#4502)

canonical · Jan 8, 2024 · b6c5a60 · b6c5a60
1 parent 0b6fecb
commit b6c5a60
Show file tree

Hide file tree

Showing 4 changed files with 292 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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",