From d71de779156dc3704e6daad079456be32b97b696 Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:45:55 +0100 Subject: [PATCH 1/8] fix: handling I/O failures + refactor: unify logging and assertions This commit also... ...reverts: https://github.com/adamrehn/ue4cli/commit/fed71c1af4cffe3fed9358b08430068ad9454b77 ...fixes: https://github.com/adamrehn/ue4cli/issues/29 --- ue4cli/JsonDataManager.py | 5 +---- ue4cli/UnrealManagerBase.py | 16 +++++++++------- ue4cli/UnrealManagerWindows.py | 2 +- ue4cli/Utility.py | 8 ++++---- ue4cli/cli.py | 25 +++++++++++++++++++++---- 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/ue4cli/JsonDataManager.py b/ue4cli/JsonDataManager.py index 53fbb77..a520143 100644 --- a/ue4cli/JsonDataManager.py +++ b/ue4cli/JsonDataManager.py @@ -28,10 +28,7 @@ def getDictionary(self): Retrieves the entire data dictionary """ if os.path.exists(self.jsonFile): - try: - return json.loads(Utility.readFile(self.jsonFile)) - except json.JSONDecodeError as err: - raise UnrealManagerException('malformed JSON configuration file "{}" ({})'.format(self.jsonFile, err)) + return json.loads(Utility.readFile(self.jsonFile)) else: return {} diff --git a/ue4cli/UnrealManagerBase.py b/ue4cli/UnrealManagerBase.py index 763557f..2952c47 100644 --- a/ue4cli/UnrealManagerBase.py +++ b/ue4cli/UnrealManagerBase.py @@ -44,13 +44,13 @@ def setEngineRootOverride(self, rootDir): # Set the new root directory rootDir = os.path.abspath(rootDir) ConfigurationManager.setConfigKey('rootDirOverride', rootDir) - print('Set engine root path override: {}'.format(rootDir)) + Utility.printStderr('Setting engine root path override:', str(rootDir)) # Check that the specified directory is valid and warn the user if it is not try: self.getEngineVersion() except: - print('Warning: the specified directory does not appear to contain a valid version of the Unreal Engine.') + UnrealManagerException('the specified directory does not appear to contain a valid version of the Unreal Engine.') def clearEngineRootOverride(self): """ @@ -94,7 +94,7 @@ def getEngineVersion(self, outputFormat = 'full'): # Verify that the requested output format is valid if outputFormat not in formats: - raise Exception('unreconised version output format "{}"'.format(outputFormat)) + raise UnrealManagerException('unreconised version output format "{}"'.format(outputFormat)) return formats[outputFormat] @@ -538,7 +538,7 @@ def listAutomationTests(self, projectFile): # Detect if the Editor terminated abnormally (i.e. not triggered by `automation quit`) # In Unreal Engine 4.27.0, the exit method changed from RequestExit to RequestExitWithStatus if 'PlatformMisc::RequestExit(' not in logOutput.stdout and 'PlatformMisc::RequestExitWithStatus(' not in logOutput.stdout: - raise RuntimeError( + raise UnrealManagerException( 'failed to retrieve the list of automation tests!' + ' stdout was: "{}", stderr was: "{}"'.format(logOutput.stdout, logOutput.stderr) ) @@ -556,7 +556,7 @@ def automationTests(self, dir=os.getcwd(), args=[]): # Verify that at least one argument was supplied if len(args) == 0: - raise RuntimeError('at least one test name must be specified') + raise UnrealManagerException('at least one test name must be specified') # Gather any additional arguments to pass directly to the Editor extraArgs = [] @@ -605,7 +605,7 @@ def automationTests(self, dir=os.getcwd(), args=[]): # Detect abnormal exit conditions (those not triggered by `automation quit`) # In Unreal Engine 4.27.0, the exit method changed from RequestExit to RequestExitWithStatus if 'PlatformMisc::RequestExit(' not in logOutput.stdout and 'PlatformMisc::RequestExitWithStatus(' not in logOutput.stdout: - sys.exit(1) + raise UnrealManagerException('abnormal exit condition') # Since UE4 doesn't consistently produce accurate exit codes across all platforms, we need to rely on # text-based heuristics to detect failed automation tests or errors related to not finding any tests to run @@ -618,12 +618,14 @@ def automationTests(self, dir=os.getcwd(), args=[]): ] for errorStr in errorStrings: if errorStr in logOutput.stdout: - sys.exit(1) + raise UnrealManagerException('abnormal exit condition') # If an explicit exit code was specified in the output text then identify it and propagate it match = re.search('TEST COMPLETE\\. EXIT CODE: ([0-9]+)', logOutput.stdout + logOutput.stderr) if match is not None: sys.exit(int(match.group(1))) + else: + raise UnrealManagerException('abnormal exit condition') # "Protected" methods diff --git a/ue4cli/UnrealManagerWindows.py b/ue4cli/UnrealManagerWindows.py index 535c69b..c229fbf 100644 --- a/ue4cli/UnrealManagerWindows.py +++ b/ue4cli/UnrealManagerWindows.py @@ -52,7 +52,7 @@ def generateProjectFiles(self, dir=os.getcwd(), args=[]): # If we are using our custom batch file, use the appropriate arguments genScript = self.getGenerateScript() projectFile = self.getProjectDescriptor(dir) - print(projectFile) + Utility.printStderr('Using project file:', projectFile) if '.ue4\\GenerateProjectFiles.bat' in genScript: Utility.run([genScript, projectFile], raiseOnError=True) else: diff --git a/ue4cli/Utility.py b/ue4cli/Utility.py index 65e4e12..ae9aab2 100644 --- a/ue4cli/Utility.py +++ b/ue4cli/Utility.py @@ -21,7 +21,7 @@ def printStderr(*args, **kwargs): Prints to stderr instead of stdout """ if os.environ.get('UE4CLI_QUIET', '0') != '1': - print(*args, file=sys.stderr, **kwargs) + print('(ue4cli)', *args, end='\n', file=sys.stderr, **kwargs) @staticmethod def readFile(filename): @@ -123,7 +123,7 @@ def capture(command, input=None, cwd=None, shell=False, raiseOnError=False): # If the child process failed and we were asked to raise an exception, do so if raiseOnError == True and proc.returncode != 0: - raise Exception( + raise subprocess.SubprocessError( 'child process ' + str(command) + ' failed with exit code ' + str(proc.returncode) + '\nstdout: "' + stdout + '"' + @@ -143,7 +143,7 @@ def run(command, cwd=None, shell=False, raiseOnError=False): returncode = subprocess.call(command, cwd=cwd, shell=shell) if raiseOnError == True and returncode != 0: - raise Exception('child process ' + str(command) + ' failed with exit code ' + str(returncode)) + raise subprocess.SubprocessError('child process ' + str(command) + ' failed with exit code ' + str(returncode)) return returncode @staticmethod @@ -152,4 +152,4 @@ def _printCommand(command): Prints a command if verbose output is enabled """ if os.environ.get('UE4CLI_VERBOSE', '0') == '1': - Utility.printStderr('[UE4CLI] EXECUTE COMMAND:', command) + Utility.printStderr('EXECUTE COMMAND:', command) diff --git a/ue4cli/cli.py b/ue4cli/cli.py index d5f51e3..d08d327 100644 --- a/ue4cli/cli.py +++ b/ue4cli/cli.py @@ -2,7 +2,10 @@ from .PluginManager import PluginManager from .UnrealManagerException import UnrealManagerException from .UnrealManagerFactory import UnrealManagerFactory -import os, sys +from .Utility import Utility +from subprocess import SubprocessError +from json import JSONDecodeError +import os, sys, logging # Our list of supported commands SUPPORTED_COMMANDS = { @@ -201,6 +204,7 @@ def displayHelp(): def main(): try: + logger = logging.getLogger(__name__) # Perform plugin detection and register our detected plugins plugins = PluginManager.getPlugins() @@ -222,7 +226,20 @@ def main(): SUPPORTED_COMMANDS[command]['action'](manager, args) else: raise UnrealManagerException('unrecognised command "' + command + '"') - - except UnrealManagerException as e: - print('Error: ' + str(e)) + except ( + UnrealManagerException, + OSError, + SubprocessError, + JSONDecodeError, + KeyboardInterrupt, + ) as e: + Utility.printStderr('(' + type(e).__name__ + ')', str(e)) + sys.exit(1) + except BaseException as e: + Utility.printStderr('Unhandled exception! Crashing...') + logging.basicConfig(level=logging.DEBUG) + logger.exception(e) + Utility.printStderr('ue4cli has crashed! Please, report it at: https://github.com/adamrehn/ue4cli/issues') sys.exit(1) + + From c17c247a4f09e9281db24c11e1239ab32b8c012f Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Sat, 23 Mar 2024 00:21:53 +0100 Subject: [PATCH 2/8] fix: add missing 'raise' keyword --- ue4cli/UnrealManagerBase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ue4cli/UnrealManagerBase.py b/ue4cli/UnrealManagerBase.py index 2952c47..da42208 100644 --- a/ue4cli/UnrealManagerBase.py +++ b/ue4cli/UnrealManagerBase.py @@ -50,7 +50,7 @@ def setEngineRootOverride(self, rootDir): try: self.getEngineVersion() except: - UnrealManagerException('the specified directory does not appear to contain a valid version of the Unreal Engine.') + raise UnrealManagerException('the specified directory does not appear to contain a valid version of the Unreal Engine.') def clearEngineRootOverride(self): """ From d1d5ac5bc3ac1fd51c643a539a2bce624378bbc6 Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Sat, 23 Mar 2024 00:49:01 +0100 Subject: [PATCH 3/8] reverse 'else' block - Adding it here was unsave - it could produce unwanted behavior --- ue4cli/UnrealManagerBase.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ue4cli/UnrealManagerBase.py b/ue4cli/UnrealManagerBase.py index da42208..33887a5 100644 --- a/ue4cli/UnrealManagerBase.py +++ b/ue4cli/UnrealManagerBase.py @@ -624,8 +624,6 @@ def automationTests(self, dir=os.getcwd(), args=[]): match = re.search('TEST COMPLETE\\. EXIT CODE: ([0-9]+)', logOutput.stdout + logOutput.stderr) if match is not None: sys.exit(int(match.group(1))) - else: - raise UnrealManagerException('abnormal exit condition') # "Protected" methods From 0e62c1214598bf54abf606549a0c8196be316129 Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Sat, 23 Mar 2024 13:58:51 +0100 Subject: [PATCH 4/8] catch SystemExit as we are using it when performing automation tests --- ue4cli/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ue4cli/cli.py b/ue4cli/cli.py index d08d327..73f7a9d 100644 --- a/ue4cli/cli.py +++ b/ue4cli/cli.py @@ -232,6 +232,7 @@ def main(): SubprocessError, JSONDecodeError, KeyboardInterrupt, + SystemExit, ) as e: Utility.printStderr('(' + type(e).__name__ + ')', str(e)) sys.exit(1) From 2747bf640c445f645f8854b369fc78a4a84a5924 Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Sat, 23 Mar 2024 21:18:54 +0100 Subject: [PATCH 5/8] refactor: wrap every I/O call with try-except block and pass the error to UtilityException with meaningful message --- ue4cli/JsonDataManager.py | 11 +++++++++-- ue4cli/UE4BuildInterrogator.py | 18 +++++++++++++++--- ue4cli/UnrealManagerBase.py | 28 ++++++++++++++++++---------- ue4cli/Utility.py | 31 +++++++++++++++++++------------ ue4cli/UtilityException.py | 2 ++ ue4cli/cli.py | 9 ++------- 6 files changed, 65 insertions(+), 34 deletions(-) create mode 100644 ue4cli/UtilityException.py diff --git a/ue4cli/JsonDataManager.py b/ue4cli/JsonDataManager.py index a520143..ba6f90d 100644 --- a/ue4cli/JsonDataManager.py +++ b/ue4cli/JsonDataManager.py @@ -1,5 +1,6 @@ from .UnrealManagerException import UnrealManagerException from .Utility import Utility +from .UtilityException import UtilityException import json, os, platform class JsonDataManager(object): @@ -28,7 +29,10 @@ def getDictionary(self): Retrieves the entire data dictionary """ if os.path.exists(self.jsonFile): - return json.loads(Utility.readFile(self.jsonFile)) + try: + return json.loads(Utility.readFile(self.jsonFile)) + except json.JSONDecodeError as e: + raise UtilityException(f'failed to load {str(self.jsonFile)} due to {type(e).__name__} {str(e)}') else: return {} @@ -48,7 +52,10 @@ def setDictionary(self, data): # Create the directory containing the JSON file if it doesn't already exist jsonDir = os.path.dirname(self.jsonFile) if os.path.exists(jsonDir) == False: - os.makedirs(jsonDir) + try: + os.makedirs(jsonDir) + except OSError as e: + raise UtilityException(f'failed to create directory {str(jsonDir)} due to {type(e).__name__} {str(e)}') # Store the dictionary Utility.writeFile(self.jsonFile, json.dumps(data)) diff --git a/ue4cli/UE4BuildInterrogator.py b/ue4cli/UE4BuildInterrogator.py index 12a42d8..0575f57 100644 --- a/ue4cli/UE4BuildInterrogator.py +++ b/ue4cli/UE4BuildInterrogator.py @@ -2,11 +2,14 @@ from .UnrealManagerException import UnrealManagerException from .CachedDataManager import CachedDataManager from .Utility import Utility +from .UtilityException import UtilityException import json, os, platform, shutil, tempfile class UE4BuildInterrogator(object): def __init__(self, engineRoot, engineVersion, engineVersionHash, runUBTFunc): + # WARN: os.path.realpath can potentially fail with OSError, + # but if it ever happens, this is most likely bug in our code self.engineRoot = os.path.realpath(engineRoot) self.engineSourceDir = 'Engine/Source/' self.engineVersion = engineVersion @@ -160,7 +163,10 @@ def _getThirdPartyLibs(self, platformIdentifier, configuration): sentinelBackup = sentinelFile + '.bak' renameSentinel = os.path.exists(sentinelFile) and os.environ.get('UE4CLI_SENTINEL_RENAME', '0') == '1' if renameSentinel == True: - shutil.move(sentinelFile, sentinelBackup) + try: + shutil.move(sentinelFile, sentinelBackup) + except OSError as e: + raise UtilityException(f'failed to move {str(sentinelFile)} to {str(sentinelBackup)} due to {type(e).__name__} {str(e)}') # Invoke UnrealBuildTool in JSON export mode (make sure we specify gathering mode, since this is a prerequisite of JSON export) # (Ensure we always perform sentinel file cleanup even when errors occur) @@ -172,10 +178,16 @@ def _getThirdPartyLibs(self, platformIdentifier, configuration): self.runUBTFunc('UE4Editor', platformIdentifier, configuration, args) finally: if renameSentinel == True: - shutil.move(sentinelBackup, sentinelFile) + try: + shutil.move(sentinelBackup, sentinelFile) + except OSError as e: + raise UtilityException(f'failed to move {str(sentinelBackup)} to {str(sentinelFile)} due to {type(e).__name__} {str(e)}') # Parse the JSON output - result = json.loads(Utility.readFile(jsonFile)) + try: + result = json.loads(Utility.readFile(jsonFile)) + except json.JSONDecodeError as e: + raise UtilityException(f'failed to load {str(jsonFile)} due to {type(e).__name__} {str(e)}') # Extract the list of third-party library modules # (Note that since UE4.21, modules no longer have a "Type" field, so we must diff --git a/ue4cli/UnrealManagerBase.py b/ue4cli/UnrealManagerBase.py index 33887a5..3efc24c 100644 --- a/ue4cli/UnrealManagerBase.py +++ b/ue4cli/UnrealManagerBase.py @@ -5,6 +5,7 @@ from .CachedDataManager import CachedDataManager from .CMakeCustomFlags import CMakeCustomFlags from .Utility import Utility +from .UtilityException import UtilityException import glob, hashlib, json, os, re, shutil, sys class UnrealManagerBase(object): @@ -316,8 +317,7 @@ def generateProjectFiles(self, dir=os.getcwd(), args=[]): # If the project is a pure Blueprint project, then we cannot generate project files if os.path.exists(os.path.join(dir, 'Source')) == False: - Utility.printStderr('Pure Blueprint project, nothing to generate project files for.') - return + raise UnrealManagerException('Pure Blueprint project, nothing to generate project files for.') # Generate the project files genScript = self.getGenerateScript() @@ -354,8 +354,7 @@ def buildDescriptor(self, dir=os.getcwd(), configuration='Development', target=' # If the project or plugin is Blueprint-only, there is no C++ code to build if os.path.exists(os.path.join(dir, 'Source')) == False: - Utility.printStderr('Pure Blueprint {}, nothing to build.'.format(descriptorType)) - return + raise UnrealManagerException('Pure Blueprint {}, nothing to build.'.format(descriptorType)) # Verify that the specified build configuration is valid if configuration not in self.validBuildConfigurations(): @@ -567,7 +566,12 @@ def automationTests(self, dir=os.getcwd(), args=[]): # Build the project if it isn't already built Utility.printStderr('Ensuring project is built...') - self.buildDescriptor(dir, suppressOutput=True) + try: + self.buildDescriptor(dir, suppressOutput=True) + except UnrealManagerException: + # FIXME: Ideally, we should NOT catch every UnrealManagerException here + # This is currently a limitation of our API that uses only one Exception class for multiple different cases + pass # Determine which arguments we are passing to the automation test commandlet projectFile = self.getProjectDescriptor(dir) @@ -605,7 +609,7 @@ def automationTests(self, dir=os.getcwd(), args=[]): # Detect abnormal exit conditions (those not triggered by `automation quit`) # In Unreal Engine 4.27.0, the exit method changed from RequestExit to RequestExitWithStatus if 'PlatformMisc::RequestExit(' not in logOutput.stdout and 'PlatformMisc::RequestExitWithStatus(' not in logOutput.stdout: - raise UnrealManagerException('abnormal exit condition') + raise UnrealManagerException('abnormal exit condition detected') # Since UE4 doesn't consistently produce accurate exit codes across all platforms, we need to rely on # text-based heuristics to detect failed automation tests or errors related to not finding any tests to run @@ -618,12 +622,12 @@ def automationTests(self, dir=os.getcwd(), args=[]): ] for errorStr in errorStrings: if errorStr in logOutput.stdout: - raise UnrealManagerException('abnormal exit condition') + raise UnrealManagerException('abnormal exit condition detected') # If an explicit exit code was specified in the output text then identify it and propagate it - match = re.search('TEST COMPLETE\\. EXIT CODE: ([0-9]+)', logOutput.stdout + logOutput.stderr) + match = re.search('TEST COMPLETE\\. EXIT CODE: ([1-9]+)', logOutput.stdout + logOutput.stderr) if match is not None: - sys.exit(int(match.group(1))) + raise UnrealManagerException('abnormal exit condition detected') # "Protected" methods @@ -638,7 +642,11 @@ def _getEngineVersionDetails(self): Parses the JSON version details for the latest installed version of UE4 """ versionFile = os.path.join(self.getEngineRoot(), 'Engine', 'Build', 'Build.version') - return json.loads(Utility.readFile(versionFile)) + jsonFile = Utility.readFile(versionFile) + try: + return json.loads(jsonFile) + except json.JSONDecodeError as e: + raise UtilityException(f'failed to load {str(jsonFile)} due to {type(e).__name__} {str(e)}') def _getEngineVersionHash(self): """ diff --git a/ue4cli/Utility.py b/ue4cli/Utility.py index ae9aab2..17f1703 100644 --- a/ue4cli/Utility.py +++ b/ue4cli/Utility.py @@ -1,3 +1,4 @@ +from .UtilityException import UtilityException import os, platform, shlex, subprocess, sys class CommandOutput(object): @@ -21,23 +22,29 @@ def printStderr(*args, **kwargs): Prints to stderr instead of stdout """ if os.environ.get('UE4CLI_QUIET', '0') != '1': - print('(ue4cli)', *args, end='\n', file=sys.stderr, **kwargs) + print('(ue4cli)', *args, file=sys.stderr, **kwargs) @staticmethod def readFile(filename): """ Reads data from a file """ - with open(filename, 'rb') as f: - return f.read().decode('utf-8') + try: + with open(filename, 'rb') as f: + return f.read().decode('utf-8') + except OSError as e: + raise UtilityException(f'failed to read file {str(filename)} due to {type(e).__name__} {str(e)}') @staticmethod def writeFile(filename, data): """ Writes data to a file """ - with open(filename, 'wb') as f: - f.write(data.encode('utf-8')) + try: + with open(filename, 'wb') as f: + f.write(data.encode('utf-8')) + except OSError as e: + raise UtilityException(f'failed to write file {str(filename)} due to {type(e).__name__} {str(e)}') @staticmethod def patchFile(filename, replacements): @@ -123,12 +130,12 @@ def capture(command, input=None, cwd=None, shell=False, raiseOnError=False): # If the child process failed and we were asked to raise an exception, do so if raiseOnError == True and proc.returncode != 0: - raise subprocess.SubprocessError( - 'child process ' + str(command) + - ' failed with exit code ' + str(proc.returncode) + - '\nstdout: "' + stdout + '"' + - '\nstderr: "' + stderr + '"' - ) + Utility.printStderr("Warning: child process failure encountered!") + Utility.printStderr("printing stdout..") + print(stdout) + Utility.printStderr("printing stderr..") + print(stderr) + raise UtilityException(f'child process {str(command)} failed with exit code {str(proc.returncode)}') return CommandOutput(proc.returncode, stdout, stderr) @@ -143,7 +150,7 @@ def run(command, cwd=None, shell=False, raiseOnError=False): returncode = subprocess.call(command, cwd=cwd, shell=shell) if raiseOnError == True and returncode != 0: - raise subprocess.SubprocessError('child process ' + str(command) + ' failed with exit code ' + str(returncode)) + raise UtilityException(f'child process {str(command)} failed with exit code {str(returncode)}') return returncode @staticmethod diff --git a/ue4cli/UtilityException.py b/ue4cli/UtilityException.py new file mode 100644 index 0000000..53af630 --- /dev/null +++ b/ue4cli/UtilityException.py @@ -0,0 +1,2 @@ +class UtilityException(Exception): + pass diff --git a/ue4cli/cli.py b/ue4cli/cli.py index 73f7a9d..25261fa 100644 --- a/ue4cli/cli.py +++ b/ue4cli/cli.py @@ -3,8 +3,7 @@ from .UnrealManagerException import UnrealManagerException from .UnrealManagerFactory import UnrealManagerFactory from .Utility import Utility -from subprocess import SubprocessError -from json import JSONDecodeError +from .UtilityException import UtilityException import os, sys, logging # Our list of supported commands @@ -228,11 +227,8 @@ def main(): raise UnrealManagerException('unrecognised command "' + command + '"') except ( UnrealManagerException, - OSError, - SubprocessError, - JSONDecodeError, + UtilityException, KeyboardInterrupt, - SystemExit, ) as e: Utility.printStderr('(' + type(e).__name__ + ')', str(e)) sys.exit(1) @@ -243,4 +239,3 @@ def main(): Utility.printStderr('ue4cli has crashed! Please, report it at: https://github.com/adamrehn/ue4cli/issues') sys.exit(1) - From 6e7c3e4647cde65dce17858bed19fb8aaf4586fa Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Sat, 23 Mar 2024 22:47:12 +0100 Subject: [PATCH 6/8] Refactor: create abstraction for shutil.move, shutil.rmtree, os.makedirs inside of Utility class --- ue4cli/CachedDataManager.py | 5 +++-- ue4cli/JsonDataManager.py | 24 +++++++++++++++--------- ue4cli/UE4BuildInterrogator.py | 20 ++++++-------------- ue4cli/UnrealManagerBase.py | 13 +++++-------- ue4cli/UnrealManagerWindows.py | 2 +- ue4cli/Utility.py | 32 +++++++++++++++++++++++++++++++- 6 files changed, 61 insertions(+), 35 deletions(-) diff --git a/ue4cli/CachedDataManager.py b/ue4cli/CachedDataManager.py index 9b03753..34678a7 100644 --- a/ue4cli/CachedDataManager.py +++ b/ue4cli/CachedDataManager.py @@ -1,6 +1,7 @@ from .ConfigurationManager import ConfigurationManager from .JsonDataManager import JsonDataManager -import os, shutil +from .Utility import Utility +import os class CachedDataManager(object): """ @@ -13,7 +14,7 @@ def clearCache(): Clears any cached data we have stored about specific engine versions """ if os.path.exists(CachedDataManager._cacheDir()) == True: - shutil.rmtree(CachedDataManager._cacheDir()) + Utility.removeDir(CachedDataManager._cacheDir()) @staticmethod def getCachedDataKey(engineVersionHash, key): diff --git a/ue4cli/JsonDataManager.py b/ue4cli/JsonDataManager.py index ba6f90d..b67fa00 100644 --- a/ue4cli/JsonDataManager.py +++ b/ue4cli/JsonDataManager.py @@ -1,7 +1,7 @@ from .UnrealManagerException import UnrealManagerException from .Utility import Utility from .UtilityException import UtilityException -import json, os, platform +import json, os class JsonDataManager(object): """ @@ -13,6 +13,18 @@ def __init__(self, jsonFile): Creates a new JsonDataManager instance for the specified JSON file """ self.jsonFile = jsonFile + + def loads(self): + """ + Wrapper for json.loads which reads owned jsonFile and loads it + In case of encountering JSONDecodeError, it will raise UtilityException + """ + try: + path = self.jsonFile + file = Utility.readFile(path) + return json.loads(file) + except json.JSONDecodeError as e: + raise UtilityException(f'failed to load {str(path)} due to {type(e).__name__} {str(e)}') def getKey(self, key): """ @@ -29,10 +41,7 @@ def getDictionary(self): Retrieves the entire data dictionary """ if os.path.exists(self.jsonFile): - try: - return json.loads(Utility.readFile(self.jsonFile)) - except json.JSONDecodeError as e: - raise UtilityException(f'failed to load {str(self.jsonFile)} due to {type(e).__name__} {str(e)}') + return self.loads() else: return {} @@ -52,10 +61,7 @@ def setDictionary(self, data): # Create the directory containing the JSON file if it doesn't already exist jsonDir = os.path.dirname(self.jsonFile) if os.path.exists(jsonDir) == False: - try: - os.makedirs(jsonDir) - except OSError as e: - raise UtilityException(f'failed to create directory {str(jsonDir)} due to {type(e).__name__} {str(e)}') + Utility.makeDirs(jsonDir) # Store the dictionary Utility.writeFile(self.jsonFile, json.dumps(data)) diff --git a/ue4cli/UE4BuildInterrogator.py b/ue4cli/UE4BuildInterrogator.py index 0575f57..098f056 100644 --- a/ue4cli/UE4BuildInterrogator.py +++ b/ue4cli/UE4BuildInterrogator.py @@ -3,7 +3,8 @@ from .CachedDataManager import CachedDataManager from .Utility import Utility from .UtilityException import UtilityException -import json, os, platform, shutil, tempfile +from .JsonDataManager import JsonDataManager +import os, tempfile class UE4BuildInterrogator(object): @@ -163,10 +164,7 @@ def _getThirdPartyLibs(self, platformIdentifier, configuration): sentinelBackup = sentinelFile + '.bak' renameSentinel = os.path.exists(sentinelFile) and os.environ.get('UE4CLI_SENTINEL_RENAME', '0') == '1' if renameSentinel == True: - try: - shutil.move(sentinelFile, sentinelBackup) - except OSError as e: - raise UtilityException(f'failed to move {str(sentinelFile)} to {str(sentinelBackup)} due to {type(e).__name__} {str(e)}') + Utility.moveFile(sentinelFile, sentinelBackup) # Invoke UnrealBuildTool in JSON export mode (make sure we specify gathering mode, since this is a prerequisite of JSON export) # (Ensure we always perform sentinel file cleanup even when errors occur) @@ -178,16 +176,10 @@ def _getThirdPartyLibs(self, platformIdentifier, configuration): self.runUBTFunc('UE4Editor', platformIdentifier, configuration, args) finally: if renameSentinel == True: - try: - shutil.move(sentinelBackup, sentinelFile) - except OSError as e: - raise UtilityException(f'failed to move {str(sentinelBackup)} to {str(sentinelFile)} due to {type(e).__name__} {str(e)}') + Utility.moveFile(sentinelBackup, sentinelFile) # Parse the JSON output - try: - result = json.loads(Utility.readFile(jsonFile)) - except json.JSONDecodeError as e: - raise UtilityException(f'failed to load {str(jsonFile)} due to {type(e).__name__} {str(e)}') + result = JsonDataManager(jsonFile).loads() # Extract the list of third-party library modules # (Note that since UE4.21, modules no longer have a "Type" field, so we must @@ -200,7 +192,7 @@ def _getThirdPartyLibs(self, platformIdentifier, configuration): # Remove the temp directory try: - shutil.rmtree(tempDir) + Utility.removeDir(tempDir) except: pass diff --git a/ue4cli/UnrealManagerBase.py b/ue4cli/UnrealManagerBase.py index 3efc24c..6c9d301 100644 --- a/ue4cli/UnrealManagerBase.py +++ b/ue4cli/UnrealManagerBase.py @@ -5,8 +5,9 @@ from .CachedDataManager import CachedDataManager from .CMakeCustomFlags import CMakeCustomFlags from .Utility import Utility +from .JsonDataManager import JsonDataManager from .UtilityException import UtilityException -import glob, hashlib, json, os, re, shutil, sys +import glob, hashlib, json, os, re class UnrealManagerBase(object): """ @@ -334,8 +335,8 @@ def cleanDescriptor(self, dir=os.getcwd()): # Because performing a clean will also delete the engine build itself when using # a source build, we simply delete the `Binaries` and `Intermediate` directories - shutil.rmtree(os.path.join(dir, 'Binaries'), ignore_errors=True) - shutil.rmtree(os.path.join(dir, 'Intermediate'), ignore_errors=True) + Utility.removeDir(os.path.join(dir, 'Binaries'), ignore_errors=True) + Utility.removeDir(os.path.join(dir, 'Intermediate'), ignore_errors=True) # If we are cleaning a project, also clean any plugins if self.isProject(descriptor): @@ -642,11 +643,7 @@ def _getEngineVersionDetails(self): Parses the JSON version details for the latest installed version of UE4 """ versionFile = os.path.join(self.getEngineRoot(), 'Engine', 'Build', 'Build.version') - jsonFile = Utility.readFile(versionFile) - try: - return json.loads(jsonFile) - except json.JSONDecodeError as e: - raise UtilityException(f'failed to load {str(jsonFile)} due to {type(e).__name__} {str(e)}') + return JsonDataManager(versionFile).loads() def _getEngineVersionHash(self): """ diff --git a/ue4cli/UnrealManagerWindows.py b/ue4cli/UnrealManagerWindows.py index c229fbf..c11210d 100644 --- a/ue4cli/UnrealManagerWindows.py +++ b/ue4cli/UnrealManagerWindows.py @@ -91,7 +91,7 @@ def _customBatchScriptDir(self): # If the script directory doesn't already exist, attempt to create it scriptDir = os.path.join(os.environ['HOMEDRIVE'] + os.environ['HOMEPATH'], '.ue4') try: - os.makedirs(scriptDir) + Utility.makeDirs(scriptDir) except: pass diff --git a/ue4cli/Utility.py b/ue4cli/Utility.py index 17f1703..d3a06fd 100644 --- a/ue4cli/Utility.py +++ b/ue4cli/Utility.py @@ -1,5 +1,5 @@ from .UtilityException import UtilityException -import os, platform, shlex, subprocess, sys +import os, platform, shlex, subprocess, sys, shutil class CommandOutput(object): """ @@ -46,6 +46,16 @@ def writeFile(filename, data): except OSError as e: raise UtilityException(f'failed to write file {str(filename)} due to {type(e).__name__} {str(e)}') + @staticmethod + def moveFile(src, dst): + """ + Moves file from 'src' to 'dst' + """ + try: + shutil.move(src, dst) + except OSError as e: + raise UtilityException(f'failed to move {str(src)} to {str(dst)} due to {type(e).__name__} {str(e)}') + @staticmethod def patchFile(filename, replacements): """ @@ -59,6 +69,26 @@ def patchFile(filename, replacements): Utility.writeFile(filename, patched) + @staticmethod + def removeDir(path, ignore_errors=False): + """ + Recursively remove directory tree + """ + try: + shutil.rmtree(path, ignore_errors) + except OSError as e: + raise UtilityException(f'failed to remove directory {str(path)} due to {type(e).__name__} {str(e)}') + + @staticmethod + def makeDirs(name, mode=0o777, exist_ok=False): + """ + Makes directory + """ + try: + os.makedirs(name, mode, exist_ok) + except OSError as e: + raise UtilityException(f'failed to create directory {str(name)} due to {type(e).__name__} {str(e)}') + @staticmethod def forwardSlashes(paths): """ From 05bd9c77bead5785e8ae6b4ca425f242c90e1ead Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Sun, 24 Mar 2024 13:58:40 +0100 Subject: [PATCH 7/8] fix formatting for assertions and add notes --- ue4cli/JsonDataManager.py | 7 ++++--- ue4cli/UnrealManagerWindows.py | 2 +- ue4cli/Utility.py | 10 +++++----- ue4cli/cli.py | 11 ++++++++--- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/ue4cli/JsonDataManager.py b/ue4cli/JsonDataManager.py index b67fa00..7e94238 100644 --- a/ue4cli/JsonDataManager.py +++ b/ue4cli/JsonDataManager.py @@ -16,15 +16,16 @@ def __init__(self, jsonFile): def loads(self): """ - Wrapper for json.loads which reads owned jsonFile and loads it - In case of encountering JSONDecodeError, it will raise UtilityException + Reads and loads owned jsonFile """ try: path = self.jsonFile file = Utility.readFile(path) return json.loads(file) except json.JSONDecodeError as e: - raise UtilityException(f'failed to load {str(path)} due to {type(e).__name__} {str(e)}') + # FIXME: This is the only place outside of Utility class where we use UtilityException. + # Not worth to create new Exception class for only one single case, at least not now. + raise UtilityException(f'failed to load "{str(path)}" due to: ({type(e).__name__}) {str(e)}') def getKey(self, key): """ diff --git a/ue4cli/UnrealManagerWindows.py b/ue4cli/UnrealManagerWindows.py index c11210d..e7a2101 100644 --- a/ue4cli/UnrealManagerWindows.py +++ b/ue4cli/UnrealManagerWindows.py @@ -42,7 +42,7 @@ def getGenerateScript(self): except: pass - raise UnrealManagerException('could not detect the location of GenerateProjectFiles.bat or UnrealVersionSelector.exe.\nThis typically indicates that .uproject files are not correctly associated with UE4.') + raise UnrealManagerException('could not detect the location of GenerateProjectFiles.bat or UnrealVersionSelector.exe. This typically indicates that .uproject files are not correctly associated with UE4.') def getRunUATScript(self): return self.getEngineRoot() + '\\Engine\\Build\\BatchFiles\\RunUAT.bat' diff --git a/ue4cli/Utility.py b/ue4cli/Utility.py index d3a06fd..7d7a8ff 100644 --- a/ue4cli/Utility.py +++ b/ue4cli/Utility.py @@ -33,7 +33,7 @@ def readFile(filename): with open(filename, 'rb') as f: return f.read().decode('utf-8') except OSError as e: - raise UtilityException(f'failed to read file {str(filename)} due to {type(e).__name__} {str(e)}') + raise UtilityException(f'failed to read file "{str(filename)}" due to: ({type(e).__name__}) {str(e)}') @staticmethod def writeFile(filename, data): @@ -44,7 +44,7 @@ def writeFile(filename, data): with open(filename, 'wb') as f: f.write(data.encode('utf-8')) except OSError as e: - raise UtilityException(f'failed to write file {str(filename)} due to {type(e).__name__} {str(e)}') + raise UtilityException(f'failed to write file "{str(filename)}" due to: ({type(e).__name__}) {str(e)}') @staticmethod def moveFile(src, dst): @@ -54,7 +54,7 @@ def moveFile(src, dst): try: shutil.move(src, dst) except OSError as e: - raise UtilityException(f'failed to move {str(src)} to {str(dst)} due to {type(e).__name__} {str(e)}') + raise UtilityException(f'failed to move file from "{str(src)}" to "{str(dst)}" due to: ({type(e).__name__}) {str(e)}') @staticmethod def patchFile(filename, replacements): @@ -77,7 +77,7 @@ def removeDir(path, ignore_errors=False): try: shutil.rmtree(path, ignore_errors) except OSError as e: - raise UtilityException(f'failed to remove directory {str(path)} due to {type(e).__name__} {str(e)}') + raise UtilityException(f'failed to remove directory "{str(path)}" due to: ({type(e).__name__}) {str(e)}') @staticmethod def makeDirs(name, mode=0o777, exist_ok=False): @@ -87,7 +87,7 @@ def makeDirs(name, mode=0o777, exist_ok=False): try: os.makedirs(name, mode, exist_ok) except OSError as e: - raise UtilityException(f'failed to create directory {str(name)} due to {type(e).__name__} {str(e)}') + raise UtilityException(f'failed to create directory "{str(name)}" due to: ({type(e).__name__}) {str(e)}') @staticmethod def forwardSlashes(paths): diff --git a/ue4cli/cli.py b/ue4cli/cli.py index 25261fa..a66fe35 100644 --- a/ue4cli/cli.py +++ b/ue4cli/cli.py @@ -202,9 +202,10 @@ def displayHelp(): print() def main(): + + logger = logging.getLogger(__name__) + try: - logger = logging.getLogger(__name__) - # Perform plugin detection and register our detected plugins plugins = PluginManager.getPlugins() for command in plugins: @@ -224,14 +225,18 @@ def main(): if command in SUPPORTED_COMMANDS: SUPPORTED_COMMANDS[command]['action'](manager, args) else: + # FIXME: This is the only place outside of UnrealManager... classes where we use UnrealManagerException. + # Not worth to create new Exception class for only one single case, at least not now. raise UnrealManagerException('unrecognised command "' + command + '"') + except ( UnrealManagerException, UtilityException, KeyboardInterrupt, ) as e: - Utility.printStderr('(' + type(e).__name__ + ')', str(e)) + Utility.printStderr(f'Error: ({type(e).__name__}) {str(e)}') sys.exit(1) + except BaseException as e: Utility.printStderr('Unhandled exception! Crashing...') logging.basicConfig(level=logging.DEBUG) From e58d718953499b063d196eb145078d10381a933c Mon Sep 17 00:00:00 2001 From: sleeptightAnsiC <91839286+sleeptightAnsiC@users.noreply.github.com> Date: Sun, 24 Mar 2024 16:38:51 +0100 Subject: [PATCH 8/8] fix: explicitly chain the exception when raising from 'except' block https://docs.python.org/3/tutorial/errors.html#exception-chaining https://github.com/adamrehn/ue4cli/pull/65/files/05bd9c77bead5785e8ae6b4ca425f242c90e1ead#r1536831787 --- ue4cli/JsonDataManager.py | 2 +- ue4cli/UnrealManagerBase.py | 22 ++++++++++++---------- ue4cli/Utility.py | 10 +++++----- ue4cli/cli.py | 2 +- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/ue4cli/JsonDataManager.py b/ue4cli/JsonDataManager.py index 7e94238..049f762 100644 --- a/ue4cli/JsonDataManager.py +++ b/ue4cli/JsonDataManager.py @@ -25,7 +25,7 @@ def loads(self): except json.JSONDecodeError as e: # FIXME: This is the only place outside of Utility class where we use UtilityException. # Not worth to create new Exception class for only one single case, at least not now. - raise UtilityException(f'failed to load "{str(path)}" due to: ({type(e).__name__}) {str(e)}') + raise UtilityException(f'failed to load "{str(path)}" due to: ({type(e).__name__}) {str(e)}') from e def getKey(self, key): """ diff --git a/ue4cli/UnrealManagerBase.py b/ue4cli/UnrealManagerBase.py index 6c9d301..ff60e6a 100644 --- a/ue4cli/UnrealManagerBase.py +++ b/ue4cli/UnrealManagerBase.py @@ -52,7 +52,7 @@ def setEngineRootOverride(self, rootDir): try: self.getEngineVersion() except: - raise UnrealManagerException('the specified directory does not appear to contain a valid version of the Unreal Engine.') + raise UnrealManagerException('the specified directory does not appear to contain a valid version of the Unreal Engine.') from None def clearEngineRootOverride(self): """ @@ -96,7 +96,7 @@ def getEngineVersion(self, outputFormat = 'full'): # Verify that the requested output format is valid if outputFormat not in formats: - raise UnrealManagerException('unreconised version output format "{}"'.format(outputFormat)) + raise UnrealManagerException(f'unreconised version output format "{str(outputFormat)}"') return formats[outputFormat] @@ -176,7 +176,7 @@ def getDescriptor(self, dir): try: return self.getPluginDescriptor(dir) except: - raise UnrealManagerException('could not detect an Unreal project or plugin in the directory "{}"'.format(dir)) + raise UnrealManagerException(f'could not detect an Unreal project or plugin in the directory "{str(dir)}"') from None def isProject(self, descriptor): """ @@ -355,11 +355,11 @@ def buildDescriptor(self, dir=os.getcwd(), configuration='Development', target=' # If the project or plugin is Blueprint-only, there is no C++ code to build if os.path.exists(os.path.join(dir, 'Source')) == False: - raise UnrealManagerException('Pure Blueprint {}, nothing to build.'.format(descriptorType)) + raise UnrealManagerException(f'Pure Blueprint {str(descriptorType)}, nothing to build.') # Verify that the specified build configuration is valid if configuration not in self.validBuildConfigurations(): - raise UnrealManagerException('invalid build configuration "' + configuration + '"') + raise UnrealManagerException(f'invalid build configuration "{str(configuration)}"') # Check if the user specified the `-notools` flag to opt out of building Engine tools when working with source builds unstripped = list(args) @@ -408,7 +408,7 @@ def packageProject(self, dir=os.getcwd(), configuration='Shipping', extraArgs=[] # Verify that the specified build configuration is valid if configuration not in self.validBuildConfigurations(): - raise UnrealManagerException('invalid build configuration "' + configuration + '"') + raise UnrealManagerException(f'invalid build configuration "{str(configuration)}"') # Strip out the `-NoCompileEditor` flag if the user has specified it, since the Development version # of the Editor modules for the project are needed in order to run the commandlet that cooks content @@ -538,10 +538,12 @@ def listAutomationTests(self, projectFile): # Detect if the Editor terminated abnormally (i.e. not triggered by `automation quit`) # In Unreal Engine 4.27.0, the exit method changed from RequestExit to RequestExitWithStatus if 'PlatformMisc::RequestExit(' not in logOutput.stdout and 'PlatformMisc::RequestExitWithStatus(' not in logOutput.stdout: - raise UnrealManagerException( - 'failed to retrieve the list of automation tests!' + - ' stdout was: "{}", stderr was: "{}"'.format(logOutput.stdout, logOutput.stderr) - ) + Utility.printStderr("Warning: abnormal Editor termination detected!") + Utility.printStderr("printing stdout..") + print(logOutput.stdout) + Utility.printStderr("printing stderr..") + print(logOutput.stderr) + raise UnrealManagerException('failed to retrieve the list of automation tests!') return sorted(list(tests)) diff --git a/ue4cli/Utility.py b/ue4cli/Utility.py index 7d7a8ff..d8989da 100644 --- a/ue4cli/Utility.py +++ b/ue4cli/Utility.py @@ -33,7 +33,7 @@ def readFile(filename): with open(filename, 'rb') as f: return f.read().decode('utf-8') except OSError as e: - raise UtilityException(f'failed to read file "{str(filename)}" due to: ({type(e).__name__}) {str(e)}') + raise UtilityException(f'failed to read file "{str(filename)}" due to: ({type(e).__name__}) {str(e)}') from e @staticmethod def writeFile(filename, data): @@ -44,7 +44,7 @@ def writeFile(filename, data): with open(filename, 'wb') as f: f.write(data.encode('utf-8')) except OSError as e: - raise UtilityException(f'failed to write file "{str(filename)}" due to: ({type(e).__name__}) {str(e)}') + raise UtilityException(f'failed to write file "{str(filename)}" due to: ({type(e).__name__}) {str(e)}') from e @staticmethod def moveFile(src, dst): @@ -54,7 +54,7 @@ def moveFile(src, dst): try: shutil.move(src, dst) except OSError as e: - raise UtilityException(f'failed to move file from "{str(src)}" to "{str(dst)}" due to: ({type(e).__name__}) {str(e)}') + raise UtilityException(f'failed to move file from "{str(src)}" to "{str(dst)}" due to: ({type(e).__name__}) {str(e)}') from e @staticmethod def patchFile(filename, replacements): @@ -77,7 +77,7 @@ def removeDir(path, ignore_errors=False): try: shutil.rmtree(path, ignore_errors) except OSError as e: - raise UtilityException(f'failed to remove directory "{str(path)}" due to: ({type(e).__name__}) {str(e)}') + raise UtilityException(f'failed to remove directory "{str(path)}" due to: ({type(e).__name__}) {str(e)}') from e @staticmethod def makeDirs(name, mode=0o777, exist_ok=False): @@ -87,7 +87,7 @@ def makeDirs(name, mode=0o777, exist_ok=False): try: os.makedirs(name, mode, exist_ok) except OSError as e: - raise UtilityException(f'failed to create directory "{str(name)}" due to: ({type(e).__name__}) {str(e)}') + raise UtilityException(f'failed to create directory "{str(name)}" due to: ({type(e).__name__}) {str(e)}') from e @staticmethod def forwardSlashes(paths): diff --git a/ue4cli/cli.py b/ue4cli/cli.py index a66fe35..df88040 100644 --- a/ue4cli/cli.py +++ b/ue4cli/cli.py @@ -227,7 +227,7 @@ def main(): else: # FIXME: This is the only place outside of UnrealManager... classes where we use UnrealManagerException. # Not worth to create new Exception class for only one single case, at least not now. - raise UnrealManagerException('unrecognised command "' + command + '"') + raise UnrealManagerException(f'unrecognised command "{str(command)}"') from None except ( UnrealManagerException,