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

[Simon Glass]

Hi Neha,

On Thu, 16 Feb 2023 at 03:49, Neha Malcom Francis <n-francis at ti.com> wrote:
>
> 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 at ti.com>
> ---
>  tools/binman/bintool.py | 42 ++++++++++++++++++++++++++++++++++-------
>  tools/patman/tools.py   |  2 ++
>  2 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
> index 8fda13ff01..a1e37699aa 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_NO_BUILD, 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_NO_BUILD: 'download source without building'

How about FETCH_SOURCE ?

>      }
>
>  # Status of tool fetching
> @@ -206,7 +207,7 @@ class Bintool:
>              result = try_fetch(method)
>          if not result:
>              return FAIL
> -        if result is not True:
> +        if result is not True and method != FETCH_NO_BUILD:
>              fname, tmpdir = result
>              dest = os.path.join(DOWNLOAD_DESTDIR, self.name)
>              print(f"- writing to '{dest}'")
> @@ -261,7 +262,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:
> @@ -278,7 +279,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,7 +308,7 @@ 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:
> @@ -315,7 +319,7 @@ class Bintool:
>          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 +358,30 @@ class Bintool:
>              return None
>          return fname, tmpdir
>
> +    @classmethod
> +    def fetch_from_git(cls, git_repo):
> +        """Fetch a bintool git repo
> +
> +        This clones the repo and returns
> +
> +        Args:
> +            git_repo (str): URL of git repo
> +            bintool_path (str): Relative path of the tool in the repo, after
> +                build is complete
> +
> +        Returns:
> +            str: Directory of fetched repo
> +            or None on error
> +        """
> +        dir = os.path.join(DOWNLOAD_DESTDIR, cls.name)
> +        os.mkdir(dir)
> +        print(f"- clone git repo '{git_repo}' to '{dir}'")
> +        tools.run('git', 'clone', '--depth', '1', git_repo, dir)
> +        if not os.path.exists(dir):
> +            print(f"- Repo '{dir}' was not produced")
> +            return None
> +        return dir
> +
>      @classmethod
>      def fetch_from_url(cls, url):
>          """Fetch a bintool from a URL
> diff --git a/tools/patman/tools.py b/tools/patman/tools.py
> index 2ac814d476..b8533c72b2 100644
> --- a/tools/patman/tools.py
> +++ b/tools/patman/tools.py
> @@ -399,6 +399,8 @@ def tool_find(name):
>          fname = os.path.join(path, name)
>          if os.path.isfile(fname) and os.access(fname, os.X_OK):
>              return fname
> +        if os.path.isdir(fname) and os.access(fname, os.X_OK):
> +            return fname

How come the duplication?

>
>  def run(name, *args, **kwargs):
>      """Run a tool with some arguments
> --
> 2.34.1
>

Please can you add a test to bintool_test.py ?

Regards,
Simon