From ade5327655802b0149a4a569dd489318ad59adc3 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 9 Jan 2022 20:13:40 -0700
Subject: [PATCH] patman: Allow running a tool and returning the full result

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>
---
 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 86c4f61620..7af4a52a8f 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.
 
-- 
2.39.5