colcon plugin: support build-time chaining

snapcraft/plugins/colcon.py

@@ -62,7 +62,7 @@
 import snapcraft
 from snapcraft.plugins import _ros
 from snapcraft.plugins import _python
-from snapcraft import common, file_utils, formatting_utils, repo
+from snapcraft import file_utils, repo
 from snapcraft.internal import errors, mangling
 
 logger = logging.getLogger(__name__)
@@ -294,17 +294,13 @@ def env(self, root):
         """Runtime environment for ROS binaries and services."""
 
         env = [
-            # The Snapcraft Core will ensure that we get a good LD_LIBRARY_PATH
-            # overall, but it defines it after this function runs. Some ROS
-            # tools will cause binaries to be run when we source the setup.sh,
-            # below, so we need to have a sensible LD_LIBRARY_PATH before then.
-            'LD_LIBRARY_PATH="$LD_LIBRARY_PATH:{}"'.format(
-                formatting_utils.combine_paths(
-                    common.get_library_paths(root, self.project.arch_triplet),
-                    prepend="",
-                    separator=":",
-                )
-            )
+            'AMENT_PYTHON_EXECUTABLE="{}"'.format(
+                os.path.join(root, "usr", "bin", "python3")
+            ),
+            'COLCON_PYTHON_EXECUTABLE="{}"'.format(
+                os.path.join(root, "usr", "bin", "python3")
+            ),
+            'SNAP_COLCON_ROOT="{}"'.format(root),
         ]
 
         # Each of these lines is prepended with an `export` when the environment is
@@ -408,20 +404,17 @@ def _source_setup_sh(self, root):
             """
             # First, source the upstream ROS underlay
             if [ -f "{underlay_setup}" ]; then
-                AMENT_CURRENT_PREFIX="{underlay}" . "{underlay_setup}"
+                . "{underlay_setup}"
             fi
 
             # Then source the overlay
             if [ -f "{overlay_setup}" ]; then
-                COLCON_CURRENT_PREFIX="{overlay}" COLCON_PYTHON_EXECUTABLE="{python_path}" . "{overlay_setup}"
+                . "{overlay_setup}"
             fi
         """
         ).format(
-            underlay=underlaydir,
             underlay_setup=os.path.join(underlaydir, "setup.sh"),
-            overlay=overlaydir,
             overlay_setup=os.path.join(overlaydir, "setup.sh"),
-            python_path=os.path.join(root, "usr", "bin", "python3"),
         )
 
         # We need to source ROS's setup.sh at this point. However, it accepts
@@ -476,7 +469,7 @@ def _prepare_build(self):
         mangling.rewrite_python_shebangs(self.installdir)
 
         # Rewrite the prefixes to point to the in-part rosdir instead of the system
-        self._fix_prefixes(self.installdir)
+        self._fix_prefixes()
 
         # Each Colcon package distributes .cmake files so they can be found via
         # find_package(). However, the Ubuntu packages pulled down as
@@ -525,7 +518,7 @@ def _new_path(path):
         self._rewrite_cmake_paths(_new_path)
 
         # Rewrite prefixes for both the underlay and overlay.
-        self._fix_prefixes("$SNAP")
+        self._fix_prefixes()
 
         # If pip dependencies were installed, generate a sitecustomize that
         # allows access to them.
@@ -534,16 +527,17 @@ def _new_path(path):
                 "3", stage_dir=self.project.stage_dir, install_dir=self.installdir
             )
 
-    def _fix_prefixes(self, new_prefix):
+    def _fix_prefixes(self):
         installdir_pattern = re.compile(r"^{}".format(self.installdir))
+        new_prefix = "$SNAP_COLCON_ROOT"
 
         def _rewrite_prefix(match):
             # Group 1 is the variable definition, group 2 is the path, which we may need
             # to modify.
             path = match.group(3).strip(" \n\t'\"")
 
