Allow for Optional Gem annotations (#13421)

* Allow gems to be marked as optional

- An optional Gem is an external Gem that will still allow the project
  to be configured if the Gem isn't found.
- Updates the o3de scripts to allow enabling optional gems, plus related
  chagnes to support this.
- Updates Subdirectories cmake to support this extra json syntax.

Signed-off-by: amzn-phist <[email protected]>

* Fix o3de script tests

- A few of the test files were failing after these changes.

Signed-off-by: amzn-phist <[email protected]>

* Added a unit test for o3de scripts

- Simple test to validate a util function.

Signed-off-by: amzn-phist <[email protected]>

* Fixed a small issue

Signed-off-by: amzn-phist <[email protected]>

* Small fix to utils.py

- Cleanup and switch order of logic

Signed-off-by: amzn-phist <[email protected]>

* Addressing PR feedback

- Updated utility function to use safe-get on the dict key, collapsed
  the logic to a single block.
- Added new tests to the utils test.

Signed-off-by: amzn-phist <[email protected]>

* Address more PR feedback

- Moves the cmake logic for optional gems to an earlier function which
  will extract the gem names to a flat list and when it's optional set a
  global cmake property attached to the gem name.

Signed-off-by: amzn-phist <[email protected]>

* Addreses feedback

- Add docstring to _edit_gem_names functions
- Update dict accessor to use .get()

Signed-off-by: amzn-phist <[email protected]>

* Updated the docstrings further for accuracy

- Removed the :return: because it doesn't return anything.
- Updated description to describe the behavior.
- Updated a param description.

Signed-off-by: amzn-phist <[email protected]>

Signed-off-by: amzn-phist <[email protected]>

cmake/Subdirectories.cmake

@@ -155,10 +155,11 @@ function(query_gem_paths_from_external_subdirs output_gem_dirs gem_names registe
     if (gem_names)
         foreach(gem_name IN LISTS gem_names)
             unset(gem_path)
+            get_property(gem_optional GLOBAL PROPERTY ${gem_name}_OPTIONAL)
             o3de_find_gem_with_registered_external_subdirs(${gem_name} gem_path "${registered_external_subdirs}")
             if (gem_path)
                 list(APPEND gem_dirs ${gem_path})
-            else()
+            elseif(NOT gem_optional)
                 list(JOIN registered_external_subdirs "\n" external_subdirs_formatted)
                 message(SEND_ERROR "The gem \"${gem_name}\""
                 " could not be found in any gem.json from the following list of registered external subdirectories:\n"
@@ -252,7 +253,23 @@ endfunction()
 function(get_all_external_subdirectories_for_o3de_object output_subdirs object_type object_name object_path object_json_filename)
     # Append the gems referenced by name from "gem_names" field in the <object>.json
     # These gems are registered in the users o3de_manifest.json
-    o3de_read_json_array(gem_names ${object_path}/${object_json_filename} "gem_names")
+    o3de_read_json_array(initial_gem_names ${object_path}/${object_json_filename} "gem_names")
+    set(gem_names "")
+    foreach(gem_name IN LISTS initial_gem_names)
+        # Use the ERROR_VARIABLE to catch the common case when it's a simple string and not a json type.
+        string(JSON json_type ERROR_VARIABLE json_error TYPE ${gem_name})
+        set(gem_optional FALSE)
+        if(${json_type} STREQUAL "OBJECT")
+            string(JSON gem_optional GET ${gem_name} "optional")
+            string(JSON gem_name GET ${gem_name} "name")
+        endif()
+
+        # Set a global "optional" property on the gem name
+        set_property(GLOBAL PROPERTY "${gem_name}_OPTIONAL" ${gem_optional})
+        # Build the gem_names list with extracted names
+        list(APPEND gem_names ${gem_name})
+    endforeach()
+
     add_registered_gems_to_external_subdirs(object_gem_reference_dirs "${gem_names}")
     list(APPEND subdirs_for_object ${object_gem_reference_dirs})
 

scripts/o3de/o3de/disable_gem.py

@@ -99,9 +99,9 @@ def disable_gem_in_project(gem_name: str = None,
 
 
 def remove_explicit_gem_activation_for_all_paths(gem_root_folders: list,
-                                 project_name: str = None,
-                                 project_path: pathlib.Path = None,
-                                 enabled_gem_file: pathlib.Path = None) -> int:
+                                                 project_name: str = None,
+                                                 project_path: pathlib.Path = None,
+                                                 enabled_gem_file: pathlib.Path = None) -> int:
     """
     walks each gem root folder directory structure and removes explicit
     activation of the gems to the project
@@ -137,9 +137,9 @@ def remove_explicit_gem_activation_for_all_paths(gem_root_folders: list,
     for gem_dir in sorted(gem_dirs_set):
         # Run the command to remove explicit activation even if previous calls failed
         ret_val = disable_gem_in_project(gem_path=gem_dir,
-                                        project_name=project_name,
-                                        project_path=project_path,
-                                        enabled_gem_file=enabled_gem_file) or ret_val
+                                         project_name=project_name,
+                                         project_path=project_path,
+                                         enabled_gem_file=enabled_gem_file) or ret_val
 
     return ret_val
 

scripts/o3de/o3de/enable_gem.py

@@ -26,7 +26,8 @@ def enable_gem_in_project(gem_name: str = None,
                           gem_path: pathlib.Path = None,
                           project_name: str = None,
                           project_path: pathlib.Path = None,
-                          enabled_gem_file: pathlib.Path = None) -> int:
+                          enabled_gem_file: pathlib.Path = None,
+                          optional: bool = False) -> int:
     """
     enable a gem in a projects enabled_gems.cmake file
     :param gem_name: name of the gem to add
@@ -34,6 +35,7 @@ def enable_gem_in_project(gem_name: str = None,
     :param project_name: name of to the project to add the gem to
     :param project_path: path to the project to add the gem to
     :param enabled_gem_file: if this dependency goes/is in a specific file
+    :param optional: mark the gem as optional
     :return: 0 for success or non 0 failure code
     """
     # we need either a project name or path
@@ -103,7 +105,8 @@ def enable_gem_in_project(gem_name: str = None,
     ret_val = 0
     # If the gem is not part of buildable set, it's gem_name should be registered to the "gem_names" field
     if gem_path not in buildable_gems:
-        ret_val = project_properties.edit_project_props(project_path, new_gem_names=gem_json_data['gem_name'])
+        ret_val = project_properties.edit_project_props(project_path, new_gem_names=gem_json_data['gem_name'],
+                                                        is_optional=optional)
 
     # add the gem if it is registered in either the project.json or engine.json
     ret_val = ret_val or cmake.add_gem_dependency(project_enabled_gem_file, gem_json_data['gem_name'])
@@ -112,9 +115,9 @@ def enable_gem_in_project(gem_name: str = None,
 
 
 def add_explicit_gem_activation_for_all_paths(gem_root_folders: list,
-                                 project_name: str = None,
-                                 project_path: pathlib.Path = None,
-                                 enabled_gem_file: pathlib.Path = None) -> int:
+                                              project_name: str = None,
+                                              project_path: pathlib.Path = None,
+                                              enabled_gem_file: pathlib.Path = None) -> int:
     """
     walks each gem root folder directory structure and adds explicit
     activation of the gems to the project
@@ -168,7 +171,8 @@ def _run_enable_gem_in_project(args: argparse) -> int:
                                      args.gem_path,
                                      args.project_name,
                                      args.project_path,
-                                     args.enabled_gem_file
+                                     args.enabled_gem_file,
+                                     args.optional
                                      )
 
 
@@ -176,7 +180,7 @@ def add_parser_args(parser):
     """
     add_parser_args is called to add arguments to each command such that it can be
     invoked locally or added by a central python file.
-    Ex. Directly run from this file alone with: python enable_gem.py --project-path "D:/TestProject" --gem-path "D:/TestGem"
+    Ex. Directly run from this file with: python enable_gem.py --project-path "D:/TestProject" --gem-path "D:/TestGem"
     :param parser: the caller passes an argparse parser like instance to this method
     """
 
@@ -196,8 +200,10 @@ def add_parser_args(parser):
     group.add_argument('-agp', '--all-gem-paths', type=pathlib.Path, nargs='*', required=False,
                        help='Explicitly activates all gems in the path recursively.')
     parser.add_argument('-egf', '--enabled-gem-file', type=pathlib.Path, required=False,
-                                   help='The cmake enabled_gem file in which the gem names are specified.'
-                                        'If not specified it will assume enabled_gems.cmake')
+                        help='The cmake enabled_gem file in which the gem names are specified.'
+                        'If not specified it will assume enabled_gems.cmake')
+    parser.add_argument('-o', '--optional', action='store_true', required=False, default=False,
+                        help='Marks the gem as optional so a project can still be configured if not found.')
 
     parser.set_defaults(func=_run_enable_gem_in_project)
 

scripts/o3de/o3de/engine_properties.py

@@ -18,25 +18,43 @@ from o3de import manifest, utils
 logger = logging.getLogger('o3de.engine_properties')
 logging.basicConfig(format=utils.LOG_FORMAT)
 
+
 def _edit_gem_names(engine_json: dict,
                     new_gem_names: str or list = None,
                     delete_gem_names: str or list = None,
-                    replace_gem_names: str or list = None):
+                    replace_gem_names: str or list = None,
+                    is_optional: bool = False):
+    """
+    Edits and modifies the 'gem_names' list in the engine_json parameter.
+    :param engine_json: The engine json data
+    :param new_gem_names: Any gem names to be added to the list
+    :param delete_gem_names: Any gem names to be removed from the list
+    :param replace_gem_names: A list of gem names that will completely replace the current list
+    :param is_optional: Only applies to new_gem_names, when true will add an 'optional' property to the gem(s)
+    """
     if new_gem_names:
-        tag_list = new_gem_names.split() if isinstance(new_gem_names, str) else new_gem_names
-        engine_json.setdefault('gem_names', []).extend(tag_list)
+        add_list = new_gem_names.split() if isinstance(new_gem_names, str) else new_gem_names
+        if is_optional:
+            add_list = [dict(name=gem_name, optional=True) for gem_name in add_list]
+        engine_json.setdefault('gem_names', []).extend(add_list)
+
     if delete_gem_names:
         removal_list = delete_gem_names.split() if isinstance(delete_gem_names, str) else delete_gem_names
         if 'gem_names' in engine_json:
-            for tag in removal_list:
-                if tag in engine_json['gem_names']:
-                    engine_json['gem_names'].remove(tag)
+            def in_list(gem: str or dict, remove_list: list) -> bool:
+                if isinstance(gem, dict):
+                    return gem.get('name', '') in remove_list
+                else:
+                    return gem in remove_list
+
+            engine_json['gem_names'] = [gem for gem in engine_json['gem_names'] if not in_list(gem, removal_list)]
+
     if replace_gem_names:
         tag_list = replace_gem_names.split() if isinstance(replace_gem_names, str) else replace_gem_names
         engine_json['gem_names'] = tag_list
 
     # Remove duplicates from list
-    engine_json['gem_names'] = list(dict.fromkeys(engine_json.get('gem_names', [])))
+    engine_json['gem_names'] = utils.remove_gem_duplicates(engine_json.get('gem_names', []))
 
 
 def edit_engine_props(engine_path: pathlib.Path = None,
@@ -80,6 +98,7 @@ def edit_engine_props(engine_path: pathlib.Path = None,
 
     return 0 if manifest.save_o3de_manifest(engine_json_data, pathlib.Path(engine_path) / 'engine.json') else 1
 
+
 def _edit_engine_props(args: argparse) -> int:
     return edit_engine_props(args.engine_path,
                              args.engine_name,
@@ -90,6 +109,7 @@ def _edit_engine_props(args: argparse) -> int:
                              args.replace_gem_names
                              )
 
+
 def add_parser_args(parser):
     group = parser.add_mutually_exclusive_group(required=True)
     group.add_argument('-ep', '--engine-path', type=pathlib.Path, required=False,
@@ -103,13 +123,14 @@ def add_parser_args(parser):
                        help='Sets the version for the engine.')
     group = parser.add_mutually_exclusive_group(required=False)
     group.add_argument('-agn', '--add-gem-names', type=str, nargs='*', required=False,
-                       help='Adds gem name(s) to gem_names field. Space delimited list (ex. -at A B C)')
+                       help='Adds gem name(s) to gem_names field. Space delimited list (ex. -agn A B C)')
     group.add_argument('-dgn', '--delete-gem-names', type=str, nargs='*', required=False,
-                       help='Removes gem name(s) from the gem_names field. Space delimited list (ex. -dt A B C')
+                       help='Removes gem name(s) from the gem_names field. Space delimited list (ex. -dgn A B C')
     group.add_argument('-rgn', '--replace-gem-names', type=str, nargs='*', required=False,
                        help='Replace entirety of gem_names field with space delimited list of values')
     parser.set_defaults(func=_edit_engine_props)
 
+
 def add_args(subparsers) -> None:
     enable_engine_props_subparser = subparsers.add_parser('edit-engine-properties')
     add_parser_args(enable_engine_props_subparser)
@@ -122,5 +143,6 @@ def main():
     ret = the_args.func(the_args) if hasattr(the_args, 'func') else 1
     sys.exit(ret)
 
+
 if __name__ == "__main__":
     main()

scripts/o3de/o3de/project_manager_interface.py

@@ -194,7 +194,7 @@ def unregister_gem(gem_path: str, project_path: str = None,):
 
 
 def get_gem_info(gem_path: str) -> dict or None:
-        """
+    """
         Call get_gem_json_data
 
         :param gem_path: Gem path to gather info for

scripts/o3de/o3de/project_properties.py

@@ -31,22 +31,39 @@ def get_project_props(name: str = None, path: pathlib.Path = None) -> dict:
 def _edit_gem_names(proj_json: dict,
                     new_gem_names: str or list = None,
                     delete_gem_names: str or list = None,
-                    replace_gem_names: str or list = None):
+                    replace_gem_names: str or list = None,
+                    is_optional: bool = False):
+    """
+    Edits and modifies the 'gem_names' list in the proj_json parameter.
+    :param proj_json: The project json data
+    :param new_gem_names: Any gem names to be added to the list
+    :param delete_gem_names: Any gem names to be removed from the list
+    :param replace_gem_names: A list of gem names that will completely replace the current list
+    :param is_optional: Only applies to new_gem_names, when true will add an 'optional' property to the gem(s)
+    """
     if new_gem_names:
-        tag_list = new_gem_names.split() if isinstance(new_gem_names, str) else new_gem_names
-        proj_json.setdefault('gem_names', []).extend(tag_list)
+        add_list = new_gem_names.split() if isinstance(new_gem_names, str) else new_gem_names
+        if is_optional:
+            add_list = [dict(name=gem_name, optional=True) for gem_name in add_list]
+        proj_json.setdefault('gem_names', []).extend(add_list)
+
     if delete_gem_names:
         removal_list = delete_gem_names.split() if isinstance(delete_gem_names, str) else delete_gem_names
         if 'gem_names' in proj_json:
-            for tag in removal_list:
-                if tag in proj_json['gem_names']:
-                    proj_json['gem_names'].remove(tag)
+            def in_list(gem: str or dict, remove_list: list) -> bool:
+                if isinstance(gem, dict):
+                    return gem.get('name', '') in remove_list
+                else:
+                    return gem in remove_list
+
+            proj_json['gem_names'] = [gem for gem in proj_json['gem_names'] if not in_list(gem, removal_list)]
+
     if replace_gem_names:
         tag_list = replace_gem_names.split() if isinstance(replace_gem_names, str) else replace_gem_names
         proj_json['gem_names'] = tag_list
 
     # Remove duplicates from list
-    proj_json['gem_names'] = list(dict.fromkeys(proj_json.get('gem_names', [])))
+    proj_json['gem_names'] = utils.remove_gem_duplicates(proj_json.get('gem_names', []))
 
 
 def edit_project_props(proj_path: pathlib.Path = None,
@@ -63,10 +80,11 @@ def edit_project_props(proj_path: pathlib.Path = None,
                        new_gem_names: str or list = None,
                        delete_gem_names: str or list = None,
                        replace_gem_names: str or list = None,
-                       new_engine_name: str or list = None
+                       new_engine_name: str or list = None,
+                       is_optional: bool = False
                        ) -> int:
     proj_json = get_project_props(proj_name, proj_path)
-    
+
     if not proj_json:
         return 1
     if new_name:
@@ -103,7 +121,7 @@ def edit_project_props(proj_path: pathlib.Path = None,
         tag_list = replace_tags.split() if isinstance(replace_tags, str) else replace_tags
         proj_json['user_tags'] = tag_list
     # Update the gem_names field in the project.json
-    _edit_gem_names(proj_json, new_gem_names, delete_gem_names, replace_gem_names)
+    _edit_gem_names(proj_json, new_gem_names, delete_gem_names, replace_gem_names, is_optional)
 
     return 0 if manifest.save_o3de_manifest(proj_json, pathlib.Path(proj_path) / 'project.json') else 1
 
@@ -150,15 +168,15 @@ def add_parser_args(parser):
     group = parser.add_mutually_exclusive_group(required=False)
     group.add_argument('-at', '--add-tags', type=str, nargs='*', required=False,
                        help='Adds tag(s) to user_tags property. Space delimited list (ex. -at A B C)')
-    group.add_argument('-dt', '--delete-tags', type=str, nargs ='*', required=False,
+    group.add_argument('-dt', '--delete-tags', type=str, nargs='*', required=False,
                        help='Removes tag(s) from the user_tags property. Space delimited list (ex. -dt A B C')
-    group.add_argument('-rt', '--replace-tags', type=str, nargs ='*', required=False,
+    group.add_argument('-rt', '--replace-tags', type=str, nargs='*', required=False,
                        help='Replace entirety of user_tags property with space delimited list of values')
     group = parser.add_mutually_exclusive_group(required=False)
     group.add_argument('-agn', '--add-gem-names', type=str, nargs='*', required=False,
-                       help='Adds gem name(s) to gem_names field. Space delimited list (ex. -at A B C)')
+                       help='Adds gem name(s) to gem_names field. Space delimited list (ex. -agn A B C)')
     group.add_argument('-dgn', '--delete-gem-names', type=str, nargs='*', required=False,
-                       help='Removes gem name(s) from the gem_names field. Space delimited list (ex. -dt A B C')
+                       help='Removes gem name(s) from the gem_names field. Space delimited list (ex. -dgn A B C')
     group.add_argument('-rgn', '--replace-gem-names', type=str, nargs='*', required=False,
                        help='Replace entirety of gem_names field with space delimited list of values')
     parser.set_defaults(func=_edit_project_props)

scripts/o3de/o3de/utils.py

@@ -57,6 +57,7 @@ class VerbosityAction(argparse.Action):
         elif count == 1:
             log.setLevel(logging.INFO)
 
+
 def add_verbosity_arg(parser: argparse.ArgumentParser) -> None:
     """
     Add a consistent/common verbosity option to an arg parser
@@ -163,6 +164,7 @@ def backup_folder(folder: str or pathlib.Path) -> None:
             if backup_folder_name.is_dir():
                 renamed = True
 
+
 def get_git_provider(parsed_uri):
     """
     Returns a git provider if one exists given the passed uri
@@ -171,6 +173,7 @@ def get_git_provider(parsed_uri):
     """
     return github_utils.get_github_provider(parsed_uri)
 
+
 def download_file(parsed_uri, download_path: pathlib.Path, force_overwrite: bool = False, object_name: str = "", download_progress_callback = None) -> int:
     """
     Download file
@@ -255,6 +258,7 @@ def download_file(parsed_uri, download_path: pathlib.Path, force_overwrite: bool
 
     return 0
 
+
 def download_zip_file(parsed_uri, download_zip_path: pathlib.Path, force_overwrite: bool, object_name: str, download_progress_callback = None) -> int:
     """
     :param parsed_uri: uniform resource identifier to zip file to download
@@ -319,3 +323,26 @@ def find_ancestor_dir_containing_file(target_file_name: pathlib.PurePath, start_
     """
     ancestor_file = find_ancestor_file(target_file_name, start_path, max_scan_up_range)
     return ancestor_file.parent if ancestor_file else None
+
+
+def remove_gem_duplicates(gems: list) -> list:
+    """
+    For working with the 'gem_names' lists in project.json
+    Adds names to a dict, and when a collision occurs, eject the existing one in favor of the new one.
+    This is because when adding gems the list is extended, so the override will come last.
+    :param gems: The original list of gems, strings or small dicts (json objects)
+    :return: A new list with duplicate gem entries removed
+    """
+    new_list = []
+    names = {}
+    for gem in gems:
+        if not (isinstance(gem, dict) or isinstance(gem, str)):
+            continue
+        gem_name = gem.get('name', '') if isinstance(gem, dict) else gem
+        if gem_name:
+            if gem_name not in names:
+                names[gem_name] = len(new_list)
+                new_list.append(gem)
+            else:
+                new_list[names[gem_name]] = gem
+    return new_list

scripts/o3de/tests/test_project_manager_interface.py

@@ -139,7 +139,7 @@ def test_get_template_json_data():
     assert template_path.annotation == str or template_path.annotation == pathlib.Path
     project_path = parameters[2]
     assert project_path.name == 'project_path'
-    assert project_path.annotation == str or project_path.annotation == pathlib.Path 
+    assert project_path.annotation == str or project_path.annotation == pathlib.Path
 
     assert sig.return_annotation == dict
 
@@ -194,7 +194,7 @@ def test_edit_engine_props():
 # register interface
 def test_register():
     sig = signature(register.register)
-    assert len(sig.parameters) >= 17
+    assert len(sig.parameters) >= 18
 
     parameters = list(sig.parameters.values())
     engine_path = parameters[0]
@@ -224,10 +224,19 @@ def test_register():
     default_third_party_folder = parameters[12]
     assert default_third_party_folder.name == 'default_third_party_folder'
     assert default_third_party_folder.annotation == pathlib.Path
-    remove = parameters[15]
+    external_subdir_engine_path = parameters[13]
+    assert external_subdir_engine_path.name == 'external_subdir_engine_path'
+    assert external_subdir_engine_path.annotation == pathlib.Path
+    external_subdir_project_path = parameters[14]
+    assert external_subdir_project_path.name == 'external_subdir_project_path'
+    assert external_subdir_project_path.annotation == pathlib.Path
+    external_subdir_gem_path = parameters[15]
+    assert external_subdir_gem_path.name == 'external_subdir_gem_path'
+    assert external_subdir_gem_path.annotation == pathlib.Path
+    remove = parameters[16]
     assert remove.name == 'remove'
     assert remove.annotation == bool
-    force = parameters[16]
+    force = parameters[17]
     assert force.name == 'force'
     assert force.annotation == bool
 

scripts/o3de/tests/test_utils.py

@@ -10,6 +10,7 @@ import pytest
 
 from o3de import utils
 
+
 @pytest.mark.parametrize(
     "value, expected_result", [
         pytest.param('Game1', True),
@@ -24,6 +25,7 @@ def test_validate_identifier(value, expected_result):
     result = utils.validate_identifier(value)
     assert result == expected_result
 
+
 @pytest.mark.parametrize(
     "value, expected_result", [
         pytest.param('{018427ae-cd08-4ff1-ad3b-9b95256c17ca}', False),
@@ -37,3 +39,18 @@ def test_validate_identifier(value, expected_result):
 def test_validate_uuid4(value, expected_result):
     result = utils.validate_uuid4(value)
     assert result == expected_result
+
+
[email protected](
+    "in_list, out_list", [
+        pytest.param(['A', 'B', 'C'], ['A', 'B', 'C']),
+        pytest.param(['A', 'B', 'C', 'A', 'C'], ['A', 'B', 'C']),
+        pytest.param(['A', {'name': 'A', 'optional': True}], [{'name': 'A', 'optional': True}]),
+        pytest.param([{'name': 'A', 'optional': True}, 'A'], ['A']),
+        pytest.param([{'name': 'A'}], [{'name': 'A'}]),
+        pytest.param([{'optional': False}], []),
+    ]
+)
+def test_remove_gem_duplicates(in_list, out_list):
+    result = utils.remove_gem_duplicates(in_list)
+    assert result == out_list