[PATCH 5/5] binman: Make the tooldir configurable

[Simon Glass]

Hi Quentin,

On Mon, 20 Feb 2023 at 04:28, Quentin Schulz
<quentin.schulz at theobroma-systems.com> wrote:
>
> 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 at 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 :)

Thanks. I sent a new version.

Regards,
Simon