Only load a manifest if it has not been loaded

docs/tutorials/otio-serialized-schema.md

@@ -221,11 +221,13 @@ parameters:
 ```
 Defines an OTIO plugin Manifest.
 
-    This is an internal OTIO implementation detail.  A manifest tracks a
-    collection of adapters and allows finding specific adapters by suffix
+    This is considered an internal OTIO implementation detail.
 
-    For writing your own adapters, consult:
-        https://opentimelineio.readthedocs.io/en/latest/tutorials/write-an-adapter.html#
+    A manifest tracks a collection of plugins and enables finding them by name
+    or other features (in the case of adapters, what file suffixes they
+    advertise support for).
+
+    For more information, consult the documenation.
 
 ```
 

src/py-opentimelineio/opentimelineio/plugins/manifest.py

@@ -22,7 +22,7 @@
 # language governing permissions and limitations under the Apache License.
 #
 
-"""Implementation of an adapter registry system for OTIO."""
+"""OTIO Python Plugin Manifest system: locates plugins to OTIO."""
 
 import inspect
 import logging
@@ -60,8 +60,9 @@ def manifest_from_file(filepath):
     """Read the .json file at filepath into a Manifest object."""
 
     result = core.deserialize_json_from_file(filepath)
-    result.source_files.append(filepath)
-    result._update_plugin_source(filepath)
+    absfilepath = os.path.abspath(filepath)
+    result.source_files.append(absfilepath)
+    result._update_plugin_source(absfilepath)
     return result
 
 
@@ -78,7 +79,7 @@ def manifest_from_string(input_string):
         name = "{}:{}".format(stack[1][1], stack[1][3])
 
     # set the value in the manifest
-    src_string = "call to manifest_from_string() in " + name
+    src_string = "call to manifest_from_string() in {}".format(name)
     result.source_files.append(src_string)
     result._update_plugin_source(src_string)
 
@@ -89,11 +90,13 @@ def manifest_from_string(input_string):
 class Manifest(core.SerializableObject):
     """Defines an OTIO plugin Manifest.
 
-    This is an internal OTIO implementation detail.  A manifest tracks a
-    collection of adapters and allows finding specific adapters by suffix
+    This is considered an internal OTIO implementation detail.
 
-    For writing your own adapters, consult:
-        https://opentimelineio.readthedocs.io/en/latest/tutorials/write-an-adapter.html#
+    A manifest tracks a collection of plugins and enables finding them by name
+    or other features (in the case of adapters, what file suffixes they
+    advertise support for).
+
+    For more information, consult the documenation.
     """
     _serializable_label = "PluginManifest.1"
 
@@ -136,8 +139,10 @@ def __init__(self):
 
     def extend(self, another_manifest):
         """
-        Extend the adapters, schemadefs, and media_linkers lists of this manifest
-        by appending the contents of the corresponding lists of another_manifest.
+        Aggregate another manifest's plugins into this one.
+
+        During startup, OTIO will deserialize the individual manifest JSON files
+        and use this function to concatenate them together.
         """
         if not another_manifest:
             return
@@ -155,10 +160,14 @@ def extend(self, another_manifest):
         self.source_files.extend(another_manifest.source_files)
 
     def _update_plugin_source(self, path):
-        """Track the source .json for a given adapter."""
+        """Set the source file path for the manifest."""
 
-        for thing in (self.adapters + self.schemadefs
-                      + self.media_linkers + self.hook_scripts):
+        for thing in (
+            self.adapters
+            + self.schemadefs
+            + self.media_linkers
+            + self.hook_scripts
+        ):
             thing._json_path = path
 
     def from_filepath(self, suffix):
@@ -175,8 +184,10 @@ def adapter_module_from_suffix(self, suffix):
         adp = self.from_filepath(suffix)
         return adp.module()
 
+    # @TODO: (breaking change) this should search all plugins by default instead
+    #        of just adapters
     def from_name(self, name, kind_list="adapters"):
-        """Return the adapter object associated with a given adapter name."""
+        """Return the plugin object associated with a given plugin name."""
 
         for thing in getattr(self, kind_list):
             if name == thing.name:
@@ -208,13 +219,29 @@ def schemadef_module_from_name(self, name):
 
 
 def load_manifest():
-    # read local adapter manifests, if they exist
-    # do this first, so that users can supersede internal adapters
+    """ Walk the plugin manifest discovery systems and accumulate manifests.
+
+    The order of loading (and precedence) is:
+        1. manifests specfied via the OTIO_PLUGIN_MANIFEST_PATH variable
+        2. builtin plugin manifest
+        3. contrib plugin manifest
+        4. setuptools.pkg_resources based plugin manifests
+    """
+
     result = Manifest()
