Added new feature to the register command to auto detect manifest file based on input path (#2967)

* Added new feature to the register command to auto detect the correct o3de manifest file to write to based on the supplied registration path Updated the create-gem command to register the gem on creation. Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Added new feature to the register command to auto detect the correct o3de manifest file to write to based on the supplied registration path Updated the create-gem command to register the gem on creation. Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fix incorrect variable reference in register_project_path Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed o3de python package unit test The enable_gem, register and engine template test have been updated to account for the logic to register a gem after creation Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Added a --no-register arg to engine-template.py This allows registration of gems/projects using the create-project and create-gem commands to be skipped Prevented registration of a gems and projects in the engine_template command test Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Wrapped first parameter to find_ancestor_dir_containing_file with pathlib.PurePath Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>
2021-08-09 16:58:19 -05:00
parent c1a0b5c686
commit 9afbd07a88
6 changed files with 233 additions and 10 deletions
@@ -1299,6 +1299,7 @@ def create_project(project_path: pathlib.Path,
                   keep_license_text: bool = False,
                   replace: list = None,
                   force: bool = False,
+                   no_register: bool = False,
                   system_component_class_id: str = None,
                   editor_system_component_class_id: str = None,
                   module_id: str = None) -> int:
@@ -1657,9 +1658,10 @@ def create_project(project_path: pathlib.Path,
                            d.write('# {END_LICENSE}\n')


-    # Register the project with the global o3de_manifest.json and set the project.json "engine" field to match the
+    # Register the project with the either o3de_manifest.json or engine.json
+    # and set the project.json "engine" field to match the
    # engine.json "engine_name" field
-    return register.register(project_path=project_path)
+    return register.register(project_path=project_path) if not no_register else 0


 def create_gem(gem_path: pathlib.Path,
@@ -1676,6 +1678,7 @@ def create_gem(gem_path: pathlib.Path,
               keep_license_text: bool = False,
               replace: list = None,
               force: bool = False,
+               no_register: bool = False,
               system_component_class_id: str = None,
               editor_system_component_class_id: str = None,
               module_id: str = None) -> int:
@@ -2035,7 +2038,8 @@ def create_gem(gem_path: pathlib.Path,
                                d.write('#\n')
                                d.write('# SPDX-License-Identifier: Apache-2.0 OR MIT\n')
                                d.write('# {END_LICENSE}\n')
-    return 0
+    # Register the gem with the either o3de_manifest.json, engine.json or project.json based on the gem path
+    return register.register(gem_path=gem_path) if not no_register else 0


 def _run_create_template(args: argparse) -> int:
@@ -2086,6 +2090,7 @@ def _run_create_project(args: argparse) -> int:
                          args.keep_license_text,
                          args.replace,
                          args.force,
+                          args.no_register,
                          args.system_component_class_id,
                          args.editor_system_component_class_id,
                          args.module_id)
@@ -2106,6 +2111,7 @@ def _run_create_gem(args: argparse) -> int:
                      args.keep_license_text,
                      args.replace,
                      args.force,
+                      args.no_register,
                      args.system_component_class_id,
                      args.editor_system_component_class_id,
                      args.module_id)
@@ -2370,6 +2376,9 @@ def add_args(subparsers) -> None:
                                               ' uuid Ex. {b60c92eb-3139-454b-a917-a9d3c5819594}')
    create_project_subparser.add_argument('-f', '--force', action='store_true', default=False,
                                      help='Copies over instantiated template directory even if it exist.')
+    create_project_subparser.add_argument('--no-register', action='store_true', default=False,
+                                      help='If the project template is instantiated successfully, it will not register the'
+                                           ' project with the global or engine manifest file.')
    create_project_subparser.set_defaults(func=_run_create_project)

    # creation of a gem from a template (like create from template but makes gem assumptions)
@@ -2463,6 +2472,9 @@ def add_args(subparsers) -> None:
                                           ' default is a random uuid Ex. {b60c92eb-3139-454b-a917-a9d3c5819594}')
    create_gem_subparser.add_argument('-f', '--force', action='store_true', default=False,
                        help='Copies over instantiated template directory even if it exist.')
+    create_gem_subparser.add_argument('--no-register', action='store_true', default=False,
+                                      help='If the gem template is instantiated successfully, it will not register the'
+                                           ' gem with the global, project or engine manifest file.')
    create_gem_subparser.set_defaults(func=_run_create_gem)