-            # Bail early if this isn't even a path
-            if os.path.sep not in path:
+            # Bail early if this isn't even a path, or if it's already been rewritten
+            if os.path.sep not in path or new_prefix in path:
                 return match.group()
 
             # If the path doesn't start with the installdir, then it needs to point to
@@ -556,21 +550,49 @@ def _rewrite_prefix(match):
                 '\\1\\2"{}"\\4'.format(installdir_pattern.sub(new_prefix, path))
             )
 
-        # Set the AMENT_CURRENT_PREFIX throughout to the in-snap, run-time prefix
-        # instead of the build-time prefix.
+        # Set the AMENT_CURRENT_PREFIX throughout to the in-snap prefix
         snapcraft.file_utils.replace_in_file(
             self.installdir,
             re.compile(r""),
             re.compile(r"(\${)(AMENT_CURRENT_PREFIX:=)(.*)(})"),
             _rewrite_prefix,
         )
 
-        # Set any *_COLCON_CURRENT_PREFIX throughout to the in-snap, run-time prefix
-        # instead of the build-time prefix.
+        # Set the COLCON_CURRENT_PREFIX (if it's in the installdir) to the in-snap
+        # prefix
+        snapcraft.file_utils.replace_in_file(
+            self.installdir,
+            re.compile(r""),
+            re.compile(
+                r"()(COLCON_CURRENT_PREFIX=)(['\"].*{}.*)()".format(self.installdir)
+            ),
+            _rewrite_prefix,
+        )
+
+        # Set the _colcon_prefix_sh_COLCON_CURRENT_PREFIX throughout to the in-snap
+        # prefix
+        snapcraft.file_utils.replace_in_file(
+            self.installdir,
+            re.compile(r""),
+            re.compile(r"()(_colcon_prefix_sh_COLCON_CURRENT_PREFIX=)(.*)()"),
+            _rewrite_prefix,
+        )
+
+        # Set the _colcon_package_sh_COLCON_CURRENT_PREFIX throughout to the in-snap
+        # prefix
+        snapcraft.file_utils.replace_in_file(
+            self.installdir,
+            re.compile(r""),
+            re.compile(r"()(_colcon_package_sh_COLCON_CURRENT_PREFIX=)(.*)()"),
+            _rewrite_prefix,
+        )
+
+        # Set the _colcon_prefix_chain_sh_COLCON_CURRENT_PREFIX throughout to the in-snap
+        # prefix
         snapcraft.file_utils.replace_in_file(
             self.installdir,
             re.compile(r""),
-            re.compile(r"()(_COLCON_CURRENT_PREFIX=)(.*)()"),
+            re.compile(r"()(_colcon_prefix_chain_sh_COLCON_CURRENT_PREFIX=)(.*)()"),
             _rewrite_prefix,
         )
 

tests/spread/plugins/colcon/run/task.yaml

@@ -1,7 +1,6 @@
 summary: Build and run a basic colcon snap
 warn-timeout: 9m  # Keep less than 10 minutes so Travis can't timeout
 priority: 100  # Run this test early so we're not waiting for it
-manual: true # LP: #1816565
 
 environment:
   SNAP_DIR: ../snaps/colcon-talker-listener

tests/spread/plugins/colcon/snaps/colcon-talker-listener/snap/snapcraft.yaml

@@ -6,7 +6,6 @@ description: |
 
 grade: stable
 confinement: strict
-base: core18
 
 apps:
   colcon-talker-listener:

tests/unit/plugins/test_colcon.py

@@ -474,18 +474,28 @@ def test_run_environment(self, run_mock):
         underlay_setup = os.path.join(plugin.options.colcon_rosdistro, "setup.sh")
         overlay_setup = os.path.join("snap", "setup.sh")
 
-        # Verify that LD_LIBRARY_PATH was set before any setup.sh is sourced. Also
-        # verify that the underlay setup is sourced before the overlay.
-        ld_library_path_index = [
-            i for i, line in enumerate(environment) if "LD_LIBRARY_PATH" in line
+        # Verify that the python executables and root are set before any setup.sh is
+        # sourced. Also verify that the underlay setup is sourced before the overlay.
+        ament_python_index = [
+            i for i, line in enumerate(environment) if "AMENT_PYTHON_EXECUTABLE" in line
+        ][0]
+        colcon_python_index = [
+            i
+            for i, line in enumerate(environment)
+            if "COLCON_PYTHON_EXECUTABLE" in line
+        ][0]
+        root_index = [
+            i for i, line in enumerate(environment) if "SNAP_COLCON_ROOT" in line
         ][0]
         underlay_source_setup_index = [
             i for i, line in enumerate(environment) if underlay_setup in line
         ][0]
         overlay_source_setup_index = [
             i for i, line in enumerate(environment) if overlay_setup in line
         ][0]
-        self.assertThat(ld_library_path_index, LessThan(underlay_source_setup_index))
+        self.assertThat(ament_python_index, LessThan(underlay_source_setup_index))
+        self.assertThat(colcon_python_index, LessThan(underlay_source_setup_index))
+        self.assertThat(root_index, LessThan(underlay_source_setup_index))
         self.assertThat(
             underlay_source_setup_index, LessThan(overlay_source_setup_index)
         )
@@ -499,18 +509,15 @@ def test_source_setup_sh(self):
         underlay_setup = os.path.join(underlay, "setup.sh")
         overlay = os.path.join("test-root", "opt", "ros", "snap")
         overlay_setup = os.path.join(overlay, "setup.sh")
-        python_path = os.path.join("test-root", "usr", "bin", "python3")
 
         # Make sure $@ is zeroed, then setup.sh sourced, then $@ is restored
         lines_of_interest = [
             "set --",
             'if [ -f "{}" ]; then'.format(underlay_setup),
-            'AMENT_CURRENT_PREFIX="{}" . "{}"'.format(underlay, underlay_setup),
+            '. "{}"'.format(underlay_setup),
             "fi",
             'if [ -f "{}" ]; then'.format(overlay_setup),
-            'COLCON_CURRENT_PREFIX="{}" COLCON_PYTHON_EXECUTABLE="{}" . "{}"'.format(
-                overlay, python_path, overlay_setup
-            ),
+            '. "{}"'.format(overlay_setup),
             "fi",
             'eval "set -- $BACKUP_ARGS"',
         ]
@@ -593,16 +600,12 @@ def test_ament_prefix_is_rewritten(self):
             {
                 "path": "setup.sh",
                 "contents": ": ${AMENT_CURRENT_PREFIX:=/opt/ros/crystal}",
-                "expected": ': ${{AMENT_CURRENT_PREFIX:="{}/opt/ros/crystal"}}'.format(
-                    self.plugin.installdir
-                ),
+                "expected": ': ${AMENT_CURRENT_PREFIX:="$SNAP_COLCON_ROOT/opt/ros/crystal"}',
             },
             {
                 "path": "test/local_setup.sh",
-                "contents": ": ${AMENT_CURRENT_PREFIX:=/foo/bar}",
-                "expected": ': ${{AMENT_CURRENT_PREFIX:="{}/foo/bar"}}'.format(
-                    self.plugin.installdir
-                ),
+                "contents": ': ${AMENT_CURRENT_PREFIX:="/foo/bar"}',
+                "expected": ': ${AMENT_CURRENT_PREFIX:="$SNAP_COLCON_ROOT/foo/bar"}',
             },
             {
                 "path": "test/no-change.sh",
@@ -696,14 +699,14 @@ def test_ament_prefix_is_rewritten(self):
             {
                 "path": "setup.sh",
                 "contents": ": ${AMENT_CURRENT_PREFIX:=/opt/ros/crystal}",
-                "expected": ': ${AMENT_CURRENT_PREFIX:="$SNAP/opt/ros/crystal"}',
+                "expected": ': ${AMENT_CURRENT_PREFIX:="$SNAP_COLCON_ROOT/opt/ros/crystal"}',
             },
             {
                 "path": "test/local_setup.sh",
                 "contents": ': ${{AMENT_CURRENT_PREFIX:="{}/foo/bar"}}'.format(
                     self.plugin.installdir
                 ),
-                "expected": ': ${AMENT_CURRENT_PREFIX:="$SNAP/foo/bar"}',
+                "expected": ': ${AMENT_CURRENT_PREFIX:="$SNAP_COLCON_ROOT/foo/bar"}',
             },
             {
                 "path": "test/no-change.sh",
@@ -731,21 +734,38 @@ def test_colcon_prefix_is_rewritten(self):
         files = [
             {
                 "path": "setup.sh",
-                "contents": "_colcon_prefix_chain_sh_COLCON_CURRENT_PREFIX=/foo/bar",
-                "expected": '_colcon_prefix_chain_sh_COLCON_CURRENT_PREFIX="$SNAP/foo/bar"',
+                "contents": "_colcon_prefix_sh_COLCON_CURRENT_PREFIX=/foo/bar",
+                "expected": '_colcon_prefix_sh_COLCON_CURRENT_PREFIX="$SNAP_COLCON_ROOT/foo/bar"',
             },
             {
                 "path": "test/local_setup.sh",
-                "contents": '_colcon_prefix_sh_COLCON_CURRENT_PREFIX="{}/baz"'.format(
+                "contents": '_colcon_package_sh_COLCON_CURRENT_PREFIX="{}/baz"'.format(
                     self.plugin.installdir
                 ),
-                "expected": '_colcon_prefix_sh_COLCON_CURRENT_PREFIX="$SNAP/baz"',
+                "expected": '_colcon_package_sh_COLCON_CURRENT_PREFIX="$SNAP_COLCON_ROOT/baz"',
             },
             {
-                "path": "test/no-change.sh",
+                "path": "test/another_local_setup.sh",
+                "contents": 'COLCON_CURRENT_PREFIX="{}/qux"'.format(
+                    self.plugin.installdir
+                ),
+                "expected": 'COLCON_CURRENT_PREFIX="$SNAP_COLCON_ROOT/qux"',
+            },
+            {
+                "path": "test/no-change1.sh",
                 "contents": "_colcon_prefix_chain_sh_COLCON_CURRENT_PREFIX=$NOT_A_PATH",
                 "expected": "_colcon_prefix_chain_sh_COLCON_CURRENT_PREFIX=$NOT_A_PATH",
             },
+            {
+                "path": "test/no-change2.sh",
+                "contents": "_colcon_package_bash_COLCON_CURRENT_PREFIX=/foo/bar",
+                "expected": "_colcon_package_bash_COLCON_CURRENT_PREFIX=/foo/bar",
+            },
+            {
+                "path": "test/no-change3.sh",
+                "contents": "COLCON_CURRENT_PREFIX=/foo/bar",
+                "expected": "COLCON_CURRENT_PREFIX=/foo/bar",
+            },
         ]
 
         for file_info in files:
@@ -768,14 +788,14 @@ def test_colcon_python_executable_is_rewritten(self):
             {
                 "path": "setup.sh",
                 "contents": "_colcon_python_executable=/usr/bin/python3",
-                "expected": '_colcon_python_executable="$SNAP/usr/bin/python3"',
+                "expected": '_colcon_python_executable="$SNAP_COLCON_ROOT/usr/bin/python3"',
             },
             {
                 "path": "test/local_setup.sh",
                 "contents": '_colcon_python_executable="{}/baz/python3"'.format(
                     self.plugin.installdir
                 ),
-                "expected": '_colcon_python_executable="$SNAP/baz/python3"',
+                "expected": '_colcon_python_executable="$SNAP_COLCON_ROOT/baz/python3"',
             },
             {
                 "path": "test/no-change.sh",