From 5af369cf1fb05937d5eceb4d258cd653616577dd Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 7 Oct 2021 14:09:09 -0700
Subject: [PATCH 01/17] First prototype warning on build of overridden packages

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/shell/__init__.py | 99 ++++++++++++++++++++++++++---------
 colcon_core/verb/build.py     | 41 +++++++++++++++
 2 files changed, 115 insertions(+), 25 deletions(-)

diff --git a/colcon_core/shell/__init__.py b/colcon_core/shell/__init__.py
index 17cf1ca5..c19b356b 100644
--- a/colcon_core/shell/__init__.py
+++ b/colcon_core/shell/__init__.py
@@ -563,7 +563,7 @@ def find_installed_packages_in_environment():
     return packages
 
 
-def find_installed_packages(install_base):
+def find_installed_packages(install_base: Path):
     """
     Find install packages under the install base path.
 
@@ -577,32 +577,81 @@ def find_installed_packages(install_base):
       layout
     :rtype: OrderedDict or None
     """
-    marker_file = install_base / '.colcon_install_layout'
-    if not marker_file.is_file():
-        return None
-    install_layout = marker_file.read_text().rstrip()
-    if install_layout not in ('isolated', 'merged'):
-        return None
 
-    packages = {}
-    if install_layout == 'isolated':
-        # for each subdirectory look for the package specific file
-        for p in install_base.iterdir():
-            if not p.is_dir():
-                continue
-            if p.name.startswith('.'):
-                continue
-            marker = p / get_relative_package_index_path() / p.name
-            if marker.is_file():
-                packages[p.name] = p
-    else:
-        # find all files in the subdirectory
-        if (install_base / get_relative_package_index_path()).is_dir():
-            package_index = install_base / get_relative_package_index_path()
-            for p in package_index.iterdir():
-                if not p.is_file():
+    def colcon_installspace(install_base: Path):
+        marker_file = install_base / '.colcon_install_layout'
+        if not marker_file.is_file():
+            return None
+        install_layout = marker_file.read_text().rstrip()
+        if install_layout not in ('isolated', 'merged'):
+            return None
+
+        packages = {}
+        if install_layout == 'isolated':
+            # for each subdirectory look for the package specific file
+            for p in install_base.iterdir():
+                if not p.is_dir():
                     continue
                 if p.name.startswith('.'):
                     continue
-                packages[p.name] = install_base
+                marker = p / get_relative_package_index_path() / p.name
+                if marker.is_file():
+                    packages[p.name] = p
+        else:
+            # find all files in the subdirectory
+            if (install_base / get_relative_package_index_path()).is_dir():
+                package_index = install_base / get_relative_package_index_path()
+                for p in package_index.iterdir():
+                    if not p.is_file():
+                        continue
+                    if p.name.startswith('.'):
+                        continue
+                    packages[p.name] = install_base
+        return packages
+
+
+    def ament_workspace(install_base: Path):
+        package_index = install_base.joinpath(
+                'share', 'ament_index', 'resource_index', 'packages')
+
+        if not package_index.exists() or not package_index.is_dir():
+            return None
+
+        packages = {}
+        for marker_path in package_index.iterdir():
+            packages[marker_path.name] = install_base
+
+        return packages
+
+    extensions = [colcon_installspace, ament_workspace]
+
+    # Combine packages found by all extensions
+    packages = {}
+    valid_prefix = False
+
+    for ext in extensions:
+        ext_packages = ext(install_base)
+        if ext_packages is None:
+            continue
+
+        valid_prefix = True
+        for pkg, path in ext_packages.items():
+            if not path.exists():
+                logger.warning(
+                    "Ignoring '{pkg}' found at '{path}' because the path"
+                    ' does not exist.'.format_map(locals()))
+                continue
+            if pkg in packages and not path.samefile(packages[pkg]):
+                    # Same package found at different paths in the same prefix
+                    first_path = packages[pkg]
+                    logger.warning(
+                        "The package '{pkg}' previously found at "
+                        "'{first_path}' was found again at '{path}'."
+                        " Ignoring '{path}'".format_map(locals()))
+            else:
+                packages[pkg] = path
+
+    if not valid_prefix:
+        # No extension said this was a valid prefix
+        return None
     return packages
diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index 8a82d966..32c0e046 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -20,6 +20,8 @@
     as add_packages_arguments
 from colcon_core.package_selection import get_packages
 from colcon_core.plugin_system import satisfies_version
+from colcon_core.prefix_path import get_chained_prefix_path
+from colcon_core.shell import find_installed_packages
 from colcon_core.shell import get_shell_extensions
 from colcon_core.task import add_task_arguments
 from colcon_core.task import get_task_extension
@@ -106,6 +108,13 @@ def add_arguments(self, *, parser):  # noqa: D102
             help='Continue other packages when a package fails to build '
                  '(packages recursively depending on the failed package are '
                  'skipped)')
+        parser.add_argument(
+            '--allow-overriding',
+            action='extend',
+            default=[],
+            metavar='PKG_NAME',
+            nargs='+',
+            help='Allow building packages that exist in an underlay workspace')
         add_executor_arguments(parser)
         add_event_handler_arguments(parser)
 
@@ -133,6 +142,38 @@ def main(self, *, context):  # noqa: D102
         jobs, unselected_packages = self._get_jobs(
             context.args, decorators, install_base)
 