+
+    # Read plugin manifests defined on the $OTIO_PLUGIN_MANIFEST_PATH
+    # environment variable.  This variable is an os.pathsep separated list of
+    # file paths to manifest json files.
     _local_manifest_path = os.environ.get("OTIO_PLUGIN_MANIFEST_PATH", None)
     if _local_manifest_path is not None:
-        for json_path in _local_manifest_path.split(os.pathsep):
-            if not os.path.exists(json_path):
+        for src_json_path in _local_manifest_path.split(os.pathsep):
+            json_path = os.path.abspath(src_json_path)
+            if (
+                not os.path.exists(json_path)
+                # the manifest has already been loaded
+                or json_path in result.source_files
+            ):
                 # XXX: In case error reporting is requested
                 # print(
                 #     "Warning: OpenTimelineIO cannot access path '{}' from "
@@ -224,32 +251,33 @@ def load_manifest():
 
             result.extend(manifest_from_file(json_path))
 
-    # build the manifest of adapters, starting with builtin adapters
-    plugin_manifest = manifest_from_file(
-        os.path.join(
-            os.path.dirname(os.path.dirname(inspect.getsourcefile(core))),
-            "adapters",
-            "builtin_adapters.plugin_manifest.json"
-        )
+    # the builtin plugin manifest
+    builtin_manifest_path = os.path.join(
+        os.path.dirname(os.path.dirname(inspect.getsourcefile(core))),
+        "adapters",
+        "builtin_adapters.plugin_manifest.json"
     )
-    result.extend(plugin_manifest)
+    if os.path.abspath(builtin_manifest_path) not in result.source_files:
+        plugin_manifest = manifest_from_file(builtin_manifest_path)
+        result.extend(plugin_manifest)
 
-    # layer contrib plugins after built in ones
+    # the contrib plugin manifest (located in the opentimelineio_contrib package)
     try:
         import opentimelineio_contrib as otio_c
 
-        contrib_manifest = manifest_from_file(
-            os.path.join(
-                os.path.dirname(inspect.getsourcefile(otio_c)),
-                "adapters",
-                "contrib_adapters.plugin_manifest.json"
-            )
+        contrib_manifest_path = os.path.join(
+            os.path.dirname(inspect.getsourcefile(otio_c)),
+            "adapters",
+            "contrib_adapters.plugin_manifest.json"
         )
-        result.extend(contrib_manifest)
+        if os.path.abspath(contrib_manifest_path) not in result.source_files:
+            contrib_manifest = manifest_from_file(contrib_manifest_path)
+            result.extend(contrib_manifest)
+
     except ImportError:
         pass
 
-    # Discover setuptools-based plugins
+    # setuptools.pkg_resources based plugins
     if pkg_resources:
         for plugin in pkg_resources.iter_entry_points(
                 "opentimelineio.plugins"
@@ -259,12 +287,38 @@ def load_manifest():
                 plugin_entry_point = plugin.load()
                 try:
                     plugin_manifest = plugin_entry_point.plugin_manifest()
+
+                    # this ignores what the plugin_manifest() function might
+                    # put into source_files in favor of using the path to the
+                    # python package as the unique identifier
+
+                    manifest_path = os.path.abspath(
+                        plugin_entry_point.__file__
+                    )
+
+                    if manifest_path in result.source_files:
+                        continue
+
+                    plugin_manifest.source_files = [manifest_path]
+                    plugin_manifest._update_plugin_source(manifest_path)
+
                 except AttributeError:
                     if not pkg_resources.resource_exists(
                             plugin.module_name,
                             'plugin_manifest.json'
                     ):
                         raise
+
+                    filepath = os.path.abspath(
+                        pkg_resources.resource_filename(
+                            plugin.module_name,
+                            'plugin_manifest.json'
+                        )
+                    )
+
+                    if filepath in result.source_files:
+                        continue
+
                     manifest_stream = pkg_resources.resource_stream(
                         plugin.module_name,
                         'plugin_manifest.json'
@@ -273,11 +327,9 @@ def load_manifest():
                         manifest_stream.read().decode('utf-8')
                     )
                     manifest_stream.close()
-                    filepath = pkg_resources.resource_filename(
-                        plugin.module_name,
-                        'plugin_manifest.json'
-                    )
+
                     plugin_manifest._update_plugin_source(filepath)
+                    plugin_manifest.source_files.append(filepath)
 
             except Exception:
                 logging.exception(
@@ -294,10 +346,13 @@ def load_manifest():
     # force the schemadefs to load and add to schemadef module namespace
     for s in result.schemadefs:
         s.module()
+
     return result
 
 
 def ActiveManifest(force_reload=False):
+    """Return the fully resolved plugin manifest."""
+
     global _MANIFEST
     if not _MANIFEST or force_reload:
         _MANIFEST = load_manifest()

tests/test_adapter_plugin.py

@@ -199,6 +199,49 @@ def test_find_adapter_by_name(self):
             "path"
         )
 
+    def test_deduplicate_env_variable_paths(self):
+        "Ensure that duplicate entries in the environment variable are ignored"
+
+        basename = "unittest.plugin_manifest.json"
+
+        # back up existing manifest
+        bak = otio.plugins.manifest._MANIFEST
+        bak_env = os.environ.get('OTIO_PLUGIN_MANIFEST_PATH')
+
+        # Generate a fake manifest in a temp file, and point at it with
+        # the environment variable
+        with tempfile.TemporaryDirectory(
+            prefix='test_find_manifest_by_environment_variable'
+        ) as temp_dir:
+            temp_file = os.path.join(temp_dir, basename)
+            otio.adapters.write_to_file(self.man, temp_file, 'otio_json')
+
+            # clear out existing manifest
+            otio.plugins.manifest._MANIFEST = None
+
+            # set where to find the new manifest
+            os.environ['OTIO_PLUGIN_MANIFEST_PATH'] = (
+                temp_file
+                # add it twice
+                + os.pathsep + temp_file
+            )
+            result = otio.plugins.manifest.load_manifest()
+
+            # ... should only appear once in the result
+            self.assertEqual(result.source_files.count(temp_file), 1)
+
+            # Rather than try and remove any other setuptools based plugins
+            # that might be installed, this check is made more permissive to
+            # see if the known unit test linker is being loaded by the manifest
+            self.assertTrue(len(result.media_linkers) > 0)
+            self.assertIn("example", (ml.name for ml in result.media_linkers))
+
+            otio.plugins.manifest._MANIFEST = bak
+            if bak_env:
+                os.environ['OTIO_PLUGIN_MANIFEST_PATH'] = bak_env
+            else:
+                del os.environ['OTIO_PLUGIN_MANIFEST_PATH']
+
     def test_find_manifest_by_environment_variable(self):
         basename = "unittest.plugin_manifest.json"
 
@@ -218,7 +261,9 @@ def test_find_manifest_by_environment_variable(self):
             otio.plugins.manifest._MANIFEST = None
 
             # set where to find the new manifest
-            os.environ['OTIO_PLUGIN_MANIFEST_PATH'] = temp_file + os.pathsep + 'foo'
+            os.environ['OTIO_PLUGIN_MANIFEST_PATH'] = (
+                temp_file + os.pathsep + 'foo'
+            )
             result = otio.plugins.manifest.load_manifest()
 
             # Rather than try and remove any other setuptools based plugins

tests/test_plugin_detection.py

@@ -62,6 +62,11 @@ def setUp(self):
             baseline_reader.path_to_baseline_directory(),
             'plugin_module',
         )
+        self.mock_module_manifest_path = os.path.join(
+            mock_module_path,
+            "otio_jsonplugin",
+            "plugin_manifest.json"
+        )
 
         # Create a WorkingSet as if the module were installed
         entries = [mock_module_path] + pkg_resources.working_set.entries
@@ -140,6 +145,50 @@ def test_detect_plugin_json_manifest(self):
         for linker in man.media_linkers:
             self.assertIsInstance(linker, otio.media_linker.MediaLinker)
 
+        self.assertTrue(
+            any(
+                True for p in man.source_files
+                if self.mock_module_manifest_path in p
+            )
+        )
+
+    def test_deduplicate_env_variable_paths(self):
+        "Ensure that duplicate entries in the environment variable are ignored"
+
+        # back up existing manifest
+        bak_env = os.environ.get('OTIO_PLUGIN_MANIFEST_PATH')
+
+        relative_path = self.mock_module_manifest_path.replace(os.getcwd(), '.')
+
+        # set where to find the new manifest
+        os.environ['OTIO_PLUGIN_MANIFEST_PATH'] = os.pathsep.join(
+            (
+                # absolute
+                self.mock_module_manifest_path,
+
+                # relative
+                relative_path
+            )
+        )
+
+        result = otio.plugins.manifest.load_manifest()
+        self.assertEqual(
+            len(
+                [
+                    p for p in result.source_files
+                    if self.mock_module_manifest_path in p
+                ]
+            ),
+            1
+        )
+        if relative_path != self.mock_module_manifest_path:
+            self.assertNotIn(relative_path, result.source_files)
+
+        if bak_env:
+            os.environ['OTIO_PLUGIN_MANIFEST_PATH'] = bak_env
+        else:
+            del os.environ['OTIO_PLUGIN_MANIFEST_PATH']
+
 
 if __name__ == '__main__':
     unittest.main()