workflows.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Guillaume Tucker <gtucker@gtucker.io>
Cc: Miguel Ojeda <ojeda@kernel.org>,
	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,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH v1 1/2] scripts: add tool to run containerized builds
Date: Fri, 19 Dec 2025 12:47:48 -0700	[thread overview]
Message-ID: <20251219194748.GA1404325@ax162> (raw)
In-Reply-To: <97dec58ebe4161027f13f2215ed9da4a43bc8c47.1765374789.git.gtucker@gtucker.io>

Hi Guillaume,

On Wed, Dec 10, 2025 at 02:58:28PM +0100, Guillaume Tucker 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 for this initial version.
> 
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Link: https://lore.kernel.org/all/affb7aff-dc9b-4263-bbd4-a7965c19ac4e@gtucker.io/
> Signed-off-by: Guillaume Tucker <gtucker@gtucker.io>

Overall, I like the concept here. It is simple and should be relatively
easy for people to drive. I think having some short quick examples (or a
link to the Documentation file that inclues them) would be good in case
people stumble across the script first.

One initial comment (or perhaps feature request) would be handling O= /
KBUILD_OUTPUT for building out of tree. It may be a little complicated
for mounting the build directory into the container but it might make it
easier for folks who build out of tree to use.

> ---
>  scripts/container | 112 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 112 insertions(+)
>  create mode 100755 scripts/container
> 
> diff --git a/scripts/container b/scripts/container
> new file mode 100755
> index 000000000000..74644ac33685
> --- /dev/null
> +++ b/scripts/container
> @@ -0,0 +1,112 @@
> +#!/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0-only
> +# Copyright (C) 2025 Guillaume Tucker
> +
> +"""Containerized builds"""
> +
> +import argparse
> +import logging
> +import os
> +import subprocess
> +import sys
> +
> +
> +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 run_docker(args):
> +    """Run a command in a Docker container"""
> +    uid = args.uid or os.getuid()
> +    gid = args.gid or args.uid or os.getgid()
> +    cmd = [
> +        'docker', 'run',
> +        '--interactive',
> +        '--volume', f'{os.getcwd()}:/src',

Is there a minimum python version required for this? If not, I would
prefer using pathlib here:

  from pathlib import Path

then

  Path.cwd()

> +        '--workdir', '/src',
> +        '--user', f'{uid}:{gid}'
> +    ]
> +    if args.env_file:
> +        cmd += ['--env-file', args.env_file]
> +    cmd.append(args.image)
> +    cmd += args.cmd
> +    return subprocess.call(cmd)
> +
> +
> +def run_podman(args):
> +    """Run a command in a Podman container"""
> +    uid = args.uid or 1000
> +    gid = args.gid or args.uid or 1000

Why 1000 instead of using getuid() and getgid() as above?

> +    cmd = [
> +        'podman', 'run',
> +        '--interactive',
> +        '--volume', f'{os.getcwd()}:/src',
> +        '--workdir', '/src',
> +        '--userns', f'keep-id:uid={uid},gid={gid}',
> +    ]
> +    if args.env_file:
> +        cmd += ['--env-file', args.env_file]
> +    cmd.append(args.image)
> +    cmd += args.cmd
> +    return subprocess.call(cmd)

Most of these two functions are the same. Maybe they could be abstracted
into a simple class so that most of the logic could be shared between
the two implementations? That also might simplify main() a bit and make
fulfilling David's request a little simpler as well.

> +def main(args):
> +    """Main entry point for the container tool"""
> +    logger = get_logger(args.verbose)
> +    logger.debug("runtime=%s, image=%s", args.runtime, args.image)
> +    runtimes = {
> +        'docker': run_docker,
> +        'podman': run_podman,
> +    }
> +    handler = runtimes.get(args.runtime)
> +    if not handler:
> +        logger.error("Unknown container runtime: %s", args.runtime)
> +        return 1
> +    try:
> +        return handler(args)
> +    except KeyboardInterrupt:
> +        logger.error("aborted")
> +        return 1
> +
> +
> +if __name__ == '__main__':
> +    parser = argparse.ArgumentParser("Containerized builds")
> +    parser.add_argument(
> +        '-e', '--env-file',
> +        help="Path to an environment file to load in the container."
> +    )

Is there documentation for how an environment file should be formatter?

> +    parser.add_argument(
> +        '-g', '--gid',
> +        help="Group ID to use inside the container."
> +    )
> +    parser.add_argument(
> +        '-i', '--image', default='gcc',
> +        help="Container image, default is gcc."
> +    )
> +    parser.add_argument(
> +        '-r', '--runtime', choices=['docker', 'podman'], default='docker',
> +        help="Container runtime, default is docker."
> +    )
> +    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 used for 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:])))
> -- 
> 2.47.3
> 

  parent reply	other threads:[~2025-12-19 19:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-10 13:58 [PATCH v1 0/2] scripts: introduce " Guillaume Tucker
2025-12-10 13:58 ` [PATCH v1 1/2] scripts: add tool to run " Guillaume Tucker
2025-12-13  4:16   ` Guillaume Tucker
2025-12-15  9:24   ` Onur Özkan
2025-12-15 10:15     ` Guillaume Tucker
2025-12-17  9:56   ` [Automated-testing] " David Gow
2025-12-17 13:51     ` Guillaume Tucker
2025-12-19 19:47   ` Nathan Chancellor [this message]
2025-12-19 21:15     ` Nathan Chancellor
2025-12-10 13:58 ` [PATCH v1 2/2] Documentation: dev-tools: add container.rst page Guillaume Tucker

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=20251219194748.GA1404325@ax162 \
    --to=nathan@kernel.org \
    --cc=arnd@arndb.de \
    --cc=automated-testing@lists.yoctoproject.org \
    --cc=gtucker@gtucker.io \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --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