+        underlay_packages = {}
+        for prefix_path in get_chained_prefix_path():
+            packages = find_installed_packages(Path(prefix_path))
+            for pkg, path in packages.items():
+                if pkg not in underlay_packages:
+                    underlay_packages[pkg] = []
+                underlay_packages[pkg].append(str(path))
+
+        override_messages = {}
+        for overlay_package in jobs.keys():
+            if overlay_package in underlay_packages:
+                if overlay_package not in context.args.allow_overriding:
+                    override_messages[overlay_package] = (
+                        "'{overlay_package}' is selected,"
+                        ' but it already exists in one or more underlay'
+                        ' workspaces:\n\t'.format_map(locals()) +
+                        '\n\t'.join(underlay_packages[overlay_package]))
+
+        if override_messages:
+            override_msg = ('\n'.join(override_messages.values()) +
+                '\nNon-leaf overriden packages must be API and ABI compatible'
+                " with the package they're overriding, and other packages in"
+                ' the overlay must have special knowledge of how to handle'
+                ' include directories from merged underlay workspaces or else'
+                ' undefined behavior at runtime may occur.'
+                '\nIf you understand the risks and want to override a'
+                ' package anyways, add the following to the command'
+                ' line:'
+                '\n\t--allow-overriding ' +
+                ' '.join(sorted(override_messages.keys())))
+            raise RuntimeError(override_msg)
+
         on_error = OnError.interrupt \
             if not context.args.continue_on_error else OnError.skip_downstream
 

From 930d2eeadfb60a02701139ed328af11d1af8c38f Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 7 Oct 2021 15:34:50 -0700
Subject: [PATCH 02/17] Add FindInstalledPackagesExtensionPoint and improve
 override warning text

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/shell/__init__.py           | 99 +++++++++++++------------
 colcon_core/shell/installed_packages.py | 55 ++++++++++++++
 colcon_core/verb/build.py               | 24 +++---
 setup.cfg                               |  4 +
 4 files changed, 124 insertions(+), 58 deletions(-)
 create mode 100644 colcon_core/shell/installed_packages.py

diff --git a/colcon_core/shell/__init__.py b/colcon_core/shell/__init__.py
index c19b356b..1a495a93 100644
--- a/colcon_core/shell/__init__.py
+++ b/colcon_core/shell/__init__.py
@@ -563,6 +563,48 @@ def find_installed_packages_in_environment():
     return packages
 
 
+class FindInstalledPackagesExtensionPoint:
+    """
+    The interface for extensions to find installed packages.
+
+    This type of extension locates installed packages inside a prefix path.
+    """
+
+    """The version of this extension interface."""
+    EXTENSION_POINT_VERSION = '1.0'
+
+    """The default priority of an extension."""
+    PRIORITY = 100
+
+    def find_installed_packages(self, install_base: Path):
+        """
+        Find installed packages in an install path.
+
+        This method must be overridden in a subclass.
+
+        :param Path prefix_path: The path of the install prefix
+        :returns: The mapping from a package name to the prefix path, or None
+            if the path is not an install layout supported by this extension.
+        :rtype: Dict or None
+        """
+        raise NotImplementedError()
+
+
+def get_find_installed_packages_extensions():
+    """
+    Get the available package identification extensions.
+
+    The extensions are grouped by their priority and each group is ordered by
+    the entry point name.
+
+    :rtype: OrderedDict
+    """
+    extensions = instantiate_extensions(__name__ + '.find_installed_packages')
+    for name, extension in extensions.items():
+        extension.PACKAGE_IDENTIFICATION_NAME = name
+    return order_extensions_grouped_by_priority(extensions)
+
+
 def find_installed_packages(install_base: Path):
     """
     Find install packages under the install base path.
@@ -575,62 +617,21 @@ def find_installed_packages(install_base: Path):
     :returns: The mapping from a package name to the prefix path, None if the
       path doesn't exist or doesn't contain a marker file with a valid install
       layout
-    :rtype: OrderedDict or None
-    """
-
-    def colcon_installspace(install_base: Path):
-        marker_file = install_base / '.colcon_install_layout'
-        if not marker_file.is_file():
-            return None
-        install_layout = marker_file.read_text().rstrip()
-        if install_layout not in ('isolated', 'merged'):
-            return None
-
-        packages = {}
-        if install_layout == 'isolated':
-            # for each subdirectory look for the package specific file
-            for p in install_base.iterdir():
-                if not p.is_dir():
-                    continue
-                if p.name.startswith('.'):
-                    continue
-                marker = p / get_relative_package_index_path() / p.name
-                if marker.is_file():
-                    packages[p.name] = p
-        else:
-            # find all files in the subdirectory
-            if (install_base / get_relative_package_index_path()).is_dir():
-                package_index = install_base / get_relative_package_index_path()
-                for p in package_index.iterdir():
-                    if not p.is_file():
-                        continue
-                    if p.name.startswith('.'):
-                        continue
-                    packages[p.name] = install_base
-        return packages
-
-
-    def ament_workspace(install_base: Path):
-        package_index = install_base.joinpath(
-                'share', 'ament_index', 'resource_index', 'packages')
-
-        if not package_index.exists() or not package_index.is_dir():
-            return None
-
-        packages = {}
-        for marker_path in package_index.iterdir():
-            packages[marker_path.name] = install_base
-
-        return packages
+    :rtype: Dict or None
+    """
 
-    extensions = [colcon_installspace, ament_workspace]
+    # priority means getting invoked first, but maybe that doesn't matter
+    extensions = []
+    prioritized_extensions = get_find_installed_packages_extensions()
+    for ext_list in prioritized_extensions.values():
+        extensions.extend(ext_list.values())
 
     # Combine packages found by all extensions
     packages = {}
     valid_prefix = False
 
     for ext in extensions:
-        ext_packages = ext(install_base)
+        ext_packages = ext.find_installed_packages(install_base)
         if ext_packages is None:
             continue
 
diff --git a/colcon_core/shell/installed_packages.py b/colcon_core/shell/installed_packages.py
new file mode 100644
index 00000000..8b56631b
--- /dev/null
+++ b/colcon_core/shell/installed_packages.py
@@ -0,0 +1,55 @@
+# Copyright 2016-2021 Dirk Thomas
+# Licensed under the Apache License, Version 2.0
+
+from pathlib import Path
+
+from colcon_core.location import get_relative_package_index_path
+from colcon_core.shell import FindInstalledPackagesExtensionPoint
+
+
+class IsolatedInstalledPackageFinder(FindInstalledPackagesExtensionPoint):
+    """Find installed packages in colcon isolated install spaces."""
+
+    def find_installed_packages(self, install_base: Path):
+        marker_file = install_base / '.colcon_install_layout'
+        if not marker_file.is_file():
+            return None
+        install_layout = marker_file.read_text().rstrip()
+        if install_layout == 'isolated':
+            return None
+
+        packages = {}
+        # for each subdirectory look for the package specific file
+        for p in install_base.iterdir():
+            if not p.is_dir():
+                continue
+            if p.name.startswith('.'):
+                continue
+            marker = p / get_relative_package_index_path() / p.name
+            if marker.is_file():
+                packages[p.name] = p
+        return packages
+
+
+class MergedInstalledPackageFinder(FindInstalledPackagesExtensionPoint):
+    """Find installed packages in colcon merged install spaces."""
+
+    def find_installed_packages(self, install_base: Path):
+        marker_file = install_base / '.colcon_install_layout'
+        if not marker_file.is_file():
+            return None
+        install_layout = marker_file.read_text().rstrip()
+        if install_layout == 'merged':
+            return None
+
+        packages = {}
+        # find all files in the subdirectory
+        if (install_base / get_relative_package_index_path()).is_dir():
+            package_index = install_base / get_relative_package_index_path()
+            for p in package_index.iterdir():
+                if not p.is_file():
+                    continue
+                if p.name.startswith('.'):
+                    continue
+                packages[p.name] = install_base
+        return packages
diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index 32c0e046..f2e3b572 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -155,18 +155,24 @@ def main(self, *, context):  # noqa: D102
             if overlay_package in underlay_packages:
                 if overlay_package not in context.args.allow_overriding:
                     override_messages[overlay_package] = (
-                        "'{overlay_package}' is selected,"
-                        ' but it already exists in one or more underlay'
-                        ' workspaces:\n\t'.format_map(locals()) +
+                        "'{overlay_package}'".format_map(locals()) +
+                        ' is selected and already exists in'
+                        '\n\t' +
                         '\n\t'.join(underlay_packages[overlay_package]))
 
         if override_messages:
-            override_msg = ('\n'.join(override_messages.values()) +
-                '\nNon-leaf overriden packages must be API and ABI compatible'
-                " with the package they're overriding, and other packages in"
-                ' the overlay must have special knowledge of how to handle'
-                ' include directories from merged underlay workspaces or else'
-                ' undefined behavior at runtime may occur.'
+            override_msg = ('Some packages already exist in one or more'
+                ' underlay workspaces.'
+                '\n' +
+                '\n'.join(override_messages.values()) +
+                '\nIf the overridden package is in a merged underlay workspace'
+                ' and it installs headers, then all packages in the overlay'
+                ' must specially order its include directories or undefined'
+                ' behavior at run time may occur.'
+                '\nIf the overridden package is used by a different package'
+                ' in any underlay, then the overriding package in the'
+                ' overlay must be API and ABI compatible or undefined'
+                ' behavior at run time may occur.'
                 '\nIf you understand the risks and want to override a'
                 ' package anyways, add the following to the command'
                 ' line:'
diff --git a/setup.cfg b/setup.cfg
index daebca5e..ded1dd3c 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -100,6 +100,7 @@ colcon_core.extension_point =
     colcon_core.prefix_path = colcon_core.prefix_path:PrefixPathExtensionPoint
     colcon_core.python_testing = colcon_core.task.python.test:PythonTestingStepExtensionPoint
     colcon_core.shell = colcon_core.shell:ShellExtensionPoint
+    colcon_core.shell.find_installed_packages = colcon_core.shell:FindInstalledPackagesExtensionPoint
     colcon_core.task.build = colcon_core.task:TaskExtensionPoint
     colcon_core.task.test = colcon_core.task:TaskExtensionPoint
     colcon_core.verb = colcon_core.verb:VerbExtensionPoint
@@ -120,6 +121,9 @@ colcon_core.shell =
     bat = colcon_core.shell.bat:BatShell
     dsv = colcon_core.shell.dsv:DsvShell
     sh = colcon_core.shell.sh:ShShell
+colcon_core.shell.find_installed_packages =
+    colcon_isolated = colcon_core.shell.installed_packages:IsolatedInstalledPackageFinder
+    colcon_merged = colcon_core.shell.installed_packages:MergedInstalledPackageFinder
 colcon_core.task.build =
     python = colcon_core.task.python.build:PythonBuildTask
 colcon_core.task.test =

From b921dd6955d0b6a06faf2d990b811b6033af4464 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 7 Oct 2021 16:00:36 -0700
Subject: [PATCH 03/17] its -> their

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/verb/build.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index f2e3b572..4302bbb9 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -167,7 +167,7 @@ def main(self, *, context):  # noqa: D102
                 '\n'.join(override_messages.values()) +
                 '\nIf the overridden package is in a merged underlay workspace'
                 ' and it installs headers, then all packages in the overlay'
-                ' must specially order its include directories or undefined'
+                ' must specially order their include directories or undefined'
                 ' behavior at run time may occur.'
                 '\nIf the overridden package is used by a different package'
                 ' in any underlay, then the overriding package in the'

From 26b5f3c5c3956b4341b98794d406f13d1eee8d38 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 7 Oct 2021 16:59:48 -0700
Subject: [PATCH 04/17] Fix CI

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/shell/__init__.py           | 14 ++++++--------
 colcon_core/shell/installed_packages.py |  6 ++++--
 colcon_core/verb/build.py               |  5 +++--
 test/spell_check.words                  |  1 +
 test/test_shell.py                      |  3 ++-
 5 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/colcon_core/shell/__init__.py b/colcon_core/shell/__init__.py
index 1a495a93..b08685a8 100644
--- a/colcon_core/shell/__init__.py
+++ b/colcon_core/shell/__init__.py
@@ -12,7 +12,6 @@
 import warnings
 
 from colcon_core.environment_variable import EnvironmentVariable
-from colcon_core.location import get_relative_package_index_path
 from colcon_core.logging import colcon_logger
 from colcon_core.plugin_system import instantiate_extensions
 from colcon_core.plugin_system import order_extensions_grouped_by_priority
@@ -619,7 +618,6 @@ def find_installed_packages(install_base: Path):
       layout
     :rtype: Dict or None
     """