@@ -285,14 +285,14 @@ def register_o3de_object_path(json_data: dict,

    manifest_data = None
    if engine_path:
-        manifest_data = manifest.get_engine_json_data(None, engine_path)
+        manifest_data = manifest.get_engine_json_data(engine_path=engine_path)
        if not manifest_data:
            logger.error(f'Cannot load engine.json data at path {engine_path}')
            return 1

        save_path = engine_path / 'engine.json'
    elif project_path:
-        manifest_data = manifest.get_project_json_data(None, project_path)
+        manifest_data = manifest.get_project_json_data(project_path=project_path)
        if not manifest_data:
            logger.error(f'Cannot load project.json data at path {project_path}')
            return 1
@@ -367,6 +367,11 @@ def register_external_subdirectory(json_data: dict,
    :return An integer return code indicating whether registration or removal of the external subdirectory
    completed successfully
    """
+    # If a project path or engine path has not been supplied auto detect which manifest to register the input path with
+    if not project_path and not engine_path:
+        project_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('project.json'), external_subdir_path)
+        if not project_path:
+            engine_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('engine.json'), external_subdir_path)
    return register_o3de_object_path(json_data, external_subdir_path, 'external_subdirectories', '', None, remove,
                                     engine_path, project_path)

@@ -376,6 +381,11 @@ def register_gem_path(json_data: dict,
                      remove: bool = False,
                      engine_path: pathlib.Path = None,
                      project_path:  pathlib.Path = None) -> int:
+    # If a project path or engine path has not been supplied auto detect which manifest to register the input path with
+    if not project_path and not engine_path:
+        project_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('project.json'), gem_path)
+        if not project_path:
+            engine_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('engine.json'), gem_path)
    return register_o3de_object_path(json_data, gem_path, 'external_subdirectories', 'gem.json',
                                     validation.valid_o3de_gem_json, remove, engine_path, project_path)

@@ -384,6 +394,11 @@ def register_project_path(json_data: dict,
                          project_path: pathlib.Path,
                          remove: bool = False,
                          engine_path: pathlib.Path = None) -> int:
+    # If an engine path has not been supplied auto detect if the project should be register with the engine.json
+    # or the ~/.o3de/o3de_manifest.json
+    if not engine_path:
+        engine_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('engine.json'), project_path)
+
    result = register_o3de_object_path(json_data, project_path, 'projects', 'project.json',
                                     validation.valid_o3de_project_json, remove, engine_path, None)

@@ -419,6 +434,11 @@ def register_template_path(json_data: dict,
                           template_path: pathlib.Path,
                           remove: bool = False,
                           engine_path: pathlib.Path = None) -> int:
+    # If a project path or engine path has not been supplied auto detect which manifest to register the input path
+    if not project_path and not engine_path:
+        project_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('project.json'), template_path)
+        if not project_path:
+            engine_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('engine.json'), template_path)
    return register_o3de_object_path(json_data, template_path, 'templates', 'template.json',
                                       validation.valid_o3de_template_json, remove, engine_path, None)

@@ -427,6 +447,11 @@ def register_restricted_path(json_data: dict,
                             restricted_path: pathlib.Path,
                             remove: bool = False,
                             engine_path: pathlib.Path = None) -> int:
+    # If a project path or engine path has not been supplied auto detect which manifest to register the input path
+    if not project_path and not engine_path:
+        project_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('project.json'), restricted_path)
+        if not project_path:
+            engine_path = utils.find_ancestor_dir_containing_file(pathlib.PurePath('engine.json'), restricted_path)
    return register_o3de_object_path(json_data, restricted_path, 'restricted', 'restricted.json',
                                     validation.valid_o3de_restricted_json, remove, engine_path, None)

@@ -8,7 +8,7 @@
 """
 This file contains utility functions
 """
-
+import sys
 import uuid
 import pathlib
 import shutil
@@ -123,4 +123,51 @@ def download_zip_file(parsed_uri, download_zip_path: pathlib.Path) -> int:
        download_zip_path.unlink()
        return 1

-    return 0
+    return 0
+
+
+def find_ancestor_file(target_file_name: pathlib.PurePath, start_path: pathlib.Path,
+                       max_scan_up_range: int=0) -> pathlib.Path or None:
+    """
+    Find a file with the given name in the ancestor directories by walking up the starting path until the file is found.
+
+    :param target_file_name: Name of the file to find.
+    :param start_path: path to start looking for the file.
+    :param max_scan_up_range: maximum number of directories to scan upwards when searching for target file
+           if the value is 0, then there is no max
+    :return: Path to the file or None if not found.
+    """
+    current_path = pathlib.Path(start_path)
+    candidate_path = current_path / target_file_name
+
+    max_scan_up_range = max_scan_up_range if max_scan_up_range else sys.maxsize
+
+    # Limit the number of directories to traverse, to avoid infinite loop in path cycles
+    for _ in range(max_scan_up_range):
+        if candidate_path.exists():
+            # Found the file we wanted
+            break
+
+        parent_path = current_path.parent
+        if parent_path == current_path:
+            # Only true when we are at the directory root, can't keep searching
+            break
+        candidate_path = parent_path / target_file_name
+        current_path = parent_path
+
+    return candidate_path if candidate_path.exists() else None
+
+def find_ancestor_dir_containing_file(target_file_name: pathlib.PurePath, start_path: pathlib.Path,
+                                      max_scan_up_range: int=0) -> pathlib.Path or None:
+    """
+    Find nearest ancestor directory that contains the file with the given name by walking up
+    from the starting path.
+
+    :param target_file_name: Name of the file to find.
+    :param start_path: path to start looking for the file.
+    :param max_scan_up_range: maximum number of directories to scan upwards when searching for target file
+           if the value is 0, then there is no max
+    :return: Path to the directory containing file or None if not found.
+    """
+    ancestor_file = find_ancestor_file(target_file_name, start_path, max_scan_up_range)
+    return ancestor_file.parent if ancestor_file else None
@@ -124,7 +124,7 @@ class TestEnableGemCommand:
                return json.loads(TEST_O3DE_MANIFEST_JSON_PAYLOAD)
            return None

-        def get_project_json_data(json_data: pathlib.Path, project_path: pathlib.Path):
+        def get_project_json_data(project_path: pathlib.Path):
            return self.enable_gem.project_data

        def get_gem_json_data(gem_path: pathlib.Path, project_path: pathlib.Path):
@@ -263,6 +263,7 @@ class TestCreateTemplate:
            s.write(templated_contents)

        template_dest_path = engine_root / instantiated_name
+        # Skip registeration in test
        with patch('uuid.uuid4', return_value=uuid.uuid5(uuid.NAMESPACE_DNS, instantiated_name)) as uuid4_mock:
            result = create_from_template_func(template_dest_path, template_path=template_default_folder, force=True,
                                                          keep_license_text=keep_license_text, **create_from_template_kwargs)
@@ -345,7 +346,7 @@ class TestCreateTemplate:
        template_json_contents = json.dumps(template_json_dict, indent=4)
        self.instantiate_template_wrapper(tmpdir, engine_template.create_project, 'TestProject', concrete_contents,
                                          templated_contents, keep_license_text, force, expect_failure,
-                                          template_json_contents, template_file_map, project_name='TestProject')
+                                          template_json_contents, template_file_map, project_name='TestProject', no_register=True)


    @pytest.mark.parametrize(
@@ -379,6 +380,8 @@ class TestCreateTemplate:
                "isTemplated": True,
                "isOptional": False
            })
+        #Convert dict back to string
+        template_json_contents = json.dumps(template_json_dict, indent=4)
        self.instantiate_template_wrapper(tmpdir, engine_template.create_gem, 'TestGem', concrete_contents,
                                          templated_contents, keep_license_text, force, expect_failure,
-                                          template_json_contents, gem_name='TestGem')
+                                          template_json_contents, template_file_map, gem_name='TestGem', no_register=True)
@@ -112,3 +112,139 @@ class TestRegisterThisEngine:
            result = register._run_register(args)
            assert result == expected_result

+
+TEST_GEM_JSON_PAYLOAD = '''
+{
+    "gem_name": "TestGem",
+    "display_name": "TestGem",
+    "license": "What license TestGem uses goes here: i.e. https://opensource.org/licenses/MIT",
+    "origin": "The primary repo for TestGem goes here: i.e. http://www.mydomain.com",
+    "type": "Code",
+    "summary": "A short description of TestGem.",
+    "canonical_tags": [
+        "Gem"
+    ],
+    "user_tags": [
+        "TestGem"
+    ],
+    "icon_path": "preview.png",
+    "requirements": ""
+}
+'''
+
+TEST_PROJECT_JSON_PAYLOAD = '''
+{
+    "project_name": "TestProject",
+    "engine": "o3de",
+    "external_subdirectories": []
+}
+'''
+
+TEST_ENGINE_JSON_PAYLOAD = '''
+{
+    "engine_name": "o3de",
+    "external_subdirectories": [],
+    "projects": [],
+    "templates": []
+}
+'''
+
+TEST_O3DE_MANIFEST_JSON_PAYLOAD = '''
+{
+    "o3de_manifest_name": "testuser",
+    "origin": "C:/Users/testuser/.o3de",
+    "default_engines_folder": "C:/Users/testuser/.o3de/Engines",
+    "default_projects_folder": "C:/Users/testuser/.o3de/Projects",
+    "default_gems_folder": "C:/Users/testuser/.o3de/Gems",
+    "default_templates_folder": "C:/Users/testuser/.o3de/Templates",
+    "default_restricted_folder": "C:/Users/testuser/.o3de/Restricted",
+    "default_third_party_folder": "C:/Users/testuser/.o3de/3rdParty",
+    "projects": [],
+    "external_subdirectories": [],
+    "templates": [],
+    "restricted": [],
+    "repos": [],
+    "engines": [],
+    "engines_path": {}
+}
+'''
+@pytest.fixture(scope='class')
+def init_register_gem_data(request):
+    request.cls.o3de_manifest_data = json.loads(TEST_O3DE_MANIFEST_JSON_PAYLOAD)
+    request.cls.project_data = json.loads(TEST_PROJECT_JSON_PAYLOAD)
+    request.cls.engine_data = json.loads(TEST_ENGINE_JSON_PAYLOAD)
+
+
+@pytest.mark.usefixtures('init_register_gem_data')
+class TestRegisterGem:
+    engine_path = pathlib.PurePath('o3de')
+    project_path = pathlib.PurePath('TestProject')
+
+    @staticmethod
+    def get_gem_json_data(gem_path: pathlib.Path = None):
+        return json.loads(TEST_GEM_JSON_PAYLOAD)
+
+    @pytest.mark.parametrize("gem_path, expected_manifest_file, expected_result", [
+                                 pytest.param(pathlib.PurePath('TestGem'), pathlib.PurePath('o3de_manifest.json'), 0),
+                                 pytest.param(project_path / 'TestGem', pathlib.PurePath('project.json'), 0),
+                                 pytest.param(engine_path / 'TestGem', pathlib.PurePath('engine.json'), 0),
+                             ])
+    def test_register_gem_auto_detects_manifest_update(self, gem_path, expected_manifest_file,expected_result):
+
+        def save_o3de_manifest(manifest_data: dict, manifest_path: pathlib.Path = None) -> bool:
+            if manifest_path == TestRegisterGem.project_path / 'project.json':
+                self.project_data = manifest_data
+            elif manifest_path == TestRegisterGem.engine_path / 'engine.json':
+                self.engine_data = manifest_data
+            else:
+                self.o3de_manifest_data = manifest_data
+            return True
+
+        def load_o3de_manifest(manifest_path: pathlib.Path = None) -> dict:
+            if manifest_path == TestRegisterGem.project_path:
+                return self.project_data
+            elif manifest_path == TestRegisterGem.engine_path:
+                return self.engine_data
+            return self.o3de_manifest_data
+
+        def get_engine_json_data(engine_path: pathlib.Path = None):
+            return json.loads(TEST_ENGINE_JSON_PAYLOAD)
+
+        def get_project_json_data(project_path: pathlib.Path = None):
+            return json.loads(TEST_PROJECT_JSON_PAYLOAD)
+
+        def find_ancestor_dir(target_file_name: pathlib.PurePath, start_path: pathlib.Path):
+            try:
+                if target_file_name == pathlib.PurePath('project.json')\
+                        and start_path.relative_to(TestRegisterGem.project_path):
+                    return TestRegisterGem.project_path
+            except ValueError:
+                pass
+            try:
+                if target_file_name == pathlib.PurePath('engine.json')\
+                        and start_path.relative_to(TestRegisterGem.engine_path):
+                    return TestRegisterGem.engine_path
+            except ValueError:
+                pass
+            return None
+
+        with patch('o3de.manifest.load_o3de_manifest', side_effect=load_o3de_manifest) as _1,\
+                patch('o3de.manifest.save_o3de_manifest', side_effect=save_o3de_manifest) as _2,\
+                patch('o3de.manifest.get_engine_json_data', side_effect=get_engine_json_data) as _3,\
+                patch('o3de.manifest.get_project_json_data', side_effect=get_project_json_data) as _4,\
+                patch('o3de.manifest.get_gem_json_data', side_effect=TestRegisterGem.get_gem_json_data) as _5,\
+                patch('o3de.utils.find_ancestor_dir_containing_file', side_effect=find_ancestor_dir) as _6,\
+                patch('pathlib.Path.is_dir', return_value=True) as _7,\
+                patch('o3de.validation.valid_o3de_gem_json', return_value=True) as _8:
+            result = register.register(gem_path=gem_path)
+            assert result == expected_result
+
+            if expected_manifest_file == pathlib.PurePath('o3de_manifest.json'):
+                assert gem_path in map(lambda subdir: pathlib.PurePath(subdir),
+                                       self.o3de_manifest_data.get('external_subdirectories', []))
+            elif expected_manifest_file == pathlib.PurePath('project.json'):
+                assert gem_path in map(lambda subdir: pathlib.PurePath(TestRegisterGem.project_path) / subdir,
+                                       self.project_data.get('external_subdirectories', []))
+            elif expected_manifest_file == pathlib.PurePath('engine.json'):
+                assert gem_path in map(lambda subdir: pathlib.PurePath(TestRegisterGem.engine_path) / subdir,
+                                       self.engine_data.get('external_subdirectories', []))