Re: [PATCH 1/2] scripts: add TOML config to container tool
Guillaume Tucker <[email protected]>
| Newsgroups | gmane.linux.kernel.workflows,gmane.linux.documentation,gmane.linux.kernel,gmane.linux.kbuild.devel |
|---|---|
| Message-ID | <[email protected]> |
Hi Nicolas, On 28/08/2026 6:29 pm, Nicolas Schier wrote: > On Mon, Aug 24, 2026 at 12:05:47PM +0200, Guillaume Tucker wrote: >> Add support for a TOML configuration file to the scripts/container >> tool. This improves user experience by not having to keep passing the >> same command line options all the time or overly relying on built-in >> default values. Include the concept of 'profiles' with different >> named sections in the file to cover various use cases. >> >> Command line options take precedence over the config file, and values >> defined in profile sections take precedence over the default one. >> >> Add a -c option to override the location of the .container.toml config >> file which should otherwise be located in the current working >> directory. If not found, the file is silently ignored as it is not >> strictly required unless the -c option is used. >> >> Add a -p option to choose a particular profile section in the config >> file rather than the default. >> >> Signed-off-by: Guillaume Tucker <[email protected]> >> --- >> scripts/container | 81 ++++++++++++++++++++++++++++++++++++++++------- >> 1 file changed, 70 insertions(+), 11 deletions(-) >> >> diff --git a/scripts/container b/scripts/container >> index b05333d8530b..cf126fa13510 100755 >> --- a/scripts/container >> +++ b/scripts/container >> @@ -1,17 +1,19 @@ >> #!/usr/bin/env python3 >> # SPDX-License-Identifier: GPL-2.0-only >> -# Copyright (C) 2025 Guillaume Tucker >> +# Copyright (C) 2025-2026 Guillaume Tucker >> >> """Containerized builds""" >> >> import abc >> import argparse >> +import dataclasses >> import logging >> import os >> import pathlib >> import shutil >> import subprocess >> import sys >> +import tomllib > > Have you seen the comment from sashiko? > > | Will this unconditional import of tomllib crash the script on startup for > | users running supported Python versions like 3.9 and 3.10? > | The kernel's baseline requirement allows Python 3.9.x, but tomllib is only > | available starting in Python 3.11. > > https://sashiko.dev/#/patchset/15e16f175f59ae666036764eb03c40cdf19809c7.1787896890.git.gtucker@gtucker.io Yes, the minimum versions listed on this page are for building and running the kernel: https://www.kernel.org/doc/html/latest/process/changes.html It states Python 3.9 which was already older than the 3.10 minimum set when the container tool was merged. Also my understanding is that it's not a hard requirement for all the kernel tools, only for building and producing a functioning kernel. If I misunderstood this then I can rework the code to use .ini files until 3.11 becomes the minimum. However, looking at the whole tree: $ vermin -v --no-make-paths-absolute $(git ls-files *.py) | grep -e "3.1[0-9]" !2, 3.10 scripts/sbom/sbom/cmd_graph/deps_parser.py !2, 3.10 scripts/sbom/sbom/spdx/build.py !2, 3.10 scripts/sbom/sbom/spdx/core.py !2, 3.10 scripts/sbom/sbom/spdx/simplelicensing.py !2, 3.10 scripts/sbom/sbom/spdx/software.py !2, 3.13 tools/lib/python/abi/abi_regex.py !2, 3.10 tools/net/sunrpc/xdrgen/generators/__init__.py !2, 3.10 tools/net/sunrpc/xdrgen/generators/program.py !2, 3.10 tools/net/sunrpc/xdrgen/subcmds/source.py !2, 3.10 tools/net/sunrpc/xdrgen/xdr_ast.py !2, 3.10 tools/perf/scripts/python/mem-phys-addr.py !2, 3.10 tools/power/cpupower/bindings/python/test_raw_pylibcpupower.py !2, 3.13 tools/testing/kunit/kunit.py !2, 3.11 tools/testing/selftests/drivers/net/hw/rss_flow_label.py !2, 3.11 tools/testing/selftests/drivers/net/hw/rss_input_xfrm.py !2, 3.10 tools/verification/rvgen/rvgen/dot2k.py > (and there are some others...) Some of the other comments are a bit bogus, the uid / gid precedence logic is correct as far as I can tell. It's a matter of convention, maybe this should just be clarified a bit better in the documentation (and we may add unit tests at some point...). The comment about injecting malicious runtime options via the configuration file seems misled as the user should be able to trust the config file just like the command line. It's true that the image name itself could be sanitised for extra safety anyway but that's not something introduced by the config file. I can do this as a follow-up I guess. The comment about a missing whitespace is valid though, and the one about profiles with integer values of 0 is valid too so I'll get them fixed in a v3. By the way, I wish we could choose to get Sashiko's review as an email directly in the thread, do you know if this can be done easily? Thanks, Guillaume