-
     # priority means getting invoked first, but maybe that doesn't matter
     extensions = []
     prioritized_extensions = get_find_installed_packages_extensions()
@@ -643,12 +641,12 @@ def find_installed_packages(install_base: Path):
                     ' does not exist.'.format_map(locals()))
                 continue
             if pkg in packages and not path.samefile(packages[pkg]):
-                    # Same package found at different paths in the same prefix
-                    first_path = packages[pkg]
-                    logger.warning(
-                        "The package '{pkg}' previously found at "
-                        "'{first_path}' was found again at '{path}'."
-                        " Ignoring '{path}'".format_map(locals()))
+                # Same package found at different paths in the same prefix
+                first_path = packages[pkg]
+                logger.warning(
+                    "The package '{pkg}' previously found at "
+                    "'{first_path}' was found again at '{path}'."
+                    " Ignoring '{path}'".format_map(locals()))
             else:
                 packages[pkg] = path
 
diff --git a/colcon_core/shell/installed_packages.py b/colcon_core/shell/installed_packages.py
index 8b56631b..f7f6885e 100644
--- a/colcon_core/shell/installed_packages.py
+++ b/colcon_core/shell/installed_packages.py
@@ -11,11 +11,12 @@ class IsolatedInstalledPackageFinder(FindInstalledPackagesExtensionPoint):
     """Find installed packages in colcon isolated install spaces."""
 
     def find_installed_packages(self, install_base: Path):
+        """Find installed packages in colcon isolated install spaces."""
         marker_file = install_base / '.colcon_install_layout'
         if not marker_file.is_file():
             return None
         install_layout = marker_file.read_text().rstrip()
-        if install_layout == 'isolated':
+        if install_layout != 'isolated':
             return None
 
         packages = {}
@@ -35,11 +36,12 @@ class MergedInstalledPackageFinder(FindInstalledPackagesExtensionPoint):
     """Find installed packages in colcon merged install spaces."""
 
     def find_installed_packages(self, install_base: Path):
+        """Find installed packages in colcon isolated install spaces."""
         marker_file = install_base / '.colcon_install_layout'
         if not marker_file.is_file():
             return None
         install_layout = marker_file.read_text().rstrip()
-        if install_layout == 'merged':
+        if install_layout != 'merged':
             return None
 
         packages = {}
diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index 4302bbb9..fe2a6829 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -161,8 +161,9 @@ def main(self, *, context):  # noqa: D102
                         '\n\t'.join(underlay_packages[overlay_package]))
 
         if override_messages:
