Hi Simon,

On 2/18/23 00:19, Simon Glass wrote:

Add a command-line argument for setting the tooldir, so that the default can be overridden. Add this directory to the toolpath automatically. Create the directory if it does not already exist.

Put the default in the argument parser instead of the class, so that it is more obvious.

Update a few tests that expect the utility name to be provided without any path (e.g. 'futility'), so they can accept a path, e.g. /path/to/futility

Update the documentation and add a few tests.

Improve the help for --toolpath while we are here.

Signed-off-by: Simon Glass sjg@chromium.org

tools/binman/binman.rst | 19 +++++++++++++++---- tools/binman/bintool.py | 8 +++++++- tools/binman/bintool_test.py | 11 ++++++++--- tools/binman/cmdline.py | 6 +++++- tools/binman/control.py | 10 ++++++++-- tools/binman/ftest.py | 21 +++++++++++++++++++-- 6 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 29034da92f1..3b0a9c38d72 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1407,7 +1407,15 @@ You can also use `--fetch all` to fetch all tools or `--fetch <tool>` to fetch a particular tool. Some tools are built from source code, in which case you will need to have at least the `build-essential` and `git` packages installed.

-Tools are fetched into the `~/.binman-tools` directory. +Tools are fetched into the `~/.binman-tools` directory. This directory is +automatically added to the toolpath so there is no need to use `--toolpath` to +specify it. If you want to use these tools outside binman, you may want to +add this directory to your `PATH`. For example, if you use bash, add this to +the end of `.bashrc`::

• PATH="$HOME/.binman-tools:$PATH"

+To select a custom directory, use the `--tooldir` option.

Bintool Documentation

@@ -1427,8 +1435,9 @@ Binman commands and arguments

Usage::

• binman [-h] [-B BUILD_DIR] [-D] [-H] [--toolpath TOOLPATH] [-T THREADS]

•    [--test-section-timeout] [-v VERBOSITY] [-V]
  

• binman [-h] [-B BUILD_DIR] [-D] [--tooldir TOOLDIR] [-H]

•    [--toolpath TOOLPATH] [-T THREADS] [--test-section-timeout]
  

•    [-v VERBOSITY] [-V]
      {build,bintool-docs,entry-docs,ls,extract,replace,test,tool} ...
  
  

  Binman provides the following commands:

@@ -1453,11 +1462,13 @@ Options: -D, --debug Enabling debugging (provides a full traceback on error)

+--tooldir TOOLDIR Set the directory to store tools

• -H, --full-help Display the README file

  --toolpath TOOLPATH

• Add a path to the directories containing tools

• Add a path to the list of directories containing tools

  -T THREADS, --threads THREADS Number of threads to use (0=single-thread). Note that -T0 is useful for

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 6ca3d886200..bd6555a0aac 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -52,7 +52,7 @@ class Bintool: missing_list = []

  # Directory to store tools

• tooldir = os.path.join(os.getenv('HOME'), '.binman-tools')

• tooldir = None

This default is an issue since None is not a valid path and will trigger a TypeError......

  def __init__(self, name, desc, version_regex=None, version_args='-V'):
      self.name = name

@@ -113,6 +113,10 @@ class Bintool: obj = cls(name) return obj

• @classmethod
• def set_tool_dir(cls, pathname):

•    cls.tooldir = pathname
  

• def show(self):
      """Show a line of information about a bintool"""
      if self.is_present():
  

@@ -210,6 +214,8 @@ class Bintool: if result is not True: fname, tmpdir = result dest = os.path.join(self.tooldir, self.name)

•        if not os.path.exists(self.tooldir):
  

..... here.

I can suggest "", which seems to make os.path.exists() happy, or check whether tooldir is None before checking if the path stored in the variable exists.

Also, nit: os.makedirs takes exist_ok argument to do the mkdir -p equivalent in Python AFAICT. (though "" does not make os.makedirs happy).

•            os.makedirs(self.tooldir)
  

Thanks for following up on the discussion :)

Cheers, Quentin