[PATCH v3] binman: bintool: Add support for tool directories

* [PATCH v3] binman: bintool: Add support for tool directories
@ 2023-02-24 11:51 Neha Malcom Francis
  2023-03-11  1:47 ` Simon Glass
  0 siblings, 1 reply; 4+ messages in thread
From: Neha Malcom Francis @ 2023-02-24 11:51 UTC (permalink / raw)
  To: u-boot, sjg, alpernebiyasak, vigneshr, rogerq; +Cc: n-francis, afd, trini

Currently, bintool supports external compilable tools as single
executable files. Adding support for git repos that can be used to run
non-compilable scripting tools that cannot otherwise be present in
binman.

Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
---
Changes in v3:
	- moved back to using DOWNLOAD_DIR as community is making
	  relevant changes
	- extended coverage for bintool_test.py
	- added function comment for new parameter

Changes in v2:
	- added parameter to obtain path to download the directory
	  optionally, enables flexibility to avoid using
	  DOWNLOAD_DESTDIR
	- added test to bintool_test.py
	- s/FETCH_NO_BUILD/FETCH_SOURCE
	- code reformatting

 tools/binman/bintool.py        | 47 +++++++++++++++++++++++++++++-----
 tools/binman/bintool_test.py   | 43 +++++++++++++++++++++++++++++++
 tools/binman/btool/_testing.py |  4 +++
 tools/patman/tools.py          |  2 +-
 4 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
index 8fda13ff01..33f563c46f 100644
--- a/tools/binman/bintool.py
+++ b/tools/binman/bintool.py
@@ -32,12 +32,13 @@ FORMAT = '%-16.16s %-12.12s %-26.26s %s'
 modules = {}
 
 # Possible ways of fetching a tool (FETCH_COUNT is number of ways)
-FETCH_ANY, FETCH_BIN, FETCH_BUILD, FETCH_COUNT = range(4)
+FETCH_ANY, FETCH_BIN, FETCH_BUILD, FETCH_SOURCE, FETCH_COUNT = range(5)
 
 FETCH_NAMES = {
     FETCH_ANY: 'any method',
     FETCH_BIN: 'binary download',
-    FETCH_BUILD: 'build from source'
+    FETCH_BUILD: 'build from source',
+    FETCH_SOURCE: 'download source without building'
     }
 
 # Status of tool fetching
@@ -201,12 +202,13 @@ class Bintool:
                 print(f'- trying method: {FETCH_NAMES[try_method]}')
                 result = try_fetch(try_method)
                 if result:
+                    method = try_method
                     break
         else:
             result = try_fetch(method)
         if not result:
             return FAIL
-        if result is not True:
+        if result is not True and method != FETCH_SOURCE:
             fname, tmpdir = result
             dest = os.path.join(DOWNLOAD_DESTDIR, self.name)
             print(f"- writing to '{dest}'")
@@ -261,7 +263,7 @@ class Bintool:
                 show_status(col.RED, 'Failures', status[FAIL])
         return not status[FAIL]
 
-    def run_cmd_result(self, *args, binary=False, raise_on_error=True):
+    def run_cmd_result(self, *args, binary=False, raise_on_error=True, add_name=True):
         """Run the bintool using command-line arguments
 
         Args:
@@ -270,6 +272,7 @@ class Bintool:
             binary (bool): True to return output as bytes instead of str
             raise_on_error (bool): True to raise a ValueError exception if the
                 tool returns a non-zero return code
+            add_name (bool): True to add bintool name to the beginning of command
 
         Returns:
             CommandResult: Resulting output from the bintool, or None if the
@@ -278,7 +281,10 @@ class Bintool:
         if self.name in self.missing_list:
             return None
         name = os.path.expanduser(self.name)  # Expand paths containing ~
-        all_args = (name,) + args
+        if add_name:
+            all_args = (name,) + args
+        else:
+            all_args = args
         env = tools.get_env_with_path()
         tout.detail(f"bintool: {' '.join(all_args)}")
         result = command.run_pipe(
@@ -304,18 +310,19 @@ class Bintool:
             tout.debug(result.stderr)
         return result
 
-    def run_cmd(self, *args, binary=False):
+    def run_cmd(self, *args, binary=False, add_name=True):
         """Run the bintool using command-line arguments
 
         Args:
             args (list of str): Arguments to provide, in addition to the bintool
                 name
             binary (bool): True to return output as bytes instead of str
+            add_name (bool): True to add bintool name to the beginning of command
 
         Returns:
             str or bytes: Resulting stdout from the bintool
         """
-        result = self.run_cmd_result(*args, binary=binary)
+        result = self.run_cmd_result(*args, binary=binary, add_name=add_name)
         if result:
             return result.stdout
 
@@ -354,6 +361,32 @@ class Bintool:
             return None
         return fname, tmpdir
 
+    @classmethod
+    def fetch_from_git(cls, git_repo, name):
+        """Fetch a bintool git repo
+
+        This clones the repo and returns
+
+        Args:
+            git_repo (str): URL of git repo
+            name (str): Bintool name assigned as tool directory name
+
+        Returns:
+            str: Directory of fetched repo
+            or None on error
+        """
+        dir = os.path.join(DOWNLOAD_DESTDIR, name)
+        if os.path.exists(dir):
+            print(f"- {dir} repo already exists")
+            return None
+        os.mkdir(dir)
+        print(f"- clone git repo '{git_repo}' to '{dir}'")
+        tools.run('git', 'clone', '--depth', '1', git_repo, dir)
+        if len(os.listdir(dir)) == 0:
+            print(f"- '{dir}' repo is empty")
+            return None
+        return dir
+
     @classmethod
     def fetch_from_url(cls, url):
         """Fetch a bintool from a URL
diff --git a/tools/binman/bintool_test.py b/tools/binman/bintool_test.py
index 7efb8391db..e391744289 100644
--- a/tools/binman/bintool_test.py
+++ b/tools/binman/bintool_test.py
@@ -258,6 +258,36 @@ class TestBintool(unittest.TestCase):
         fname = os.path.join(self._indir, '_testing')
         return fname if write_file else self.fname, stdout.getvalue()
 
+    def check_fetch_source_method(self, write_file):
+        """Check output from fetching using SOURCE method
+
+        Args:
+            write_file (bool): True to write to output directory
+
+        Returns:
+            tuple:
+                str: Filename of directory created
+                str: Contents of stdout
+        """
+        def fake_run(*cmd):
+            if cmd[0] == 'git':
+                # See Bintool.fetch_from_git()
+                tmpdir = cmd[5]
+                self.fname = os.path.join(tmpdir, 'gitfile')
+                if write_file:
+                    tools.write_file(self.fname, b'hello')
+
+        btest = Bintool.create('_testing')
+        col = terminal.Color()
+        self.dirname = None
+        with unittest.mock.patch.object(bintool, 'DOWNLOAD_DESTDIR',
+                                        self._indir):
+            with unittest.mock.patch.object(tools, 'run', side_effect=fake_run):
+                with test_util.capture_sys_output() as (stdout, _):
+                    btest.fetch_tool(bintool.FETCH_SOURCE, col, False)
+        dirname = os.path.join(self._indir, 'mygit')
+        return self.fname if write_file else dirname, stdout.getvalue()
+
     def test_build_method(self):
         """Test fetching using the build method"""
         fname, stdout = self.check_build_method(write_file=True)
@@ -270,6 +300,17 @@ class TestBintool(unittest.TestCase):
         self.assertFalse(os.path.exists(fname))
         self.assertIn(f"File '{fname}' was not produced", stdout)
 
+    def test_fetch_source_method(self):
+        """Test fetching using the source method"""
+        dirname, stdout = self.check_fetch_source_method(write_file=True)
+        self.assertTrue(os.path.exists(dirname))
+
+    def test_fetch_source_method_fail(self):
+        """Test fetching using the source method when directory is empty"""
+        dirname, stdout = self.check_fetch_source_method(write_file=False)
+        self.assertEqual(len(os.listdir(dirname)), 0)
+        self.assertIn(f"'{dirname}' repo is empty", stdout)
+
     def test_install(self):
         """Test fetching using the install method"""
         btest = Bintool.create('_testing')
@@ -347,6 +388,8 @@ class TestBintool(unittest.TestCase):
         btool = Bintool.create('_testing')
         result = btool.run_cmd_result('fred')
         self.assertIsNone(result)
+        result = btool.run_cmd_result('daphne', add_name=False)
+        self.assertIsNone(result)
 
 
 if __name__ == "__main__":
diff --git a/tools/binman/btool/_testing.py b/tools/binman/btool/_testing.py
index 4005e8a8a5..b291d35c58 100644
--- a/tools/binman/btool/_testing.py
+++ b/tools/binman/btool/_testing.py
@@ -5,6 +5,8 @@
 
 This is not a real bintool, just one used for testing"""
 
+import tempfile
+
 from binman import bintool
 
 # pylint: disable=C0103
@@ -33,4 +35,6 @@ class Bintool_testing(bintool.Bintool):
             return self.fetch_from_drive('junk')
         if method == bintool.FETCH_BUILD:
             return self.build_from_git('url', 'target', 'pathname')
+        if method == bintool.FETCH_SOURCE:
+            return self.fetch_from_git('giturl', 'mygit')
         return None
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index 2ac814d476..b69a651eab 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -397,7 +397,7 @@ def tool_find(name):
         paths += tool_search_paths
     for path in paths:
         fname = os.path.join(path, name)
-        if os.path.isfile(fname) and os.access(fname, os.X_OK):
+        if (os.path.isfile(fname) or os.path.isdir(fname)) and os.access(fname, os.X_OK):
             return fname
 
 def run(name, *args, **kwargs):
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread