U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: U-Boot Mailing List <u-boot@lists.denx.de>
Cc: Simon Glass <sjg@chromium.org>
Subject: [PATCH v2 05/38] patman: Allow running a tool and returning the full result
Date: Sun,  9 Jan 2022 20:13:40 -0700	[thread overview]
Message-ID: <20220110031413.1970836-6-sjg@chromium.org> (raw)
In-Reply-To: <20220110031413.1970836-1-sjg@chromium.org>

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.
 
-- 
2.34.1.575.g55b058a8bb-goog


  parent reply	other threads:[~2022-01-10  3:15 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-10  3:13 [PATCH v2 00/38] binman: Add support for bintools and missing tools Simon Glass
2022-01-10  3:13 ` [PATCH v2 01/38] Makefile: Fake external blobs by default with binman Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-10-10 10:15   ` Rasmus Villemoes
2022-10-10 14:25     ` Tom Rini
2022-10-10 15:19     ` Simon Glass
2022-10-10 15:24       ` Tom Rini
2022-10-10 15:33         ` Simon Glass
2022-10-10 15:36           ` Tom Rini
2022-10-10 15:43             ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 02/38] binman: Tweak elf tests for a toolchain change Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 03/38] mkimage: Show the external-offset error Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 04/38] binman: Expand the external FIT test a little Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` Simon Glass [this message]
2022-01-26 15:37   ` [PATCH v2 05/38] patman: Allow running a tool and returning the full result Simon Glass
2022-01-10  3:13 ` [PATCH v2 06/38] buildman: Move the download function to tools Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 07/38] patman: Tidy up the download function a little Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 08/38] patman: Add a function to find a tool on the path Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 09/38] binman: Write fake blobs to the output directory Simon Glass
2022-01-10  3:13 ` [PATCH v2 10/38] binman: Drop the image name from the fake-blob message Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 11/38] binman: Allow faked blobs in blob-ext-list Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 12/38] binman: Correct path for fip_util Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 13/38] binman: Add installation instructions Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 14/38] binman: Add support for bintools Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 15/38] binman: Plumb in " Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 16/38] binman: Add tests for bintool Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 17/38] binman: Add a bintool implementation for cbfstool Simon Glass
2022-01-26 15:37   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 18/38] binman: Add a bintool implementation for fiptool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 19/38] binman: Add a bintool implementation for futility Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 20/38] binman: Add a bintool implementation for ifwitool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 21/38] binman: Add a bintool implementation for mkimage Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 22/38] binman: Enable bintool tests including cmdline processing Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 23/38] binman: Convert to using the CBFS bintool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:13 ` [PATCH v2 24/38] binman: Convert to using the FIP bintool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 25/38] binman: Convert to using the futility bintool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 26/38] binman: Convert to using the ifwitool bintool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 27/38] binman: Convert to using the mkimage bintool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 28/38] binman: Move compression into binman Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 29/38] binman: Tidy up pylint warnings in comp_util Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 30/38] binman: Add a bintool implementation for lz4 Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 31/38] binman: Convert to using the lz4 bintool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 32/38] binman: Add a bintool implementation for lzma_alone Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 33/38] binman: Convert to using the lzma_alone bintool Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 34/38] binman: Plumb in support for missing bintools Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 35/38] binman: Complete test coverage of comp_util Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 36/38] binman: Add a command to generate bintool docs Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 37/38] binman: Add documentation for bintools Simon Glass
2022-01-26 15:36   ` Simon Glass
2022-01-10  3:14 ` [PATCH v2 38/38] RFC: Move Odroid-C2 to use binman to produce the image Simon Glass
2022-01-21  2:02 ` [PATCH v2 00/38] binman: Add support for bintools and missing tools Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220110031413.1970836-6-sjg@chromium.org \
    --to=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox