From 5417da574e058efc66b471a3286cef5ac3bba1e1 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sun, 9 Jan 2022 20:13:58 -0700
Subject: [PATCH] binman: Convert to using the CBFS bintool

Update the CBFS tests to use this bintool, instead of running cbfstool
directly. This simplifies the overall code and provides more consistency,
as well as supporting missing bintools.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/cbfs_util.py      | 24 ----------------
 tools/binman/cbfs_util_test.py | 51 +++++++++++++---------------------
 2 files changed, 19 insertions(+), 56 deletions(-)

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index 39973371b9..00664bcf43 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -861,27 +861,3 @@ class CbfsReader(object):
                 val += data[:pos]
                 break
         return val.decode('utf-8')
-
-
-def cbfstool(fname, *cbfs_args, **kwargs):
-    """Run cbfstool with provided arguments
-
-    If the tool fails then this function raises an exception and prints out the
-    output and stderr.
-
-    Args:
-        fname: Filename of CBFS
-        *cbfs_args: List of arguments to pass to cbfstool
-
-    Returns:
-        CommandResult object containing the results
-    """
-    args = ['cbfstool', fname] + list(cbfs_args)
-    if kwargs.get('base') is not None:
-        args += ['-b', '%#x' % kwargs['base']]
-    result = command.RunPipe([args], capture=not VERBOSE,
-                             capture_stderr=not VERBOSE, raise_on_error=False)
-    if result.return_code:
-        print(result.stderr, file=sys.stderr)
-        raise Exception("Failed to run (error %d): '%s'" %
-                        (result.return_code, ' '.join(args)))
diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index 2c62c8a0f8..70b42795bf 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -16,6 +16,7 @@ import struct
 import tempfile
 import unittest
 
+from binman import bintool
 from binman import cbfs_util
 from binman.cbfs_util import CbfsWriter
 from binman import elf
@@ -45,11 +46,8 @@ class TestCbfs(unittest.TestCase):
         # compressing files
         tools.PrepareOutputDir(None)
 
-        cls.have_cbfstool = True
-        try:
-            tools.Run('which', 'cbfstool')
-        except:
-            cls.have_cbfstool = False
+        cls.cbfstool = bintool.Bintool.create('cbfstool')
+        cls.have_cbfstool = cls.cbfstool.is_present()
 
         cls.have_lz4 = True
         try:
@@ -177,19 +175,19 @@ class TestCbfs(unittest.TestCase):
         if not self.have_cbfstool or not self.have_lz4:
             return None
         cbfs_fname = os.path.join(self._indir, 'test.cbfs')
-        cbfs_util.cbfstool(cbfs_fname, 'create', '-m', arch, '-s', '%#x' % size)
+        self.cbfstool.create_new(cbfs_fname, size, arch)
         if base:
             base = [(1 << 32) - size + b for b in base]
-        cbfs_util.cbfstool(cbfs_fname, 'add', '-n', 'u-boot', '-t', 'raw',
-                           '-c', compress and compress[0] or 'none',
-                           '-f', tools.GetInputFilename(
-                               compress and 'compress' or 'u-boot.bin'),
-                           base=base[0] if base else None)
-        cbfs_util.cbfstool(cbfs_fname, 'add', '-n', 'u-boot-dtb', '-t', 'raw',
-                           '-c', compress and compress[1] or 'none',
-                           '-f', tools.GetInputFilename(
-                               compress and 'compress' or 'u-boot.dtb'),
-                           base=base[1] if base else None)
+        self.cbfstool.add_raw(
+            cbfs_fname, 'u-boot',
+            tools.GetInputFilename(compress and 'compress' or 'u-boot.bin'),
+            compress[0] if compress else None,
+            base[0] if base else None)
+        self.cbfstool.add_raw(
+            cbfs_fname, 'u-boot-dtb',
+            tools.GetInputFilename(compress and 'compress' or 'u-boot.dtb'),
+            compress[1] if compress else None,
+            base[1] if base else None)
         return cbfs_fname
 
     def _compare_expected_cbfs(self, data, cbfstool_fname):
@@ -223,18 +221,9 @@ class TestCbfs(unittest.TestCase):
         """Test failure to run cbfstool"""
         if not self.have_cbfstool:
             self.skipTest('No cbfstool available')
-        try:
-            # In verbose mode this test fails since stderr is not captured. Fix
-            # this by turning off verbosity.
-            old_verbose = cbfs_util.VERBOSE
-            cbfs_util.VERBOSE = False
-            with test_util.capture_sys_output() as (_stdout, stderr):
-                with self.assertRaises(Exception) as e:
-                    cbfs_util.cbfstool('missing-file', 'bad-command')
-        finally:
-            cbfs_util.VERBOSE = old_verbose
-        self.assertIn('Unknown command', stderr.getvalue())
-        self.assertIn('Failed to run', str(e.exception))
+        with self.assertRaises(ValueError) as exc:
+            out = self.cbfstool.fail()
+        self.assertIn('cbfstool missing-file bad-command', str(exc.exception))
 
     def test_cbfs_raw(self):
         """Test base handling of a Coreboot Filesystem (CBFS)"""
@@ -515,10 +504,8 @@ class TestCbfs(unittest.TestCase):
         # Compare against what cbfstool creates
         if self.have_cbfstool:
             cbfs_fname = os.path.join(self._indir, 'test.cbfs')
-            cbfs_util.cbfstool(cbfs_fname, 'create', '-m', 'x86', '-s',
-                               '%#x' % size)
-            cbfs_util.cbfstool(cbfs_fname, 'add-stage', '-n', 'u-boot',
-                               '-f', elf_fname)
+            self.cbfstool.create_new(cbfs_fname, size)
+            self.cbfstool.add_stage(cbfs_fname, 'u-boot', elf_fname)
             self._compare_expected_cbfs(data, cbfs_fname)
 
     def test_cbfs_raw_compress(self):
-- 
2.39.5