From: "Onur Özkan" <work@onurozkan.dev>
To: Guillaume Tucker <gtucker@gtucker.io>
Cc: Nathan Chancellor <nathan@kernel.org>,
Nicolas Schier <nsc@kernel.org>, Miguel Ojeda <ojeda@kernel.org>,
David Gow <davidgow@google.com>, Arnd Bergmann <arnd@arndb.de>,
linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kbuild@vger.kernel.org,
automated-testing@lists.yoctoproject.org,
workflows@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH v4 1/2] scripts: add tool to run containerized builds
Date: Thu, 22 Jan 2026 19:39:48 +0300 [thread overview]
Message-ID: <20260122193948.6af09199@nimda> (raw)
In-Reply-To: <611ce02f-2b48-4338-b37f-1df39e02da03@gtucker.io>
On Thu, 22 Jan 2026 15:59:54 +0100
Guillaume Tucker <gtucker@gtucker.io> wrote:
> Hi Onur,
>
> On 22/01/2026 15:29, Onur Özkan wrote:
> > Hi Guillaume,
> >
> > Just 2 notes from my end.
> >
> > On Thu, 22 Jan 2026 15:06:59 +0100
> > Guillaume Tucker <gtucker@gtucker.io> wrote:
> >
> >> Add a 'scripts/container' tool written in Python to run any
> >> command in the source tree from within a container. This can
> >> typically be used to call 'make' with a compiler toolchain image
> >> to run reproducible builds but any arbitrary command can be run
> >> too. Only Docker and Podman are supported in this initial version.
> >>
> >> Add a new entry to MAINTAINERS accordingly.
> >>
> >> Cc: Nathan Chancellor <nathan@kernel.org>
> >> Cc: Nicolas Schier <nsc@kernel.org>
> >> Cc: Miguel Ojeda <ojeda@kernel.org>
> >> Cc: David Gow <davidgow@google.com>
> >> Cc: "Onur Özkan" <work@onurozkan.dev>
> >> Link:
> >> https://lore.kernel.org/all/affb7aff-dc9b-4263-bbd4-a7965c19ac4e@gtucker.io/
> >> Signed-off-by: Guillaume Tucker <gtucker@gtucker.io> ---
> >> MAINTAINERS | 6 ++
> >> scripts/container | 199
> >> ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 205
> >> insertions(+) create mode 100755 scripts/container
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index da9dbc1a4019..affd55ff05e0 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -6384,6 +6384,11 @@ S: Supported
> >> F: drivers/video/console/
> >> F: include/linux/console*
> >>
> >> +CONTAINER BUILD SCRIPT
> >> +M: Guillaume Tucker <gtucker@gtucker.io>
> >> +S: Maintained
> >> +F: scripts/container
> >> +
> >> CONTEXT TRACKING
> >> M: Frederic Weisbecker <frederic@kernel.org>
> >> M: "Paul E. McKenney" <paulmck@kernel.org>
> >> @@ -13676,6 +13681,7 @@ F: scripts/Makefile*
> >> F: scripts/bash-completion/
> >> F: scripts/basic/
> >> F: scripts/clang-tools/
> >> +F: scripts/container
> >> F: scripts/dummy-tools/
> >> F: scripts/include/
> >> F: scripts/mk*
> >> diff --git a/scripts/container b/scripts/container
> >> new file mode 100755
> >> index 000000000000..09663eccb8d3
> >> --- /dev/null
> >> +++ b/scripts/container
> >> @@ -0,0 +1,199 @@
> >> +#!/usr/bin/env python3
> >> +# SPDX-License-Identifier: GPL-2.0-only
> >> +# Copyright (C) 2025 Guillaume Tucker
> >> +
> >> +"""Containerized builds"""
> >> +
> >> +import abc
> >> +import argparse
> >> +import logging
> >> +import os
> >> +import pathlib
> >> +import shutil
> >> +import subprocess
> >> +import sys
> >> +import uuid
> >> +
> >> +
> >> +class ContainerRuntime(abc.ABC):
> >> + """Base class for a container runtime implementation"""
> >> +
> >> + name = None # Property defined in each implementation class
> >> +
> >> + def __init__(self, args, logger):
> >> + self._uid = args.uid or os.getuid()
> >> + self._gid = args.gid or args.uid or os.getgid()
> >> + self._env_file = args.env_file
> >> + self._shell = args.shell
> >> + self._logger = logger
> >> +
> >> + @classmethod
> >> + def is_present(cls):
> >> + """Determine whether the runtime is present on the
> >> system"""
> >> + return shutil.which(cls.name) is not None
> >> +
> >> + @abc.abstractmethod
> >> + def _do_run(self, image, cmd, container_name):
> >> + """Runtime-specific handler to run a command in a
> >> container""" +
> >> + @abc.abstractmethod
> >> + def _do_abort(self, container_name):
> >> + """Runtime-specific handler to abort a running
> >> container""" +
> >> + def run(self, image, cmd):
> >> + """Run a command in a runtime container"""
> >> + container_name = str(uuid.uuid4())
> >> + self._logger.debug("container: %s", container_name)
> >> + try:
> >> + return self._do_run(image, cmd, container_name)
> >> + except KeyboardInterrupt:
> >> + self._logger.error("user aborted")
> >> + self._do_abort(container_name)
> >> + return 1
> >> +
> >> +
> >> +class CommonRuntime(ContainerRuntime):
> >> + """Common logic for Docker and Podman"""
> >> +
> >> + def _do_run(self, image, cmd, container_name):
> >> + cmdline = [self.name, 'run']
> >> + cmdline += self._get_opts(container_name)
> >> + cmdline.append(image)
> >> + cmdline += cmd
> >> + self._logger.debug('command: %s', ' '.join(cmdline))
> >> + return subprocess.call(cmdline)
> >> +
> >> + def _get_opts(self, container_name):
> >> + opts = [
> >> + '--name', container_name,
> >> + '--rm',
> >> + '--volume', f'{pathlib.Path.cwd()}:/src',
> >> + '--workdir', '/src',
> >> + ]
> >> + if self._env_file:
> >> + opts += ['--env-file', self._env_file]
> >> + if self._shell:
> >> + opts += ['--interactive', '--tty']
> >> + return opts
> >> +
> >> + def _do_abort(self, container_name):
> >> + subprocess.call([self.name, 'kill', container_name])
> >> +
> >> +
> >> +class DockerRuntime(CommonRuntime):
> >> + """Run a command in a Docker container"""
> >> +
> >> + name = 'docker'
> >> +
> >> + def _get_opts(self, container_name):
> >> + return super()._get_opts(container_name) + [
> >> + '--user', f'{self._uid}:{self._gid}'
> >> + ]
> >> +
> >> +
> >> +class PodmanRuntime(CommonRuntime):
> >> + """Run a command in a Podman container"""
> >> +
> >> + name = 'podman'
> >> +
> >> + def _get_opts(self, container_name):
> >> + return super()._get_opts(container_name) + [
> >> + '--userns',
> >> f'keep-id:uid={self._uid},gid={self._gid}',
> >> + ]
> >> +
> >> +
> >> +class Runtimes:
> >> + """List of all supported runtimes"""
> >> +
> >> + runtimes = [PodmanRuntime, DockerRuntime]
> >> +
> >> + @classmethod
> >> + def get_names(cls):
> >> + """Get a list of all the runtime names"""
> >> + return list(runtime.name for runtime in cls.runtimes)
> >> +
> >> + @classmethod
> >> + def get(cls, name):
> >> + """Get a single runtime class matching the given name"""
> >> + for runtime in cls.runtimes:
> >> + if runtime.name == name:
> >> + if not runtime.is_present():
> >> + raise ValueError(f"runtime not found: {name}")
> >> + return runtime
> >> + raise ValueError(f"unknown runtime: {runtime}")
> >> +
> >
> > I think you meant to use "{name}" not "{runtime}" inside ValueError.
>
> Ah yes, sorry. The parser already checks that the runtime name is in
> the list so I've never hit this error. We would probably need some
> unit tests at some point.
>
> So yes this fixes the issue, which can be reproduced with a small
> hack in the script to relax the -r option checks:
>
> --- a/scripts/container
> +++ b/scripts/container
> @@ -120,7 +120,7 @@ class Runtimes:
> if not runtime.is_present():
> raise ValueError(f"runtime not found: {name}")
> return runtime
> - raise ValueError(f"unknown runtime: {runtime}")
> + raise ValueError(f"unknown runtime: {name}")
>
> @classmethod
> def find(cls):
>
>
> Nathan, would you be OK with folding this in or should I send a v5?
>
> >> + @classmethod
> >> + def find(cls):
> >> + """Find the first runtime present on the system"""
> >> + for runtime in cls.runtimes:
> >> + if runtime.is_present():
> >> + return runtime
> >> + raise ValueError("no runtime found")
> >> +
> >
> > nit: We could extend the error message like: "Couldn't find any
> > runtime. Use -r <runtime> to specify one manually". What do you
> > think?
>
> I'm all for improving the user experience. It's good to keep the
> implementation logic separate from the command line interface though.
> Maybe this is something I could improve in a follow-up? There are a
> few other potential things to rework in this area; a more detailed
> error could be logged in main().
>
Sure, sounds good to me.
Thanks,
Onur
> >> +
> >> +def _get_logger(verbose):
> >> + """Set up a logger with the appropriate level"""
> >> + logger = logging.getLogger('container')
> >> + handler = logging.StreamHandler()
> >> + handler.setFormatter(logging.Formatter(
> >> + fmt='[container {levelname}] {message}', style='{'
> >> + ))
> >> + logger.addHandler(handler)
> >> + logger.setLevel(logging.DEBUG if verbose is True else
> >> logging.INFO)
> >> + return logger
> >> +
> >> +
> >> +def main(args):
> >> + """Main entry point for the container tool"""
> >> + logger = _get_logger(args.verbose)
> >> + try:
> >> + cls = Runtimes.get(args.runtime) if args.runtime else
> >> Runtimes.find()
> >> + except ValueError as ex:
> >> + logger.error(ex)
> >> + return 1
> >> + logger.debug("runtime: %s", cls.name)
> >> + logger.debug("image: %s", args.image)
> >> + return cls(args, logger).run(args.image, args.cmd)
> >> +
> >> +
> >> +if __name__ == '__main__':
> >> + parser = argparse.ArgumentParser(
> >> + 'container',
> >> + description="See the documentation for more details: "
> >> + "https://docs.kernel.org/dev-tools/container.html"
> >> + )
> >> + parser.add_argument(
> >> + '-e', '--env-file',
> >> + help="Path to an environment file to load in the
> >> container."
> >> + )
> >> + parser.add_argument(
> >> + '-g', '--gid',
> >> + help="Group ID to use inside the container."
> >> + )
> >> + parser.add_argument(
> >> + '-i', '--image', required=True,
> >> + help="Container image name."
> >> + )
> >> + parser.add_argument(
> >> + '-r', '--runtime', choices=Runtimes.get_names(),
> >> + help="Container runtime name. If not specified, the first
> >> one found "
> >> + "on the system will be used i.e. Podman if present,
> >> otherwise Docker."
> >> + )
> >> + parser.add_argument(
> >> + '-s', '--shell', action='store_true',
> >> + help="Run the container in an interactive shell."
> >> + )
> >> + parser.add_argument(
> >> + '-u', '--uid',
> >> + help="User ID to use inside the container. If the -g
> >> option is not "
> >> + "specified, the user ID will also be set as the group ID."
> >> + )
> >> + parser.add_argument(
> >> + '-v', '--verbose', action='store_true',
> >> + help="Enable verbose output."
> >> + )
> >> + parser.add_argument(
> >> + 'cmd', nargs='+',
> >> + help="Command to run in the container"
> >> + )
> >> + sys.exit(main(parser.parse_args(sys.argv[1:])))
> >
> > The rest LGTM.
>
> Thanks for the reviews.
>
> Cheers,
> Guillaume
next prev parent reply other threads:[~2026-01-22 16:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-22 14:06 [PATCH v4 0/2] scripts: introduce " Guillaume Tucker
2026-01-22 14:06 ` [PATCH v4 1/2] scripts: add tool to run " Guillaume Tucker
2026-01-22 14:29 ` Onur Özkan
2026-01-22 14:59 ` Guillaume Tucker
2026-01-22 16:39 ` Onur Özkan [this message]
2026-01-22 18:49 ` Nathan Chancellor
2026-01-22 19:57 ` Nicolas Schier
2026-01-22 14:07 ` [PATCH v4 2/2] Documentation: dev-tools: add container.rst page Guillaume Tucker
2026-01-22 14:22 ` Onur Özkan
2026-01-30 0:10 ` [PATCH v4 0/2] scripts: introduce containerized builds Nathan Chancellor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260122193948.6af09199@nimda \
--to=work@onurozkan.dev \
--cc=arnd@arndb.de \
--cc=automated-testing@lists.yoctoproject.org \
--cc=davidgow@google.com \
--cc=gtucker@gtucker.io \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=nathan@kernel.org \
--cc=nsc@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=workflows@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox