[PATCH 0/5] binman: Tidy up the location for bintools

This series makes the directory used for downloaded bintools configurable and changes the default to something that is unlikely to be unintentionally on the user's path. See [1] for discussion relating to this.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230210135941.1.I81a5b410c...
Simon Glass (5): binman: Correct an 'aot' typo binman: Update bintools documentation binman: Move the tools directory into the Bintool class binman: Use a private directory for bintools binman: Make the tooldir configurable
tools/binman/binman.rst | 19 ++++++++-- tools/binman/bintool.py | 15 +++++--- tools/binman/bintool_test.py | 15 +++++--- tools/binman/bintools.rst | 70 ++++++++++++++++++++++++++++++++++++ tools/binman/cmdline.py | 6 +++- tools/binman/control.py | 10 ++++-- tools/binman/ftest.py | 21 +++++++++-- 7 files changed, 139 insertions(+), 17 deletions(-)

Fix this typo.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/bintool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 8fda13ff012..e1dff9aa1b5 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -389,7 +389,7 @@ class Bintool:
@classmethod def apt_install(cls, package): - """Install a bintool using the 'aot' tool + """Install a bintool using the 'apt' tool
This requires use of servo so may request a password

This was not regenerated with recent changes. Update it.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/bintools.rst | 70 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+)
diff --git a/tools/binman/bintools.rst b/tools/binman/bintools.rst index edb373ab59b..c30e7eb9ff5 100644 --- a/tools/binman/bintools.rst +++ b/tools/binman/bintools.rst @@ -10,6 +10,20 @@ binaries. It is fairly easy to create new bintools. Just add a new file to the
+Bintool: bzip2: Compression/decompression using the bzip2 algorithm +------------------------------------------------------------------- + +This bintool supports running `bzip2` to compress and decompress data, as +used by binman. + +It is also possible to fetch the tool, which uses `apt` to install it. + +Documentation is available via:: + + man bzip2 + + + Bintool: cbfstool: Coreboot filesystem (CBFS) tool --------------------------------------------------
@@ -58,6 +72,20 @@ See `Chromium OS vboot documentation`_ for more information.
+Bintool: gzip: Compression/decompression using the gzip algorithm +----------------------------------------------------------------- + +This bintool supports running `gzip` to compress and decompress data, as +used by binman. + +It is also possible to fetch the tool, which uses `apt` to install it. + +Documentation is available via:: + + man gzip + + + Bintool: ifwitool: Handles the 'ifwitool' tool ----------------------------------------------
@@ -101,6 +129,20 @@ Documentation is available via::
+Bintool: lzop: Compression/decompression using the lzop algorithm +----------------------------------------------------------------- + +This bintool supports running `lzop` to compress and decompress data, as +used by binman. + +It is also possible to fetch the tool, which uses `apt` to install it. + +Documentation is available via:: + + man lzop + + + Bintool: mkimage: Image generation for U-Boot ---------------------------------------------
@@ -113,3 +155,31 @@ Support is provided for fetching this on Debian-like systems, using apt.
+Bintool: xz: Compression/decompression using the xz algorithm +------------------------------------------------------------- + +This bintool supports running `xz` to compress and decompress data, as +used by binman. + +It is also possible to fetch the tool, which uses `apt` to install it. + +Documentation is available via:: + + man xz + + + +Bintool: zstd: Compression/decompression using the zstd algorithm +----------------------------------------------------------------- + +This bintool supports running `zstd` to compress and decompress data, as +used by binman. + +It is also possible to fetch the tool, which uses `apt` to install it. + +Documentation is available via:: + + man zstd + + +

We want to be able to change this directory. Use a class member to hold the value, since changing a constant is not good.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/bintool.py | 7 ++++--- tools/binman/bintool_test.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index e1dff9aa1b5..302161fcb47 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -43,8 +43,6 @@ FETCH_NAMES = { # Status of tool fetching FETCHED, FAIL, PRESENT, STATUS_COUNT = range(4)
-DOWNLOAD_DESTDIR = os.path.join(os.getenv('HOME'), 'bin') - class Bintool: """Tool which operates on binaries to help produce entry contents
@@ -53,6 +51,9 @@ class Bintool: # List of bintools to regard as missing missing_list = []
+ # Directory to store tools + tooldir = os.path.join(os.getenv('HOME'), 'bin') + def __init__(self, name, desc, version_regex=None, version_args='-V'): self.name = name self.desc = desc @@ -208,7 +209,7 @@ class Bintool: return FAIL if result is not True: fname, tmpdir = result - dest = os.path.join(DOWNLOAD_DESTDIR, self.name) + dest = os.path.join(self.tooldir, self.name) print(f"- writing to '{dest}'") shutil.move(fname, dest) if tmpdir: diff --git a/tools/binman/bintool_test.py b/tools/binman/bintool_test.py index 7efb8391db2..57e866eff93 100644 --- a/tools/binman/bintool_test.py +++ b/tools/binman/bintool_test.py @@ -139,7 +139,7 @@ class TestBintool(unittest.TestCase): dest_fname = os.path.join(destdir, '_testing') self.seq = 0
- with unittest.mock.patch.object(bintool, 'DOWNLOAD_DESTDIR', destdir): + with unittest.mock.patch.object(bintool.Bintool, 'tooldir', destdir): with unittest.mock.patch.object(tools, 'download', side_effect=handle_download): with test_util.capture_sys_output() as (stdout, _): @@ -250,7 +250,7 @@ class TestBintool(unittest.TestCase): btest = Bintool.create('_testing') col = terminal.Color() self.fname = None - with unittest.mock.patch.object(bintool, 'DOWNLOAD_DESTDIR', + with unittest.mock.patch.object(bintool.Bintool, 'tooldir', self._indir): with unittest.mock.patch.object(tools, 'run', side_effect=fake_run): with test_util.capture_sys_output() as (stdout, _):

At present binman writes tools into the ~/bin directory. This is convenient but some may be concerned about downloading unverified binaries and running them. Place then in a special ~/.binman-tools directory instead.
Mention this in the documentation.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/binman/binman.rst | 2 ++ tools/binman/bintool.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 2bcb7d3886f..29034da92f1 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1407,6 +1407,8 @@ 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. + Bintool Documentation =====================
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 302161fcb47..6ca3d886200 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'), 'bin') + tooldir = os.path.join(os.getenv('HOME'), '.binman-tools')
def __init__(self, name, desc, version_regex=None, version_args='-V'): self.name = name

On Fri, Feb 17, 2023 at 04:19:22PM -0700, Simon Glass wrote:
At present binman writes tools into the ~/bin directory. This is convenient but some may be concerned about downloading unverified binaries and running them. Place then in a special ~/.binman-tools directory instead.
Mention this in the documentation.
Signed-off-by: Simon Glass sjg@chromium.org
Thanks for following up here.
Reviewed-by: Tom Rini trini@konsulko.com

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
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): + os.makedirs(self.tooldir) print(f"- writing to '{dest}'") shutil.move(fname, dest) if tmpdir: diff --git a/tools/binman/bintool_test.py b/tools/binman/bintool_test.py index 57e866eff93..39e4fb13e92 100644 --- a/tools/binman/bintool_test.py +++ b/tools/binman/bintool_test.py @@ -134,8 +134,10 @@ class TestBintool(unittest.TestCase): dirname = os.path.join(self._indir, 'download_dir') os.mkdir(dirname) fname = os.path.join(dirname, 'downloaded') + + # Rely on bintool to create this directory destdir = os.path.join(self._indir, 'dest_dir') - os.mkdir(destdir) + dest_fname = os.path.join(destdir, '_testing') self.seq = 0
@@ -344,8 +346,11 @@ class TestBintool(unittest.TestCase):
def test_failed_command(self): """Check that running a command that does not exist returns None""" - btool = Bintool.create('_testing') - result = btool.run_cmd_result('fred') + destdir = os.path.join(self._indir, 'dest_dir') + os.mkdir(destdir) + with unittest.mock.patch.object(bintool.Bintool, 'tooldir', destdir): + btool = Bintool.create('_testing') + result = btool.run_cmd_result('fred') self.assertIsNone(result)
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 986d6f1a315..4eed3073dce 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -7,6 +7,7 @@
import argparse from argparse import ArgumentParser +import os from binman import state
def make_extract_parser(subparsers): @@ -80,8 +81,11 @@ controlled by a description in the board device tree.''' help='Enabling debugging (provides a full traceback on error)') parser.add_argument('-H', '--full-help', action='store_true', default=False, help='Display the README file') + parser.add_argument('--tooldir', type=str, + default=os.path.join(os.getenv('HOME'), '.binman-tools'), + help='Set the directory to store tools') parser.add_argument('--toolpath', type=str, action='append', - help='Add a path to the directories containing tools') + help='Add a path to the list of directories containing tools') parser.add_argument('-T', '--threads', type=int, default=None, help='Number of threads to use (0=single-thread)') parser.add_argument('--test-section-timeout', action='store_true', diff --git a/tools/binman/control.py b/tools/binman/control.py index e64740094f6..abe01b76773 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -650,6 +650,14 @@ def Binman(args): from binman.image import Image from binman import state
+ tool_paths = [] + if args.toolpath: + tool_paths += args.toolpath + if args.tooldir: + tool_paths.append(args.tooldir) + tools.set_tool_paths(tool_paths or None) + bintool.Bintool.set_tool_dir(args.tooldir) + if args.cmd in ['ls', 'extract', 'replace', 'tool']: try: tout.init(args.verbosity) @@ -667,7 +675,6 @@ def Binman(args): allow_resize=not args.fix_size, write_map=args.map)
if args.cmd == 'tool': - tools.set_tool_paths(args.toolpath) if args.list: bintool.Bintool.list_all() elif args.fetch: @@ -719,7 +726,6 @@ def Binman(args): try: tools.set_input_dirs(args.indir) tools.prepare_output_dir(args.outdir, args.preserve) - tools.set_tool_paths(args.toolpath) state.SetEntryArgs(args.entry_arg) state.SetThreads(args.threads)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 062f54adb0e..7fe0ba4f3aa 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1750,7 +1750,7 @@ class TestFunctional(unittest.TestCase):
def _HandleGbbCommand(self, pipe_list): """Fake calls to the futility utility""" - if pipe_list[0][0] == 'futility': + if 'futility' in pipe_list[0][0]: fname = pipe_list[0][-1] # Append our GBB data to the file, which will happen every time the # futility command is called. @@ -1812,7 +1812,7 @@ class TestFunctional(unittest.TestCase): self._hash_data is False, it writes VBLOCK_DATA, else it writes a hash of the input data (here, 'input.vblock'). """ - if pipe_list[0][0] == 'futility': + if 'futility' in pipe_list[0][0]: fname = pipe_list[0][3] with open(fname, 'wb') as fd: if self._hash_data: @@ -6386,6 +6386,23 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertEqual(['u-boot', 'atf-2'], fdt_util.GetStringList(node, 'loadables'))
+ def testTooldir(self): + """Test that we can specify the tooldir""" + with test_util.capture_sys_output() as (stdout, stderr): + self.assertEqual(0, self._DoBinman('--tooldir', 'fred', + 'tool', '-l')) + self.assertEqual('fred', bintool.Bintool.tooldir) + + # Check that the toolpath is updated correctly + self.assertEqual(['fred'], tools.tool_search_paths) + + # Try with a few toolpaths; the tooldir should be at the end + with test_util.capture_sys_output() as (stdout, stderr): + self.assertEqual(0, self._DoBinman( + '--toolpath', 'mary', '--toolpath', 'anna', '--tooldir', 'fred', + 'tool', '-l')) + self.assertEqual(['mary', 'anna', 'fred'], tools.tool_search_paths) +
if __name__ == "__main__": unittest.main()

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

Hi Quentin,
On Mon, 20 Feb 2023 at 04:28, Quentin Schulz quentin.schulz@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@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
participants (3)
-
Quentin Schulz
-
Simon Glass
-
Tom Rini