Ver Fonte

Fix missing restricted objects being an error (#19040)

* Removes errors and makes `restricted` objects optional

This change does the following:

1. Makes it so that when a gem, project, or other object
   is instantiated, from a template, it only creates and
   registers the "restricted" object if there are actually
   restricted files within the template.  If the template
   contains no such files, it does not make or register the
   (empty) restricted object.

2. Enables "info" level logging for the o3de.py scripts that
   the user is likely to interact with so that it actually
   outputs something useful to the user.  Previously, most
   commands resulted in no output whatsoever.

3. Makes it so that the cmake scripts skip over any restricted
   files listed in the manifest, if they do not exist.  Previously,
   if a restricted file was listed in the manifest, but did not
   exist, it would count as a CMAKE error, and would abort configure,
   meaning, if you ever erased a restricted file, even for a
   completely different unrelated project that you were not trying
   to compile, you could no longer build any o3de project.

Signed-off-by: Nicholas Lawson <[email protected]>

* Fix removal of line I did not intend to remove

Signed-off-by: Nicholas Lawson <[email protected]>

---------

Signed-off-by: Nicholas Lawson <[email protected]>
Nicholas Lawson há 1 mês atrás
pai
commit
42f06dd28a

+ 7 - 0
cmake/PAL.cmake

@@ -203,6 +203,13 @@ function(o3de_find_restricted_folder restricted_name restricted_path)
     # Iterate over the restricted directories from the manifest file
     foreach(restricted_entry ${restricted_entries})
         set(restricted_json_file ${restricted_entry}/restricted.json)
+        if (NOT EXISTS ${restricted_json_file})
+            message(STATUS "Restricted file '${restricted_entry}' is listed in the o3de manifest, but does not exist.\n"
+                         "      If this is a left-over from an old project, consider removing it from the manifest by using\n"
+                         "      the o3de command line tool (excute this from a CLI inside the scripts folder in the engine)\n"
+                         "        o3de register --remove -rp \"${restricted_entry}\"")
+            continue()
+        endif()
         ly_file_read(${restricted_json_file} restricted_json)
         string(JSON this_restricted_name ERROR_VARIABLE json_error GET ${restricted_json} "restricted_name")
         if(json_error)

+ 1 - 0
scripts/o3de.py

@@ -12,6 +12,7 @@ import pathlib
 import sys
 
 logger = logging.getLogger('o3de')
+logger.setLevel(logging.INFO)
 
 def add_args(parser: argparse.ArgumentParser) -> None:
     """

+ 1 - 0
scripts/o3de/o3de/disable_gem.py

@@ -19,6 +19,7 @@ from o3de import cmake, manifest, project_properties, utils
 
 logger = logging.getLogger('o3de.disable_gem')
 logging.basicConfig(format=utils.LOG_FORMAT)
+logger.setLevel(logging.INFO)
 
 
 def disable_gem_in_project(gem_name: str = None,

+ 1 - 0
scripts/o3de/o3de/download.py

@@ -27,6 +27,7 @@ from o3de import manifest, repo, utils, register
 
 logger = logging.getLogger('o3de.download')
 logging.basicConfig(format=utils.LOG_FORMAT)
+logger.setLevel(logging.INFO)
 
 
 def unzip_manifest_json_data(download_zip_path: pathlib.Path, zip_file_name: str) -> dict:

+ 138 - 127
scripts/o3de/o3de/engine_template.py

@@ -23,6 +23,7 @@ from o3de import manifest, register, validation, utils
 
 logger = logging.getLogger('o3de.engine_template')
 logging.basicConfig(format=utils.LOG_FORMAT)
+logger.setLevel(logging.INFO)
 
 binary_file_ext = {
     '.pak',
@@ -311,18 +312,11 @@ def _execute_restricted_template_json(template_json_data: dict,
                                       keep_restricted_in_instance: bool = False,
                                       keep_license_text: bool = False) -> None:
 
-    # if we are not keeping restricted in instance make restricted.json if not present
-    if not keep_restricted_in_instance:
-        restricted_json = destination_restricted_path / 'restricted.json'
-        os.makedirs(os.path.dirname(restricted_json), exist_ok=True)
-        if not os.path.isfile(restricted_json):
-            with open(restricted_json, 'w') as s:
-                restricted_json_data = {}
-                restricted_json_data.update({"restricted_name": destination_name})
-                s.write(json.dumps(restricted_json_data, indent=4) + '\n')
-
+    
     ###################################################################################
     # for each createDirectories in the template copy any entries in the json_data that are for this platform
+
+    had_any_restricted_files = False
     for create_directory in template_json_data['createDirectories']:
         new_dir = pathlib.Path(create_directory['dir'])
         if not keep_restricted_in_instance and 'Platform' in new_dir.parts:
@@ -357,6 +351,7 @@ def _execute_restricted_template_json(template_json_data: dict,
                     copy_files = []
                     if 'copyFiles' in json_data.keys():
                         copy_files = json_data['copyFiles']
+                        had_any_restricted_files = True
                     copy_files.append(copy_file)
                     json_data.update({'copyFiles': copy_files})
 
@@ -365,6 +360,21 @@ def _execute_restricted_template_json(template_json_data: dict,
     # every entry is saved in its combined location, so if not keep_restricted_in_instance
     # then we need to palify into the restricted folder
 
+    # if there are no restricted createdirs, and no restricted copyfiles, we should early out
+    # and prevent creating any empty restricted folders
+    if not keep_restricted_in_instance and not had_any_restricted_files:
+        return
+    
+    # Create the restricted.json file in the destination_restricted_path
+    if not keep_restricted_in_instance:
+        restricted_json = destination_restricted_path / 'restricted.json'
+        os.makedirs(os.path.dirname(restricted_json), exist_ok=True)
+        if not os.path.isfile(restricted_json):
+            with open(restricted_json, 'w') as s:
+                restricted_json_data = {}
+                restricted_json_data.update({"restricted_name": destination_name})
+                s.write(json.dumps(restricted_json_data, indent=4) + '\n')
+    
     # create dirs first
     # for each createDirectory entry, transform the folder name
     if 'createDirectories' in json_data:
@@ -450,6 +460,7 @@ def _instantiate_template(template_json_data: dict,
     """
     # execute the template json
     # this will filter out any restricted platforms in the template
+    logger.info(f'Instantiating template: `{template_name}` into `{destination_path}`...')
     _execute_template_json(template_json_data,
                            destination_path,
                            template_path,
@@ -1488,23 +1499,28 @@ def create_from_template(destination_path: pathlib.Path,
     # restricted folder, so make sure the restricted has this destinations name
     # Note there is no linking of non o3de objects to o3de restricted. So this will make no attempt to figure out
     # if this destination was actually an o3de object and try to alter the <type>.json
+
+    # also note, if there were no restricted objects in the template, it is also not going to make a restricted
+    # object for this template.
+
     if not keep_restricted_in_instance:
         if destination_restricted_path:
-            os.makedirs(destination_restricted_path, exist_ok=True)
-
-            # write the restricted_name to the destination restricted.json
-            restricted_json = destination_restricted_path / 'restricted.json'
-            if not os.path.isfile(restricted_json):
-                with open(restricted_json, 'w') as s:
-                    restricted_json_data = {}
-                    restricted_json_data.update({'restricted_name': destination_name})
-                    s.write(json.dumps(restricted_json_data, indent=4) + '\n')
-
-            # Register the restricted
-            if not no_register:
-                if register.register(restricted_path=destination_restricted_path):
-                    logger.error(f'Failed to register the restricted {destination_restricted_path}.')
-                    return 1
+            # Only generate the restricted.json if there were any objects in the template
+            # if this was the case, they would have already been copied in there.
+            if  os.path.isdir(destination_restricted_path):
+                # write the restricted_name to the destination restricted.json
+                restricted_json = destination_restricted_path / 'restricted.json'
+                if not os.path.isfile(restricted_json):
+                    with open(restricted_json, 'w') as s:
+                        restricted_json_data = {}
+                        restricted_json_data.update({'restricted_name': destination_name})
+                        s.write(json.dumps(restricted_json_data, indent=4) + '\n')
+
+                # Register the restricted
+                if not no_register:
+                    if register.register(restricted_path=destination_restricted_path):
+                        logger.error(f'Failed to register the restricted {destination_restricted_path}.')
+                        return 1
 
     logger.warning(f'Instantiation successful. NOTE: This is a generic instantiation of the template. If this'
                    f' was a template of an o3de object like a project, gem, template, etc. then the create-project'
@@ -1841,60 +1857,62 @@ def create_project(project_path: pathlib.Path,
     # restricted json and set that as this projects restricted
     if not keep_restricted_in_project:
         if project_restricted_path:
-            os.makedirs(project_restricted_path, exist_ok=True)
+            # we only want to do this if the project template had any restricted files in it
+            # if it didn't there is no point in creating a restricted object for it that is empty.
+            # the user can always add a restricted object to their project at any later time.
+            if  os.path.isdir(project_restricted_path):
+                # read the restricted_name from the projects restricted.json
+                restricted_json = project_restricted_path / 'restricted.json'
+                if os.path.isfile(restricted_json):
+                    if not validation.valid_o3de_restricted_json(restricted_json):
+                        logger.error(f'Restricted json {restricted_json} is not valid.')
+                        return 1
+                else:
+                    with open(restricted_json, 'w') as s:
+                        restricted_json_data = {}
+                        restricted_json_data.update({'restricted_name': project_name})
+                        s.write(json.dumps(restricted_json_data, indent=4) + '\n')
 
-            # read the restricted_name from the projects restricted.json
-            restricted_json = project_restricted_path / 'restricted.json'
-            if os.path.isfile(restricted_json):
-                if not validation.valid_o3de_restricted_json(restricted_json):
-                    logger.error(f'Restricted json {restricted_json} is not valid.')
-                    return 1
-            else:
-                with open(restricted_json, 'w') as s:
-                    restricted_json_data = {}
-                    restricted_json_data.update({'restricted_name': project_name})
-                    s.write(json.dumps(restricted_json_data, indent=4) + '\n')
+                with open(restricted_json, 'r') as s:
+                    try:
+                        restricted_json_data = json.load(s)
+                    except json.JSONDecodeError as e:
+                        logger.error(f'Failed to load restricted json {restricted_json}.')
+                        return 1
 
-            with open(restricted_json, 'r') as s:
                 try:
-                    restricted_json_data = json.load(s)
-                except json.JSONDecodeError as e:
-                    logger.error(f'Failed to load restricted json {restricted_json}.')
+                    restricted_name = restricted_json_data["restricted_name"]
+                except KeyError as e:
+                    logger.error(f'Failed to read "restricted_name" from restricted json {restricted_json}.')
                     return 1
 
-            try:
-                restricted_name = restricted_json_data["restricted_name"]
-            except KeyError as e:
-                logger.error(f'Failed to read "restricted_name" from restricted json {restricted_json}.')
-                return 1
-
-            # set the "restricted": <restricted_name> element of the project.json
-            project_json = project_path / 'project.json'
-            if not validation.valid_o3de_project_json(project_json):
-                logger.error(f'Project json {project_json} is not valid.')
-                return 1
-
-            with open(project_json, 'r') as s:
-                try:
-                    project_json_data = json.load(s)
-                except json.JSONDecodeError as e:
-                    logger.error(f'Failed to load project json {project_json}.')
+                # set the "restricted": <restricted_name> element of the project.json
+                project_json = project_path / 'project.json'
+                if not validation.valid_o3de_project_json(project_json):
+                    logger.error(f'Project json {project_json} is not valid.')
                     return 1
 
-            project_json_data.update({"restricted": restricted_name})
-            os.unlink(project_json)
-            with open(project_json, 'w') as s:
-                try:
-                    s.write(json.dumps(project_json_data, indent=4) + '\n')
-                except OSError as e:
-                    logger.error(f'Failed to write project json {project_json}.')
-                    return 1
+                with open(project_json, 'r') as s:
+                    try:
+                        project_json_data = json.load(s)
+                    except json.JSONDecodeError as e:
+                        logger.error(f'Failed to load project json {project_json}.')
+                        return 1
 
-            # Register the restricted
-            if not no_register:
-                if register.register(restricted_path=project_restricted_path):
-                    logger.error(f'Failed to register the restricted {project_restricted_path}.')
-                    return 1
+                project_json_data.update({"restricted": restricted_name})
+                os.unlink(project_json)
+                with open(project_json, 'w') as s:
+                    try:
+                        s.write(json.dumps(project_json_data, indent=4) + '\n')
+                    except OSError as e:
+                        logger.error(f'Failed to write project json {project_json}.')
+                        return 1
+
+                # Register the restricted
+                if not no_register:
+                    if register.register(restricted_path=project_restricted_path):
+                        logger.error(f'Failed to register the restricted {project_restricted_path}.')
+                        return 1
 
     print(f'Project created at {project_path}')
 
@@ -2280,69 +2298,62 @@ def create_gem(gem_path: pathlib.Path,
     # restricted json and set that as this gems restricted
     if not keep_restricted_in_gem:
         if gem_restricted_path:
-            os.makedirs(gem_restricted_path, exist_ok=True)
+            # note that we should only actually do this if gem_restricted_path already exists
+            # if it doesn't it means that there were no restricted platform files anywhere in the template
+            # and as such, it is not necessary to make an empty useless restricted object.
+            if os.path.isdir(gem_restricted_path): 
+                # read the restricted_name from the gems restricted.json
+                restricted_json = gem_restricted_path / 'restricted.json'
+                if os.path.isfile(restricted_json):
+                    if not validation.valid_o3de_restricted_json(restricted_json):
+                        logger.error(f'Restricted json {restricted_json} is not valid.')
+                        return 1
+                else:
+                    with open(restricted_json, 'w') as s:
+                        restricted_json_data = {}
+                        restricted_json_data.update({'restricted_name': gem_name})
+                        s.write(json.dumps(restricted_json_data, indent=4) + '\n')
 
-            # read the restricted_name from the gems restricted.json
-            restricted_json = gem_restricted_path / 'restricted.json'
-            if os.path.isfile(restricted_json):
-                if not validation.valid_o3de_restricted_json(restricted_json):
-                    logger.error(f'Restricted json {restricted_json} is not valid.')
-                    return 1
-            else:
-                with open(restricted_json, 'w') as s:
-                    restricted_json_data = {}
-                    restricted_json_data.update({'restricted_name': gem_name})
-                    s.write(json.dumps(restricted_json_data, indent=4) + '\n')
+                with open(restricted_json, 'r') as s:
+                    try:
+                        restricted_json_data = json.load(s)
+                    except json.JSONDecodeError as e:
+                        logger.error(f'Failed to load restricted json {restricted_json}.')
+                        return 1
 
-            with open(restricted_json, 'r') as s:
                 try:
-                    restricted_json_data = json.load(s)
-                except json.JSONDecodeError as e:
-                    logger.error(f'Failed to load restricted json {restricted_json}.')
+                    restricted_name = restricted_json_data["restricted_name"]
+                except KeyError as e:
+                    logger.error(f'Failed to read "restricted_name" from restricted json {restricted_json}.')
                     return 1
 
-            try:
-                restricted_name = restricted_json_data["restricted_name"]
-            except KeyError as e:
-                logger.error(f'Failed to read "restricted_name" from restricted json {restricted_json}.')
-                return 1
-
-            # set the "restricted_name": <restricted_name> element of the gem.json
-            gem_json = gem_path / 'gem.json'
-            if not validation.valid_o3de_gem_json(gem_json):
-                logger.error(f'Gem json {gem_json} is not valid.')
-                return 1
-
-            with open(gem_json, 'r') as s:
-                try:
-                    gem_json_data = json.load(s)
-                except json.JSONDecodeError as e:
-                    logger.error(f'Failed to load gem json {gem_json}.')
+                # set the "restricted_name": <restricted_name> element of the gem.json
+                gem_json = gem_path / 'gem.json'
+                if not validation.valid_o3de_gem_json(gem_json):
+                    logger.error(f'Gem json {gem_json} is not valid.')
                     return 1
 
-            gem_json_data.update({"restricted": restricted_name})
-            os.unlink(gem_json)
-            with open(gem_json, 'w') as s:
-                try:
-                    s.write(json.dumps(gem_json_data, indent=4) + '\n')
-                except OSError as e:
-                    logger.error(f'Failed to write project json {gem_json}.')
-                    return 1
-            '''
-            for restricted_platform in restricted_platforms:
-                restricted_gem = gem_restricted_path / restricted_platform / gem_name
-                os.makedirs(restricted_gem, exist_ok=True)
-                cmakelists_file_name = restricted_gem / 'CMakeLists.txt'
-                if not os.path.isfile(cmakelists_file_name):
-                    with open(cmakelists_file_name, 'w') as d:
-                        if keep_license_text:
-                            d.write(O3DE_LICENSE_TEXT)
-            '''
-            # Register the restricted
-            if not no_register:
-                if register.register(restricted_path=gem_restricted_path):
-                    logger.error(f'Failed to register the restricted {gem_restricted_path}.')
-                    return 1
+                with open(gem_json, 'r') as s:
+                    try:
+                        gem_json_data = json.load(s)
+                    except json.JSONDecodeError as e:
+                        logger.error(f'Failed to load gem json {gem_json}.')
+                        return 1
+
+                gem_json_data.update({"restricted": restricted_name})
+                os.unlink(gem_json)
+                with open(gem_json, 'w') as s:
+                    try:
+                        s.write(json.dumps(gem_json_data, indent=4) + '\n')
+                    except OSError as e:
+                        logger.error(f'Failed to write project json {gem_json}.')
+                        return 1
+                
+                # Register the restricted
+                if not no_register:
+                    if register.register(restricted_path=gem_restricted_path):
+                        logger.error(f'Failed to register the restricted {gem_restricted_path}.')
+                        return 1
 
     print(f'Gem created at {gem_path}')
 

+ 1 - 0
scripts/o3de/o3de/global_project.py

@@ -18,6 +18,7 @@ from o3de import manifest, validation, utils
 
 logger = logging.getLogger('o3de.global_project')
 logging.basicConfig(format=utils.LOG_FORMAT)
+logger.setLevel(logging.INFO)
 
 DEFAULT_BOOTSTRAP_SETREG = pathlib.Path('~/.o3de/Registry/bootstrap.setreg').expanduser()
 PROJECT_PATH_KEY = ('Amazon', 'AzCore', 'Bootstrap', 'project_path')

+ 10 - 0
scripts/o3de/o3de/register.py

@@ -291,6 +291,12 @@ def register_o3de_object_path(json_data: dict,
                               dry_run: bool = False) -> int:
     # save_path variable is used to save the changes to the store the path to the file to save
     # if the registration is for the project or engine
+
+    if remove:
+        logger.info(f'Removing `{o3de_object_path}` from key `{o3de_object_key}` in the manifest.')
+    else:
+        logger.info(f'Adding `{o3de_object_path}` to key `{o3de_object_key}` in the manifest.')
+
     save_path = None
 
     if not o3de_object_path:
@@ -926,6 +932,10 @@ def register(engine_path: pathlib.Path = None,
 
 
 def _run_register(args: argparse) -> int:
+    # at least inform the user where the file is so that they can poke at it if something goes wrong.
+    manifest_file = manifest.get_o3de_manifest()
+    logger.info(f"O3DE Manifest file location: `{manifest_file}`")
+    
     if args.update:
         remove_invalid_o3de_objects()
         return repo.refresh_repos()

+ 1 - 0
scripts/o3de/o3de/repo.py

@@ -18,6 +18,7 @@ from o3de import manifest, utils, validation
 
 logger = logging.getLogger('o3de.repo')
 logging.basicConfig(format=utils.LOG_FORMAT)
+logger.setLevel(logging.INFO)
 
 REPO_IMPLICIT_SCHEMA_VERSION = "0.0.0"
 

+ 1 - 0
scripts/o3de/o3de/utils.py

@@ -33,6 +33,7 @@ UI_MSG_FORMAT = '%(message)s'
 
 logger = logging.getLogger('o3de.utils')
 logging.basicConfig(format=LOG_FORMAT)
+logger.setLevel(logging.INFO)
 
 COPY_BUFSIZE = 64 * 1024