Add a new function which returns the entire result from running a tool, not just stdout. Update Run() to use this and to return stdout on error, if stderr is empty, since some unfortunate tools write their error output to stdout rather than stderr.
Move building of the PATH to a separate function.
Make the exception catching more specific, to catch just ValueError, since broad exceptions are a pain to debug.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
tools/patman/tools.py | 56 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 10 deletions(-)
diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 86c4f616206..7af4a52a8fd 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -313,7 +313,22 @@ def GetTargetCompileTool(name, cross_compile=None): target_name = name return target_name, extra_args
-def Run(name, *args, **kwargs): +def get_env_with_path(): + """Get an updated environment with the PATH variable set correctly + + If there are any search paths set, these need to come first in the PATH so + that these override any other version of the tools. + + Returns: + dict: New environment with PATH updated, or None if there are not search + paths + """ + if tool_search_paths: + env = dict(os.environ) + env['PATH'] = ':'.join(tool_search_paths) + ':' + env['PATH'] + return env + +def run_result(name, *args, **kwargs): """Run a tool with some arguments
This runs a 'tool', which is a program used by binman to process files and @@ -326,6 +341,7 @@ def Run(name, *args, **kwargs): for_host: True to resolve the command to the version for the host for_target: False to run the command as-is, without resolving it to the version for the compile target + raise_on_error: Raise an error if the command fails (True by default)
Returns: CommandResult object @@ -334,10 +350,8 @@ def Run(name, *args, **kwargs): binary = kwargs.get('binary') for_host = kwargs.get('for_host', False) for_target = kwargs.get('for_target', not for_host) - env = None - if tool_search_paths: - env = dict(os.environ) - env['PATH'] = ':'.join(tool_search_paths) + ':' + env['PATH'] + raise_on_error = kwargs.get('raise_on_error', True) + env = get_env_with_path() if for_target: name, extra_args = GetTargetCompileTool(name) args = tuple(extra_args) + args @@ -349,11 +363,12 @@ def Run(name, *args, **kwargs): result = command.RunPipe([all_args], capture=True, capture_stderr=True, env=env, raise_on_error=False, binary=binary) if result.return_code: - raise ValueError("Error %d running '%s': %s" % - (result.return_code,' '.join(all_args), - result.stderr)) - return result.stdout - except: + if raise_on_error: + raise ValueError("Error %d running '%s': %s" % + (result.return_code,' '.join(all_args), + result.stderr or result.stdout)) + return result + except ValueError: if env and not PathHasFile(env['PATH'], name): msg = "Please install tool '%s'" % name package = packages.get(name) @@ -362,6 +377,27 @@ def Run(name, *args, **kwargs): raise ValueError(msg) raise
+def Run(name, *args, **kwargs): + """Run a tool with some arguments + + This runs a 'tool', which is a program used by binman to process files and + perhaps produce some output. Tools can be located on the PATH or in a + search path. + + Args: + name: Command name to run + args: Arguments to the tool + for_host: True to resolve the command to the version for the host + for_target: False to run the command as-is, without resolving it + to the version for the compile target + + Returns: + CommandResult object + """ + result = run_result(name, *args, **kwargs) + if result is not None: + return result.stdout + def Filename(fname): """Resolve a file path to an absolute path.