From 2478c60793a09b170ea36d34b1f414e8069f945c Mon Sep 17 00:00:00 2001 From: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> Date: Fri, 2 Jul 2021 00:22:12 -0500 Subject: [PATCH] Fixed several o3de python package and install layout issues (#1714) * Updated the CrashLog directory path to save to the project user directory instead of the engine-root directory Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Removing the custom OUTPUT_NAME for the Multiplayer Gem Builder target Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Changed the default value for the LY_3RDPARTY_PATH cache variable to be ~/.o3de/3rdParty This simplifies the first time user experience when running cmake Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed Windows only issue where using creating a VS Solution for an O3DE project on a different drive resulted in an unloaded ":" entry appearing in the solution explorer Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Refactored install layout logic for External Subdirectories Instead of performing a regular expression over the Gems/* directory, now the each external subdirectory including the project (external) directory is recursied over and scanned for any folders that aren't excluded. By default those folders are the [Cc]ache, [Bb]uild and [Uu]ser directories Afterwards the list files to copy over are then split into a directory list and a file list that is filtered by an include regex Next the directory list is iterated over and the directories are copied to the install layout Finally the file list is iterated and the list of files are also copied to the install layout Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed the o3de.bat script changing the working directory before running the o3de.py script Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed exception in engine-template.py script when the create-gem command is invoked with the --template-name parameter that does not correspond to a registered Template Updated the create-project command to register the project with the o3de_manifest.json and the engine with the project as the final step Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed register.py over-registration of non-engine directories when then --this-engine parameter is supplied. All the projects, gems and templates inside of the default o3de_manifest folder locations were being registered with the engine that was being registered Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed register.py register_project_path() method to return 0 when the project path is successfully registered. This issue was that the save_o3de_manifest method "return" value was being checked and that method doesn't actually return a value Added question mark to the engine_template.py to correct text around notifying the user if the project was registered Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Added doc comment on the new cmke ly_get_vs_folder_directory function() Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Added clarifying comment to the ly_setup_others() command logic to copy over directories and files from any external subdirectories(Gems) that are registered with the engine at the time of install Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed infinite recursion when trying to create a template with the source directory used to seed the template Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> --- Code/Editor/IEditorImpl.cpp | 13 +++- Gems/Multiplayer/Code/CMakeLists.txt | 1 - cmake/3rdParty.cmake | 19 ++++- cmake/LYWrappers.cmake | 39 ++++++++-- cmake/Platform/Common/Install_common.cmake | 89 ++++++++++++++-------- scripts/o3de.bat | 4 +- scripts/o3de/o3de/engine_template.py | 47 +++++------- scripts/o3de/o3de/manifest.py | 6 +- scripts/o3de/o3de/register.py | 6 +- 9 files changed, 142 insertions(+), 82 deletions(-) diff --git a/Code/Editor/IEditorImpl.cpp b/Code/Editor/IEditorImpl.cpp index d9e07f0ec4..f472e12fc9 100644 --- a/Code/Editor/IEditorImpl.cpp +++ b/Code/Editor/IEditorImpl.cpp @@ -22,7 +22,9 @@ AZ_PUSH_DISABLE_WARNING(4251 4355 4996, "-Wunknown-warning-option") AZ_POP_DISABLE_WARNING // AzCore +#include #include +#include // AzFramework #include @@ -195,8 +197,15 @@ CEditorImpl::CEditorImpl() m_pAssetBrowserRequestHandler = nullptr; m_assetEditorRequestsHandler = nullptr; - AZ::IO::SystemFile::CreateDir("SessionStatus"); - QFile::setPermissions(m_crashLogFileName, QFileDevice::ReadOther | QFileDevice::WriteOther); + if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr) + { + if (AZ::IO::FixedMaxPath crashLogPath; settingsRegistry->Get(crashLogPath.Native(), AZ::SettingsRegistryMergeUtils::FilePathKey_ProjectUserPath)) + { + crashLogPath /= m_crashLogFileName; + AZ::IO::SystemFile::CreateDir(crashLogPath.ParentPath().FixedMaxPathString().c_str()); + QFile::setPermissions(crashLogPath.c_str(), QFileDevice::ReadOther | QFileDevice::WriteOther); + } + } } void CEditorImpl::Initialize() diff --git a/Gems/Multiplayer/Code/CMakeLists.txt b/Gems/Multiplayer/Code/CMakeLists.txt index 89372025e3..ab6e22225b 100644 --- a/Gems/Multiplayer/Code/CMakeLists.txt +++ b/Gems/Multiplayer/Code/CMakeLists.txt @@ -107,7 +107,6 @@ if (PAL_TRAIT_BUILD_HOST_TOOLS) ly_add_target( NAME Multiplayer.Builders GEM_MODULE NAMESPACE Gem - OUTPUT_NAME Gem.Multiplayer.Builders FILES_CMAKE multiplayer_tools_files.cmake INCLUDE_DIRECTORIES diff --git a/cmake/3rdParty.cmake b/cmake/3rdParty.cmake index 204aaedb79..15dde3c3f8 100644 --- a/cmake/3rdParty.cmake +++ b/cmake/3rdParty.cmake @@ -8,7 +8,24 @@ # Do not overcomplicate searching for the 3rdParty path, if it is not easy to find, # the user should define it. -set(LY_3RDPARTY_PATH "" CACHE PATH "Path to the 3rdParty folder") +#! get_default_third_party_folder: Stores the default 3rdParty directory into the supplied output variable +# +# \arg:output_third_party_path name of variable to set the default project directory into +# It defaults to the ~/.o3de/3rdParty directory +function(get_default_third_party_folder output_third_party_path) + cmake_path(SET home_directory "$ENV{USERPROFILE}") # Windows + if(NOT EXISTS ${home_directory}) + cmake_path(SET home_directory "$ENV{HOME}") # Unix + if (NOT EXISTS ${home_directory}) + return() + endif() + endif() + + set(${output_third_party_path} ${home_directory}/.o3de/3rdParty PARENT_SCOPE) +endfunction() + +get_default_third_party_folder(o3de_default_third_party_path) +set(LY_3RDPARTY_PATH "${o3de_default_third_party_path}" CACHE PATH "Path to the 3rdParty folder") if(LY_3RDPARTY_PATH) file(TO_CMAKE_PATH ${LY_3RDPARTY_PATH} LY_3RDPARTY_PATH) diff --git a/cmake/LYWrappers.cmake b/cmake/LYWrappers.cmake index 89a4ab29f0..b6daf8b6d7 100644 --- a/cmake/LYWrappers.cmake +++ b/cmake/LYWrappers.cmake @@ -257,14 +257,7 @@ function(ly_add_target) # IDE organization ly_source_groups_from_folders("${ALLFILES}") source_group("Generated Files" REGULAR_EXPRESSION "(${CMAKE_BINARY_DIR})") # Any file coming from the output folder - file(RELATIVE_PATH project_path ${LY_ROOT_FOLDER} ${CMAKE_CURRENT_SOURCE_DIR}) - # Visual Studio cannot load a project with a FOLDER that starts with a "../" relative path - # Strip away any leading ../ and then add a prefix of ExternalTargets/ as that in this scenario - # A relative directory with ../ would be outside of the Lumberyard Engine Root therefore it is external - set(ide_path ${project_path}) - if (${project_path} MATCHES [[^(\.\./)+(.*)]]) - set(ide_path "${CMAKE_MATCH_2}") - endif() + ly_get_vs_folder_directory(${CMAKE_CURRENT_SOURCE_DIR} ide_path) set_property(TARGET ${project_NAME} PROPERTY FOLDER ${ide_path}) @@ -639,3 +632,33 @@ function(ly_de_alias_target target_name output_variable_name) endif() set(${output_variable_name} ${de_aliased_target_name} PARENT_SCOPE) endfunction() + +#! ly_get_vs_folder_directory: Sets the Visual Studio folder name used for organizing vcxproj +# in the IDE +# +# Visual Studio cannot load projects that with a ".." relative path or contain a colon ":" as part of its FOLDER +# Therefore if the .vcxproj is absolute, the drive letter must be removed from the folder name +# +# What this method does is first check if the target being added to the Visual Studio solution is within +# the LY_ROOT_FOLDER(i.e is the LY_ROOT_FOLDER a prefix of the target source directory) +# If it is a relative path to the target is used as the folder name +# Otherwise the target directory would either +# 1. Be a path outside of the LY_ROOT_FOLDER on the same drive. +# In that case forming a relative path would cause it to start with ".." which will not work +# 2. Be an path outside of the LY_ROOT_FOLDER on a different drive +# Here a relative path cannot be formed and therefore the path would start with ":/Path/To/Project" +# Which shows up as unloaded due to containing a colon +# In this scenario the relative part of the path from the drive letter is used as the FOLDER name +# to make sure the projects show up in the loaded .sln +function(ly_get_vs_folder_directory absolute_target_source_dir output_source_dir) + # Get a relative directory to the LY_ROOT_FOLDER if possible for the Visual Studio solution hierarchy + # If a relative path cannot be formed, then retrieve a path with the drive letter stripped from it + cmake_path(IS_PREFIX LY_ROOT_FOLDER ${absolute_target_source_dir} is_target_prefix_of_engine_root) + if(is_target_prefix_of_engine_root) + cmake_path(RELATIVE_PATH absolute_target_source_dir BASE_DIRECTORY ${LY_ROOT_FOLDER} OUTPUT_VARIABLE relative_target_source_dir) + else() + cmake_path(GET absolute_target_source_dir RELATIVE_PART relative_target_source_dir) + endif() + + set(${output_source_dir} ${relative_target_source_dir} PARENT_SCOPE) +endfunction() diff --git a/cmake/Platform/Common/Install_common.cmake b/cmake/Platform/Common/Install_common.cmake index 4c94aa7fc5..7ab6006220 100644 --- a/cmake/Platform/Common/Install_common.cmake +++ b/cmake/Platform/Common/Install_common.cmake @@ -163,6 +163,7 @@ function(ly_setup_target OUTPUT_CONFIGURED_TARGET ALIAS_TARGET_NAME absolute_tar unset(RUNTIME_DEPENDENCIES_PLACEHOLDER) endif() + get_target_property(inteface_build_dependencies_props ${TARGET_NAME} INTERFACE_LINK_LIBRARIES) unset(INTERFACE_BUILD_DEPENDENCIES_PLACEHOLDER) if(inteface_build_dependencies_props) @@ -522,47 +523,75 @@ function(ly_setup_others) DESTINATION . ) - # Gem Source Assets and Registry + # Gem Source Assets and configuration files # Find all gem directories relative to the CMake Source Dir - file(GLOB_RECURSE - gems_assets_path - LIST_DIRECTORIES TRUE - RELATIVE "${LY_ROOT_FOLDER}/" - "Gems/*" - ) - list(FILTER gems_assets_path INCLUDE REGEX "/(Assets|Registry)$") - foreach (gem_assets_path ${gems_assets_path}) - set(gem_abs_assets_path ${LY_ROOT_FOLDER}/${gem_assets_path}/) - if (EXISTS ${gem_abs_assets_path}) + # This first loop is to filter out transient and .gitignore'd folders that should be added to + # the install layout from the root directory. Such as /Cache. + # This is also done to avoid globbing thousands of files in subdirectories that shouldn't + # be processed. + foreach(gem_candidate_dir IN LISTS LY_EXTERNAL_SUBDIRS LY_PROJECTS) + file(REAL_PATH ${gem_candidate_dir} gem_candidate_dir BASE_DIRECTORY ${LY_ROOT_FOLDER}) + # Don't recurse immediately in order to exclude transient source artifacts + file(GLOB + external_subdir_files + LIST_DIRECTORIES TRUE + "${gem_candidate_dir}/*" + ) + # Exclude transient artifacts that shouldn't be copied to the install layout + list(FILTER external_subdir_files EXCLUDE REGEX "/([Bb]uild|[Cc]ache|[Uu]ser)$") + list(APPEND filtered_asset_paths ${external_subdir_files}) + endforeach() + + # At this point the filtered_assets_paths contains the list of all directories and files + # that are non-excluded candidates that can be scanned for target directories and files + # to copy over to the install layout + foreach(filtered_asset_path IN LISTS filtered_asset_paths) + if(IS_DIRECTORY ${filtered_asset_path}) + file(GLOB_RECURSE + recurse_assets_paths + LIST_DIRECTORIES TRUE + "${filtered_asset_path}/*" + ) + set(gem_file_paths ${recurse_assets_paths}) + # Make sure to prepend the current path iteration to the gem_dirs_path to filter + set(gem_dir_paths ${filtered_asset_path} ${recurse_assets_paths}) + + # Gather directories to copy over + # Currently only the Assets, Registry and Config directories are copied over + list(FILTER gem_dir_paths INCLUDE REGEX "/(Assets|Registry|Config)$") + list(APPEND gems_assets_dir_path ${gem_dir_paths}) + else() + set(gem_file_paths ${filtered_asset_path}) + endif() + + # Gather files to copy over + # Currently only the gem.json file is copied over + list(FILTER gem_file_paths INCLUDE REGEX "/(gem.json)$") + list(APPEND gems_assets_file_path "${gem_file_paths}") + endforeach() + + # gem directories to install + foreach(gem_absolute_dir_path ${gems_assets_dir_path}) + cmake_path(RELATIVE_PATH gem_absolute_dir_path BASE_DIRECTORY ${LY_ROOT_FOLDER} OUTPUT_VARIABLE gem_relative_dir_path) + if (EXISTS ${gem_absolute_dir_path}) # The trailing slash is IMPORTANT here as that is needed to prevent # the "Assets" folder from being copied underneath the /Assets folder - install(DIRECTORY ${gem_abs_assets_path} - DESTINATION ${gem_assets_path} + install(DIRECTORY "${gem_absolute_dir_path}/" + DESTINATION ${gem_relative_dir_path} ) endif() endforeach() - # gem.json files - file(GLOB_RECURSE - gems_json_path - LIST_DIRECTORIES FALSE - RELATIVE "${LY_ROOT_FOLDER}" - "Gems/*/gem.json" - ) - foreach(gem_json_path ${gems_json_path}) - get_filename_component(gem_relative_path ${gem_json_path} DIRECTORY) - install(FILES ${gem_json_path} - DESTINATION ${gem_relative_path} + # gem files to install + foreach(gem_absolute_file_path ${gems_assets_file_path}) + cmake_path(RELATIVE_PATH gem_absolute_file_path BASE_DIRECTORY ${LY_ROOT_FOLDER} OUTPUT_VARIABLE gem_relative_file_path) + cmake_path(GET gem_relative_file_path PARENT_PATH gem_relative_parent_dir) + install(FILES ${gem_absolute_file_path} + DESTINATION ${gem_relative_parent_dir} ) endforeach() - # Additional files needed by gems - install(DIRECTORY - ${LY_ROOT_FOLDER}/Gems/Atom/Asset/ImageProcessingAtom/Config - DESTINATION Gems/Atom/Asset/ImageProcessingAtom - ) - # Templates install(DIRECTORY ${LY_ROOT_FOLDER}/Templates diff --git a/scripts/o3de.bat b/scripts/o3de.bat index 98603bbeb2..b7e3a67261 100644 --- a/scripts/o3de.bat +++ b/scripts/o3de.bat @@ -9,7 +9,7 @@ REM pushd %~dp0% CD %~dp0.. SET "BASE_PATH=%CD%" -CD %~dp0 +popd SET "PYTHON_DIRECTORY=%BASE_PATH%\python" IF EXIST "%PYTHON_DIRECTORY%" GOTO pythonPathAvailable GOTO pythonDirNotFound @@ -25,10 +25,8 @@ GOTO fail ECHO Python executable not found: %PYTHON_EXECUTABLE% GOTO fail :fail -popd EXIT /b 1 :end -popd EXIT /b %ERRORLEVEL% diff --git a/scripts/o3de/o3de/engine_template.py b/scripts/o3de/o3de/engine_template.py index e424288eb5..1bc2cfc9fc 100755 --- a/scripts/o3de/o3de/engine_template.py +++ b/scripts/o3de/o3de/engine_template.py @@ -17,7 +17,7 @@ import uuid import re -from o3de import manifest, validation, utils +from o3de import manifest, register, validation, utils logger = logging.getLogger() logging.basicConfig() @@ -388,10 +388,11 @@ def create_template(source_path: pathlib.Path, if not source_path: logger.error('Src path cannot be empty.') return 1 - if not os.path.isdir(source_path): + if not source_path.is_dir(): logger.error(f'Src path {source_path} is not a folder.') return 1 + source_path = source_path.resolve() # source_name is now the last component of the source_path if not source_name: source_name = os.path.basename(source_path) @@ -401,11 +402,11 @@ def create_template(source_path: pathlib.Path, if not template_path: logger.info(f'Template path empty. Using source name {source_name}') template_path = source_name - if not os.path.isabs(template_path): + if not template_path.is_absolute(): default_templates_folder = manifest.get_registered(default_folder='templates') - template_path = default_templates_folder/ template_path + template_path = default_templates_folder / template_path logger.info(f'Template path not a full path. Using default templates folder {template_path}') - if not force and os.path.isdir(template_path): + if not force and template_path.is_dir(): logger.error(f'Template path {template_path} already exists.') return 1 @@ -415,8 +416,7 @@ def create_template(source_path: pathlib.Path, except ValueError: pass else: - logger.error(f'Template output path {template_path} cannot be a subdirectory of the source_path {source_path}:\n' - f'{err}') + logger.error(f'Template output path {template_path} cannot be a subdirectory of the source_path {source_path}\n') return 1 # template name is now the last component of the template_path @@ -1200,7 +1200,7 @@ def create_from_template(destination_path: pathlib.Path, if not destination_name: # destination name is now the last component of the destination_path - destination_name = os.path.basename(destination_path) + destination_name = destination_path.name # destination name cannot be the same as a restricted platform name if destination_name in restricted_platforms: @@ -1654,28 +1654,10 @@ def create_project(project_path: pathlib.Path, d.write('# SPDX-License-Identifier: Apache-2.0 OR MIT\n') d.write('# {END_LICENSE}\n') - # set the "engine" element of the project.json - engine_json_data = manifest.get_engine_json_data(engine_path=manifest.get_this_engine_path()) - try: - engine_name = engine_json_data['engine_name'] - except KeyError as e: - logger.error(f"engine_name for this engine not found in engine.json.") - return 1 - - project_json_data = manifest.get_project_json_data(project_path=project_path) - if not project_json_data: - # get_project_json_data already logs an error if the project.json is mising - return 1 - - project_json_data.update({"engine": engine_name}) - 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 at {project_path}.') - return 1 - return 0 + # Register the project with the global o3de_manifest.json and set the project.json "engine" field to match the + # engine.json "engine_name" field + return register.register(project_path=project_path) def create_gem(gem_path: pathlib.Path, @@ -1745,6 +1727,11 @@ def create_gem(gem_path: pathlib.Path, if template_name and not template_path: template_path = manifest.get_registered(template_name=template_name) + if not template_path: + logger.error(f'Could not find the template path using name {template_name}.\n' + 'Has the template been registered yet? It can be registered via the ' + '"o3de.py register --tp " command') + return 1 if not os.path.isdir(template_path): logger.error(f'Could not find the template {template_name}=>{template_path}') return 1 @@ -2068,7 +2055,7 @@ def _run_create_from_template(args: argparse) -> int: return create_from_template(args.destination_path, args.template_path, args.template_name, - args.destination_path, + args.destination_name, args.destination_restricted_path, args.destination_restricted_name, args.template_restricted_path, diff --git a/scripts/o3de/o3de/manifest.py b/scripts/o3de/o3de/manifest.py index 832987be3c..c4a7cf8f36 100644 --- a/scripts/o3de/o3de/manifest.py +++ b/scripts/o3de/o3de/manifest.py @@ -194,9 +194,9 @@ def load_o3de_manifest(manifest_path: pathlib.Path = None) -> dict: return json_data -def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> None: +def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> bool: """ - Save the json dictionary to the supplied manifest file or ~/.o3de/o3de_manifest.json if None + Save the json dictionary to the supplied manifest file or ~/.o3de/o3de_manifest.json if manifest_path is None :param json_data: dictionary to save in json format at the file path :param manifest_path: optional path to manifest file to save @@ -206,8 +206,10 @@ def save_o3de_manifest(json_data: dict, manifest_path: pathlib.Path = None) -> N with manifest_path.open('w') as s: try: s.write(json.dumps(json_data, indent=4) + '\n') + return True except OSError as e: logger.error(f'Manifest json failed to save: {str(e)}') + return False # Data query methods diff --git a/scripts/o3de/o3de/register.py b/scripts/o3de/o3de/register.py index 7ad341784e..40556205de 100644 --- a/scripts/o3de/o3de/register.py +++ b/scripts/o3de/o3de/register.py @@ -341,7 +341,7 @@ def register_o3de_object_path(json_data: dict, try: paths_to_remove.append(o3de_object_path.relative_to(save_path.parent)) except ValueError: - pass # It is OK relative path cannot be formed + pass # It is OK relative path cannot be formed manifest_data[o3de_object_key] = list(filter(lambda p: pathlib.Path(p) not in paths_to_remove, manifest_data.setdefault(o3de_object_key, []))) @@ -425,7 +425,6 @@ def register_project_path(json_data: dict, if not manifest.save_o3de_manifest(project_json_data, project_json_path): return 1 - return 0 @@ -754,9 +753,6 @@ def _run_register(args: argparse) -> int: return repo.refresh_repos() elif args.this_engine: ret_val = register(engine_path=manifest.get_this_engine_path(), force=args.force) - error_code = register_shipped_engine_o3de_objects(force=args.force) - if error_code: - ret_val = error_code return ret_val elif args.all_engines_path: return register_all_engines_in_folder(args.all_engines_path, args.remove, args.force)