Re: [PATCH v1 1/2] scripts: add tool to run containerized builds

[Guillaume Tucker <gtucker@gtucker.io>]

Hi Nathan,

Thanks for the follow-up reviews, I just sent a v3.  Here's a couple
more things I didn't address in your original email.

> 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.

Yes, that could be done but I would prefer to add it incrementally
rather than in the initial version.  Handling a build output
directory outside of the source tree means that the "make O=" option
would need to be provided in coordination with the data volume e.g.:

    container --output=$HOME/my/directory:/data/output -- make O=/data/output

We could rely on a convention for /data/output but then I don't
really want to start parsing and patching the "make" arguments yet.
A workaround is to bind-mount your build directory inside the tree
and then just pass "make O=" in the container, but that comes with
some caveats too (root privilege, potential security issues).

On a related note, I did a quick experiment with a "container -C"
option to mount the source tree from elsewhere than the current
working directory as that can be done independently of the command
run inside the container.  It's needed for Docker-in-Docker as you
have to provide the path on the host to mount volumes.  Other than
that I'm not entirely sure how useful this may be so I'm leaving it
aside for now as well.

> Is there a minimum python version required for this? If not, I would
> prefer using pathlib here:
> 
>   from pathlib import Path
> 
> then
> 
>   Path.cwd()
> 

Sure, the pathlib package was added in Python 3.4 so that's ancient
enough.  I reworked this part in the v3 series and added a note in
the docs setting Python 3.10 as a reasonable minimum requirement.

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

I added something about this in the v3 docs, essentially the file is
passed as-is to the container runtime so it's not for the container
script itself to parse it.  The short answer is that it should look
like the output of the env command.

Cheers,
Guillaume