-            override_msg = ('Some packages already exist in one or more'
-                ' underlay workspaces.'
+            override_msg = (
+                'Some packages already exist in one or more underlay '
+                ' workspaces.'
                 '\n' +
                 '\n'.join(override_messages.values()) +
                 '\nIf the overridden package is in a merged underlay workspace'
diff --git a/test/spell_check.words b/test/spell_check.words
index b420f7cb..8a23e607 100644
--- a/test/spell_check.words
+++ b/test/spell_check.words
@@ -123,3 +123,4 @@ unrenamed
 veyor
 wildcards
 workaround
+workspaces
diff --git a/test/test_shell.py b/test/test_shell.py
index 7fc4d26a..1fa92785 100644
--- a/test/test_shell.py
+++ b/test/test_shell.py
@@ -356,7 +356,8 @@ def test_find_installed_packages():
         assert len(packages) == 0
 
         with patch(
-            'colcon_core.shell.get_relative_package_index_path',
+            'colcon_core.shell.installed_packages'
+            '.get_relative_package_index_path',
             return_value=Path('relative/package/index')
         ) as rel_path:
             # setup for isolated case

From 3a73414cecbf49bca0712884b6b575b1cab1d170 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 7 Oct 2021 17:08:22 -0700
Subject: [PATCH 05/17] Update docstring of find_installed_packages

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/shell/__init__.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/colcon_core/shell/__init__.py b/colcon_core/shell/__init__.py
index b08685a8..f1991c75 100644
--- a/colcon_core/shell/__init__.py
+++ b/colcon_core/shell/__init__.py
@@ -608,14 +608,12 @@ def find_installed_packages(install_base: Path):
     """
     Find install packages under the install base path.
 
-    The path must contain a marker file with the install layout.
-    Based on the install layout the packages are discovered i different
+    Based on the install layout the packages may be discovered in different
     locations.
 
     :param Path install_base: The base path to find installed packages in
     :returns: The mapping from a package name to the prefix path, None if the
-      path doesn't exist or doesn't contain a marker file with a valid install
-      layout
+      path doesn't exist or doesn't a valid install layout
     :rtype: Dict or None
     """
     # priority means getting invoked first, but maybe that doesn't matter

From a0e64df4d6f444feaf636db0cc2fe2b196b42e96 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 28 Oct 2021 12:01:33 -0700
Subject: [PATCH 06/17] Make unit tests aware of
 FindInstalledPackagesExtensionPoint

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 test/test_shell.py | 110 +++++++++++++++++++++++++++------------------
 1 file changed, 67 insertions(+), 43 deletions(-)

diff --git a/test/test_shell.py b/test/test_shell.py
index 1fa92785..9f9f5545 100644
--- a/test/test_shell.py
+++ b/test/test_shell.py
@@ -14,8 +14,12 @@
 from colcon_core.shell import get_colcon_prefix_path
 from colcon_core.shell import get_command_environment
 from colcon_core.shell import get_environment_variables
+from colcon_core.shell import get_find_installed_packages_extensions
 from colcon_core.shell import get_shell_extensions
+from colcon_core.shell import FindInstalledPackagesExtensionPoint
 from colcon_core.shell import ShellExtensionPoint
+from colcon_core.shell.installed_packages import IsolatedInstalledPackageFinder
+from colcon_core.shell.installed_packages import MergedInstalledPackageFinder
 from mock import Mock
 from mock import patch
 import pytest
@@ -312,6 +316,22 @@ def test_check_dependency_availability():
         assert '--packages-ignore pkgA' in warn.call_args[0][0]
 
 
+class FIExtension1(FindInstalledPackagesExtensionPoint):
+    PRIORITY = 90
+
+
+class FIExtension2(FindInstalledPackagesExtensionPoint):
+    pass
+
+
+def test_get_find_installed_packages_extensions():
+    with EntryPointContext(extension1=FIExtension1, extension2=FIExtension2):
+        extensions = get_find_installed_packages_extensions()
+    assert list(extensions.keys()) == [100, 90]
+    assert list(extensions[100].keys()) == ['extension2']
+    assert list(extensions[90].keys()) == ['extension1']
+
+
 def test_find_installed_packages_in_environment():
     with TemporaryDirectory(prefix='test_colcon_') as prefix_path:
         prefix_path = Path(prefix_path)
@@ -339,51 +359,55 @@ def test_find_installed_packages_in_environment():
 
 
 def test_find_installed_packages():
-    with TemporaryDirectory(prefix='test_colcon_') as install_base:
-        install_base = Path(install_base)
-
-        # install base doesn't exist
-        assert find_installed_packages(install_base) is None
+    with EntryPointContext(
+        colcon_isolated=IsolatedInstalledPackageFinder,
+        colcon_merged=MergedInstalledPackageFinder
+    ):
+        with TemporaryDirectory(prefix='test_colcon_') as install_base:
+            install_base = Path(install_base)
 
-        # unknown install layout
-        marker_file = install_base / '.colcon_install_layout'
-        marker_file.write_text('unknown')
-        assert find_installed_packages(install_base) is None
+            # install base doesn't exist
+            assert find_installed_packages(install_base) is None
 
-        # package index directory doesn't exist
-        marker_file.write_text('merged')
-        packages = find_installed_packages(install_base)
-        assert len(packages) == 0
-
-        with patch(
-            'colcon_core.shell.installed_packages'
-            '.get_relative_package_index_path',
-            return_value=Path('relative/package/index')
-        ) as rel_path:
-            # setup for isolated case
-            (install_base / 'dummy_file').write_text('')
-            (install_base / '.hidden_dir').mkdir()
-            (install_base / 'dummy_dir' / rel_path() / 'dummy_dir').mkdir(
-                parents=True)
-            (install_base / 'pkgA' / rel_path()).mkdir(parents=True)
-            (install_base / 'pkgA' / rel_path() / 'pkgA').write_text('')
-
-            # setup for merged case
-            (install_base / rel_path() / 'dummy_dir').mkdir(parents=True)
-            (install_base / rel_path() / '.dummy').write_text('')
-            (install_base / rel_path() / 'pkgB').write_text('')
-            (install_base / rel_path() / 'pkgC').write_text('')
-
-            marker_file.write_text('isolated')
-            packages = find_installed_packages(install_base)
-            assert len(packages) == 1
-            assert 'pkgA' in packages.keys()
-            assert packages['pkgA'] == install_base / 'pkgA'
+            # unknown install layout
+            marker_file = install_base / '.colcon_install_layout'
+            marker_file.write_text('unknown')
+            assert find_installed_packages(install_base) is None
 
+            # package index directory doesn't exist
             marker_file.write_text('merged')
             packages = find_installed_packages(install_base)
-            assert len(packages) == 2
-            assert 'pkgB' in packages.keys()
-            assert packages['pkgC'] == install_base
-            assert 'pkgC' in packages.keys()
-            assert packages['pkgB'] == install_base
+            assert len(packages) == 0
+
+            with patch(
+                'colcon_core.shell.installed_packages'
+                '.get_relative_package_index_path',
+                return_value=Path('relative/package/index')
+            ) as rel_path:
+                # setup for isolated case
+                (install_base / 'dummy_file').write_text('')
+                (install_base / '.hidden_dir').mkdir()
+                (install_base / 'dummy_dir' / rel_path() / 'dummy_dir').mkdir(
+                    parents=True)
+                (install_base / 'pkgA' / rel_path()).mkdir(parents=True)
+                (install_base / 'pkgA' / rel_path() / 'pkgA').write_text('')
+
+                # setup for merged case
+                (install_base / rel_path() / 'dummy_dir').mkdir(parents=True)
+                (install_base / rel_path() / '.dummy').write_text('')
+                (install_base / rel_path() / 'pkgB').write_text('')
+                (install_base / rel_path() / 'pkgC').write_text('')
+
+                marker_file.write_text('isolated')
+                packages = find_installed_packages(install_base)
+                assert len(packages) == 1
+                assert 'pkgA' in packages.keys()
+                assert packages['pkgA'] == install_base / 'pkgA'
+
+                marker_file.write_text('merged')
+                packages = find_installed_packages(install_base)
+                assert len(packages) == 2
+                assert 'pkgB' in packages.keys()
+                assert packages['pkgC'] == install_base
+                assert 'pkgC' in packages.keys()
+                assert packages['pkgB'] == install_base

From 956fa046dd38e4bd63e6cfec31971b27b615568e Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 28 Oct 2021 12:06:45 -0700
Subject: [PATCH 07/17] Fix import order

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 test/test_shell.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test_shell.py b/test/test_shell.py
index 9f9f5545..48f009bb 100644
--- a/test/test_shell.py
+++ b/test/test_shell.py
@@ -11,12 +11,12 @@
 from colcon_core.shell import create_environment_hook
 from colcon_core.shell import find_installed_packages
 from colcon_core.shell import find_installed_packages_in_environment
+from colcon_core.shell import FindInstalledPackagesExtensionPoint
 from colcon_core.shell import get_colcon_prefix_path
 from colcon_core.shell import get_command_environment
 from colcon_core.shell import get_environment_variables
 from colcon_core.shell import get_find_installed_packages_extensions
 from colcon_core.shell import get_shell_extensions
-from colcon_core.shell import FindInstalledPackagesExtensionPoint
 from colcon_core.shell import ShellExtensionPoint
 from colcon_core.shell.installed_packages import IsolatedInstalledPackageFinder
 from colcon_core.shell.installed_packages import MergedInstalledPackageFinder

From 51b12558dbedcb7a4c5e07bf4e113b99fad13541 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Thu, 28 Oct 2021 14:25:00 -0700
Subject: [PATCH 08/17] Test raises NotImplementedError

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 test/test_shell.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/test/test_shell.py b/test/test_shell.py
index 48f009bb..f19724f7 100644
--- a/test/test_shell.py
+++ b/test/test_shell.py
@@ -332,6 +332,11 @@ def test_get_find_installed_packages_extensions():
     assert list(extensions[90].keys()) == ['extension1']
 
 
+def test_find_installed_packages_extension_not_implemented():
+    with pytest.raises(NotImplementedError):
+        FindInstalledPackagesExtensionPoint().find_installed_packages(Path())
+
+
 def test_find_installed_packages_in_environment():
     with TemporaryDirectory(prefix='test_colcon_') as prefix_path:
         prefix_path = Path(prefix_path)

From 5caf0ad976b812008a0052d10dbbb9e1e1afa151 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@openrobotics.org>
Date: Mon, 22 Nov 2021 16:58:12 -0800
Subject: [PATCH 09/17] Add extend action for older python versions

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
---
 colcon_core/verb/build.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index fe2a6829..df41adb4 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -1,10 +1,12 @@
 # Copyright 2016-2018 Dirk Thomas
 # Licensed under the Apache License, Version 2.0
 
+import argparse
 from collections import OrderedDict
 import os
 import os.path
 from pathlib import Path
+import sys
 import traceback
 
 from colcon_core.argument_parser.destination_collector \
@@ -33,6 +35,19 @@
 from colcon_core.verb import VerbExtensionPoint
 
 
+if sys.version_info < (3,8):
+    # TODO(sloretz) remove when minimum supported Python version is 3.8
+    # https://stackoverflow.com/a/41153081
+    class _ExtendAction(argparse.Action):
+        """Add argparse action to extend a list."""
+
+        def __call__(self, parser, namespace, values, option_string=None):
+            """Extend the list with new arguments."""
+            items = getattr(namespace, self.dest) or []
+            items.extend(values)
+            setattr(namespace, self.dest, items)
+
+
 class BuildPackageArguments:
     """Arguments to build a specific package."""
 
@@ -83,6 +98,9 @@ def __init__(self):  # noqa: D107
         satisfies_version(VerbExtensionPoint.EXTENSION_POINT_VERSION, '^1.0')
 
     def add_arguments(self, *, parser):  # noqa: D102
+        if sys.version_info < (3,8):
+            # TODO(sloretz) remove when minimum supported Python version is 3.8
+            parser.register('action', 'extend', _ExtendAction)
         parser.add_argument(
             '--build-base',
             default='build',

From 4308cd68e8be6552f9367b969858c713411ea814 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@openrobotics.org>
Date: Mon, 22 Nov 2021 17:06:21 -0800
Subject: [PATCH 10/17] Flake8 tuple whitespace

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
---
 colcon_core/verb/build.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index df41adb4..b900a7eb 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -35,7 +35,7 @@
 from colcon_core.verb import VerbExtensionPoint
 
 
-if sys.version_info < (3,8):
+if sys.version_info < (3, 8):
     # TODO(sloretz) remove when minimum supported Python version is 3.8
     # https://stackoverflow.com/a/41153081
     class _ExtendAction(argparse.Action):
@@ -98,7 +98,7 @@ def __init__(self):  # noqa: D107
         satisfies_version(VerbExtensionPoint.EXTENSION_POINT_VERSION, '^1.0')
 
     def add_arguments(self, *, parser):  # noqa: D102
-        if sys.version_info < (3,8):
+        if sys.version_info < (3, 8):
             # TODO(sloretz) remove when minimum supported Python version is 3.8
             parser.register('action', 'extend', _ExtendAction)
         parser.add_argument(

From d3faf5e24fb726ec0784557cf851cd2958402f7a Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@openrobotics.org>
Date: Mon, 22 Nov 2021 17:06:29 -0800
Subject: [PATCH 11/17] Spellcheck extra words

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
---
 test/spell_check.words | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/spell_check.words b/test/spell_check.words
index d01f109f..85fcd564 100644
--- a/test/spell_check.words
+++ b/test/spell_check.words
@@ -94,7 +94,9 @@ setupscript
 setuptools
 shlex
 sigint
+sloretz
 stacklevel
+stackoverflow
 staticmethod
 stdeb
 stringify

From 5fdc6a40affd03a311f1520bdcc3ddb18d6a6a50 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Tue, 23 Nov 2021 09:24:18 -0800
Subject: [PATCH 12/17] Make override warning clearer

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/verb/build.py | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index b900a7eb..447b5006 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -132,7 +132,7 @@ def add_arguments(self, *, parser):  # noqa: D102
             default=[],
             metavar='PKG_NAME',
             nargs='+',
-            help='Allow building packages that exist in an underlay workspace')
+            help='Allow building packages that exist in underlay workspaces')
         add_executor_arguments(parser)
         add_event_handler_arguments(parser)
 
@@ -174,25 +174,25 @@ def main(self, *, context):  # noqa: D102
                 if overlay_package not in context.args.allow_overriding:
                     override_messages[overlay_package] = (
                         "'{overlay_package}'".format_map(locals()) +
-                        ' is selected and already exists in'
-                        '\n\t' +
-                        '\n\t'.join(underlay_packages[overlay_package]))
+                        ' is in: ' +
+                        ', '.join(underlay_packages[overlay_package]))
 
         if override_messages:
             override_msg = (
-                'Some packages already exist in one or more underlay '
-                ' workspaces.'
-                '\n' +
-                '\n'.join(override_messages.values()) +
-                '\nIf the overridden package is in a merged underlay workspace'
+                'Some selected packages are already built in one or more'
+                ' underlay workspaces:'
+                '\n\t' +
+                '\n\t'.join(override_messages.values()) +
+                '\nIf a package in a merged underlay workspace is overridden'
                 ' and it installs headers, then all packages in the overlay'
-                ' must specially order their include directories or undefined'
-                ' behavior at run time may occur.'
-                '\nIf the overridden package is used by a different package'
+                ' must sort their include directories by workspace order.'
+                ' Failure to do so may result in build failures or undefined'
+                ' behavior at run time.'
+                '\nIf the overridden package is used by another package'
                 ' in any underlay, then the overriding package in the'
                 ' overlay must be API and ABI compatible or undefined'
                 ' behavior at run time may occur.'
-                '\nIf you understand the risks and want to override a'
+                '\n\nIf you understand the risks and want to override a'
                 ' package anyways, add the following to the command'
                 ' line:'
                 '\n\t--allow-overriding ' +

From 6e6b45e01da5ddfe9415986b29d8ed9681e17b6e Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Mon, 29 Nov 2021 11:16:28 -0800
Subject: [PATCH 13/17] Grammar in path arg documentation

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/shell/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/colcon_core/shell/__init__.py b/colcon_core/shell/__init__.py
index f1991c75..b62fa3d0 100644
--- a/colcon_core/shell/__init__.py
+++ b/colcon_core/shell/__init__.py
@@ -613,7 +613,7 @@ def find_installed_packages(install_base: Path):
 
     :param Path install_base: The base path to find installed packages in
     :returns: The mapping from a package name to the prefix path, None if the
-      path doesn't exist or doesn't a valid install layout
+      path is not a supported install layout or it doesn't exist
     :rtype: Dict or None
     """
     # priority means getting invoked first, but maybe that doesn't matter

From ac6cf9220cc276c6a4a6504c251c5dd7e8585870 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Mon, 29 Nov 2021 11:18:48 -0800
Subject: [PATCH 14/17] Check packages is not None

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 colcon_core/verb/build.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/colcon_core/verb/build.py b/colcon_core/verb/build.py
index 447b5006..c52ae7eb 100644
--- a/colcon_core/verb/build.py
+++ b/colcon_core/verb/build.py
@@ -163,10 +163,11 @@ def main(self, *, context):  # noqa: D102
         underlay_packages = {}
         for prefix_path in get_chained_prefix_path():
             packages = find_installed_packages(Path(prefix_path))
-            for pkg, path in packages.items():
-                if pkg not in underlay_packages:
-                    underlay_packages[pkg] = []
-                underlay_packages[pkg].append(str(path))
+            if packages:
+                for pkg, path in packages.items():
+                    if pkg not in underlay_packages:
+                        underlay_packages[pkg] = []
+                    underlay_packages[pkg].append(str(path))
 
         override_messages = {}
         for overlay_package in jobs.keys():

From db813457d7356f423ebdb2064add572d2855e114 Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Wed, 1 Dec 2021 15:03:23 -0800
Subject: [PATCH 15/17] Test warning when extension reports package that
 doesn't exist

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 test/test_shell.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/test_shell.py b/test/test_shell.py
index f19724f7..a4220bba 100644
--- a/test/test_shell.py
+++ b/test/test_shell.py
@@ -416,3 +416,20 @@ def test_find_installed_packages():
                 assert packages['pkgC'] == install_base
                 assert 'pkgC' in packages.keys()
                 assert packages['pkgB'] == install_base
+
+
+class FIExtensionPathNotExist(FindInstalledPackagesExtensionPoint):
+
+    def find_installed_packages(self, install_base: Path):
+        return {'pkgA': Path('/does/not/exist')}
+
+
+def test_inconsistent_package_finding_extensions():
+    with EntryPointContext(dne=FIExtensionPathNotExist):
+        with TemporaryDirectory(prefix='test_colcon_') as install_base:
+            install_base = Path(install_base)
+            with patch('colcon_core.shell.logger.warning') as mock_warn:
+                assert {} == find_installed_packages(install_base)
+                mock_warn.assert_called_once_with(
+                    "Ignoring 'pkgA' found at '/does/not/exist'"
+                    ' because the path does not exist.')

From da8dfcf3dc838ebe98b2accfeeccad0584ec715b Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Wed, 1 Dec 2021 15:17:29 -0800
Subject: [PATCH 16/17] Test two extensions finding same package in same
 install base

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 test/test_shell.py | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/test/test_shell.py b/test/test_shell.py
index a4220bba..9540bca6 100644
--- a/test/test_shell.py
+++ b/test/test_shell.py
@@ -433,3 +433,30 @@ def test_inconsistent_package_finding_extensions():
                 mock_warn.assert_called_once_with(
                     "Ignoring 'pkgA' found at '/does/not/exist'"
                     ' because the path does not exist.')
+
+
+def test_find_package_two_locations():
+    with TemporaryDirectory(prefix='test_colcon_') as base:
+        base = Path(base)
+        location1 = base / 'pkgA'
+        location2 = base / 'pkgB'
+        location1.mkdir()
+        location2.mkdir()
+
+        class PackageLocation1(FindInstalledPackagesExtensionPoint):
+
+            def find_installed_packages(self, base: Path):
+                return {'pkgA': location1}
+
+        class PackageLocation2(FindInstalledPackagesExtensionPoint):
+
+            def find_installed_packages(self, base: Path):
+                return {'pkgA': location2}
+
+        with EntryPointContext(loc1=PackageLocation1, loc2=PackageLocation2):
+            with patch('colcon_core.shell.logger.warning') as mock_warn:
+                assert {'pkgA': location1} == find_installed_packages(base)
+                mock_warn.assert_called_once_with(
+                    "The package 'pkgA' previously found at"
+                    " '{location1}' was found again at '{location2}'."
+                    " Ignoring '{location2}'".format_map(locals()))

From 6c750f8a2a8687722feb35fd21e2b40f4d1d085e Mon Sep 17 00:00:00 2001
From: Shane Loretz <sloretz@osrfoundation.org>
Date: Wed, 1 Dec 2021 15:24:12 -0800
Subject: [PATCH 17/17] Fix expected string on windows

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
---
 test/test_shell.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/test/test_shell.py b/test/test_shell.py
index 9540bca6..29eede30 100644
--- a/test/test_shell.py
+++ b/test/test_shell.py
@@ -430,9 +430,10 @@ def test_inconsistent_package_finding_extensions():
             install_base = Path(install_base)
             with patch('colcon_core.shell.logger.warning') as mock_warn:
                 assert {} == find_installed_packages(install_base)
+                dne_path = Path('/does/not/exist')
                 mock_warn.assert_called_once_with(
-                    "Ignoring 'pkgA' found at '/does/not/exist'"
-                    ' because the path does not exist.')
+                    "Ignoring 'pkgA' found at '{0}'"
+                    ' because the path does not exist.'.format(dne_path))
 
 
 def test_find_package_two_locations():