Enable gems with found implicit gem dependencies

Signed-off-by: Alex Peterson <[email protected]>

scripts/o3de/o3de/compatibility.py

@@ -70,7 +70,7 @@ def get_incompatible_gem_dependencies(gem_json_data:dict, all_gems_json_data:dic
     if not gem_dependencies:
         return set()
 
-    return get_incompatible_gem_version_specifiers(gem_dependencies, all_gems_json_data)
+    return get_incompatible_gem_version_specifiers(gem_json_data, all_gems_json_data, checked_specifiers=set())
 
 
 def get_gem_project_incompatible_objects(gem_json_data:dict, project_path:pathlib.Path, all_gems_json_data:dict = None) -> set:
@@ -100,7 +100,7 @@ def get_gem_project_incompatible_objects(gem_json_data:dict, project_path:pathli
         logger.error(f'Failed to load engine.json data based on the engine field in project.json or detect the engine from the current folder')
         return set(f'engine.json (missing)') 
 
-    if not all_gems_json_data:
+    if not isinstance(all_gems_json_data, dict):
         all_gems_json_data = manifest.get_gems_json_data_by_name(engine_path, project_path, include_manifest_gems=True)
 
     # compatibility will be based on the engine the project uses and the gems visible to
@@ -170,13 +170,14 @@ def get_incompatible_objects_for_engine(object_json_data:dict, engine_json_data:
     return incompatible_objects
 
 
-def get_incompatible_gem_version_specifiers(gem_version_specifier_list:list, all_gems_json_data:dict) -> set:
+def get_incompatible_gem_version_specifiers(gem_json_data:dict, all_gems_json_data:dict, checked_specifiers:set) -> set:
     """
     Returns a set of gem version specifiers that are not compatible with the gem's provided
     If a gem_version_specifier_list entry only has a gem name, it is assumed compatible with every gem version with that name.
     :param gem_version_specifier_list: a list of gem names and (optional)version specifiers
     :param all_gems_json_data: json data of all gems to use for compatibility checks
     """
+    gem_version_specifier_list = gem_json_data.get('dependencies')
     if not gem_version_specifier_list:
         return set()
 
@@ -185,21 +186,38 @@ def get_incompatible_gem_version_specifiers(gem_version_specifier_list:list, all
         return set(gem_version_specifier_list)
 
     incompatible_gem_version_specifiers = set()
+
+    # helper function to check dependency tree for incompatible gem dependencies
+    def get_gem_dependency_version_specifiers(gem_name):
+        gem_dependencies = all_gems_json_data[gem_name].get('dependencies')
+        if gem_dependencies:
+            incompatible_dependency_specifiers = get_incompatible_gem_version_specifiers(all_gems_json_data[gem_name], all_gems_json_data, checked_specifiers)
+            if incompatible_dependency_specifiers:
+                incompatible_gem_version_specifiers.update(incompatible_dependency_specifiers)
+
     for gem_version_specifier in gem_version_specifier_list:
+        if gem_version_specifier in checked_specifiers:
+            continue
+
+        checked_specifiers.add(gem_version_specifier)
+
         gem_name, version_specifier = utils.get_object_name_and_optional_version_specifier(gem_version_specifier)
         if not gem_name in all_gems_json_data:
-            incompatible_gem_version_specifiers.add(f"{gem_version_specifier} (missing dependency)")
+            incompatible_gem_version_specifiers.add(f"{gem_json_data['gem_name']} is missing the dependency {gem_version_specifier}")
             continue
 
         if not version_specifier:
             # when no version specifier is provided we assume compatibility with any version
+            get_gem_dependency_version_specifiers(gem_name)
             continue
         
         gem_version = all_gems_json_data[gem_name].get('version')
         if gem_version and not has_compatible_version([gem_version_specifier], gem_name, gem_version):
-            incompatible_gem_version_specifiers.add(f"{gem_version_specifier} (different version found ${gem_version})")
+            incompatible_gem_version_specifiers.add(f"{gem_json_data['gem_name']} depends on {gem_version_specifier} but {gem_name} version {gem_version} was found")
             continue
 
+        get_gem_dependency_version_specifiers(gem_name)
+
     return incompatible_gem_version_specifiers
 
 

scripts/o3de/o3de/enable_gem.py

@@ -120,8 +120,8 @@ def enable_gem_in_project(gem_name: str = None,
             # it takes about 150ms to populate this structure with 137 gems, 4696 bytes in total
             all_gems_json_data = manifest.get_gems_json_data_by_name(project_path=project_path, include_manifest_gems=True, include_engine_gems=True)
 
-            # remove any gems that are not active or dependencies
-            manifest.remove_non_dependency_gem_json_data(enabled_gem_names, all_gems_json_data)
+            # Note: we don't remove gems that are not active or dependencies
+            # because they will be implicitely found and activated via cmake 
 
             incompatible_objects = compatibility.get_gem_project_incompatible_objects(gem_json_data, project_path, all_gems_json_data)
             if incompatible_objects:

scripts/o3de/tests/test_enable_gem.py

@@ -207,7 +207,7 @@ class TestEnableGemCommand:
                 assert gem not in project_json.get('gem_names', [])
 
     @pytest.mark.parametrize("gem_registered_with_project, gem_registered_with_engine, "
-                             "gem_version, gem_dependencies, gem_names_and_versions, dry_run, force, "
+                             "gem_version, gem_dependencies, gem_json_data_by_name, dry_run, force, "
                              "compatible_engines, engine_api_dependencies, test_engine_name, test_engine_version, "
                              "test_engine_api_versions, is_optional_gem, expected_result", [
         # passes when no version information is provided
@@ -227,22 +227,28 @@ class TestEnableGemCommand:
         # fails when no compatible engine is found
         pytest.param(False, False, '1.2.3', [], {}, False, False, ['o3de-test<1.0.0'], [], 'other-engine', '0.0.0', {}, False, 1),
         # passes when dependent gems and engines are found
-        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':'1.2.3', 'testgem2':'2.0.0'}, 
+        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':{'version':'1.2.3'}, 'testgem2':{'version':'2.0.0'}}, 
+                    False, False, ['o3de-test<=1.0.0'], [], 'o3de-test', '1.0.0', {}, False, 0),
+        # passes when dependent gems with implicit dependencies and engines are found
+        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3'], { 'testgem1':{'version':'1.2.3','dependencies':['testgem2>1.0.0']}, 'testgem2':{'version':'2.0.0'}}, 
                     False, False, ['o3de-test<=1.0.0'], [], 'o3de-test', '1.0.0', {}, False, 0),
         # passes when dependent gems without version specifiers are used
-        pytest.param(False, False, '1.2.3', ['testgem1','testgem2'], { 'testgem1':'1.2.3', 'testgem2':'2.0.0'}, 
+        pytest.param(False, False, '1.2.3', ['testgem1','testgem2'], { 'testgem1':{'version':'1.2.3'}, 'testgem2':{'version':'2.0.0'}},
                     False, False, ['o3de-test<=1.0.0'], [], 'o3de-test', '1.0.0', {}, False, 0),
         # passes when dependent gems with and without version specifiers are used
-        pytest.param(False, False, '1.2.3', ['testgem1','testgem2~=2.0.0'], { 'testgem1':'1.2.3', 'testgem2':'2.0.1'}, 
+        pytest.param(False, False, '1.2.3', ['testgem1','testgem2~=2.0.0'], { 'testgem1':{'version':'1.2.3'}, 'testgem2':{'version':'2.0.1'}},
                     False, False, ['o3de-test<=1.0.0'], [], 'o3de-test', '1.0.0', {}, False, 0),
         # fails when dependent gem is not found
-        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':'1.2.3'}, 
+        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':{'version':'1.2.3'}},
+                    False, False, ['o3de-test<=1.0.0'], [], 'o3de-test', '0.0.0', {}, False, 1),
+        # fails when implicit dependent gem is not found
+        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3'], { 'testgem1':{'version':'1.2.3','dependencies':['testgem2>1.0.0']}},
                     False, False, ['o3de-test<=1.0.0'], [], 'o3de-test', '0.0.0', {}, False, 1),
         # fails when dependent gem with wrong version found
-        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':'1.0.0', 'testgem2':'1.0.0'}, 
+        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':{'version':'1.0.0'}, 'testgem2':{'version':'1.0.0'}},
                     False, False, ['o3de-test~=1.0.0'], [], 'o3de-test', '1.0.0', {}, False, 1),
         # does not modify project when check only
-        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':'1.2.3', 'testgem2':'2.0.0'}, 
+        pytest.param(False, False, '1.2.3', ['testgem1==1.2.3','testgem2>1.0.0'], { 'testgem1':{'version':'1.2.3'}, 'testgem2':{'version':'2.0.1'}},
                     True, False, ['o3de-test<=1.0.0'], [], 'o3de-test', '1.0.0', {}, False, 0),
         # passes when a engine api versions found
         pytest.param(False, False, '1.2.3', [], {}, False, False, [], ['api==1.2.3'], 'o3de-test', '1.0.0', {'api':'1.2.3'}, False, 0),
@@ -262,7 +268,7 @@ class TestEnableGemCommand:
     )
     def test_enable_gem_checks_engine_compatibility(self, gem_registered_with_project, gem_registered_with_engine, 
                                                     gem_version, gem_dependencies, 
-                                                    gem_names_and_versions, dry_run, force, compatible_engines, 
+                                                    gem_json_data_by_name, dry_run, force, compatible_engines, 
                                                     engine_api_dependencies, test_engine_name, test_engine_version, 
                                                     test_engine_api_versions, is_optional_gem, expected_result):
         project_path = pathlib.PurePath('TestProject')
@@ -293,7 +299,7 @@ class TestEnableGemCommand:
                                         include_manifest_gems: bool = False,
                                         include_engine_gems: bool = False) -> dict:
             all_gems_json_data = {}
-            for gem_name in gem_names_and_versions.keys():
+            for gem_name in gem_json_data_by_name.keys():
                 all_gems_json_data[gem_name] = get_gem_json_data(gem_name=gem_name)
             return all_gems_json_data
 
@@ -322,10 +328,10 @@ class TestEnableGemCommand:
         def get_gem_json_data(gem_name: str = None, gem_path: str or pathlib.Path = None,
                             project_path: pathlib.Path = None) -> dict or None:
             gem_data = self.enable_gem.gem_data.copy()
-            if gem_name in gem_names_and_versions:
+            if gem_name in gem_json_data_by_name:
                 # gem dependencies data
+                gem_data.update(gem_json_data_by_name[gem_name])
                 gem_data['gem_name'] = gem_name 
-                gem_data['version'] = gem_names_and_versions[gem_name] 
             else:
                 # test gem data
                 gem_data['version'] = gem_version
@@ -343,7 +349,7 @@ class TestEnableGemCommand:
             return set() 
 
         def get_engine_gems():
-            gem_paths = list(map(lambda path:pathlib.Path(path).resolve(), gem_names_and_versions.keys()))
+            gem_paths = list(map(lambda path:pathlib.Path(path).resolve(), gem_json_data_by_name.keys()))
             if gem_registered_with_engine:
                 gem_paths.append(pathlib.Path(gem_path).resolve())
             return gem_paths