Re: [PATCH 1/2] scripts: add TOML config to container tool

[Guillaume Tucker]

On 28/08/2026 19:25, Guillaume Tucker wrote:
> 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:

[...]

>>> +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@xxxxxxxxxx

Here's the latest Sashiko review from the v3:

  https://sashiko.dev/#/patchset/0a88d9d0ebd73a9f6e72399f93705ebc7b49ae9c.1788341513.git.gtucker%40gtucker.io

The bump to Python 3.10 is being kept as discussed earlier with
Miguel and others.

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

The comments about UID and GID have been addressed as well as minor
typos with missing whitespace etc.

The issue about container image name validation will be addressed as
a follow-up since it's not introduced by this series.  I've prepared
some changes with a compliant regex to send on top already but it's
quite invasive so I thought it'd be best to get this first series
done first.

There's a new comment which seems fair enough although that's also
partly a matter of CLI options interpretation:

| Since there is no --registry command line argument to override or unset the
| configuration value, a default registry in .container.toml (like docker.io)
| will be blindly prepended to fully-qualified images provided via the -i
| option (e.g., quay.io/lib/img).
|
| This results in invalid image paths like docker.io/quay.io/lib/img and
| prevents the use of images from other registries via the command line. This
| appears to contradict the documentation's claim that command line options take
| precedence over configuration values.

I can definitely take this into account as part of the image
validation rework since it'll also be able to parse the registry part
of a fully-qualified image name.  If the registry config option is
currently seen as ambiguous, I can just drop it from this series and
consider adding it with a more robust implementation then.

There's also a new comment about handling TypeError exceptions when a
TOML config value has an invalid type (e.g. string instead of int).
This isn't really a blocker IMHO but can be handled to improve user
experience.  In fact I'd consider improving error handling overall
and factor-in an earlier comment about when no runtime is found
automatically and a more useful message could be provided.


So to recap, here's what I would suggest:

* keep Python 3.10 for this series, continue discussion about minimum
  and optional version numbers tree-wide in other threads

* drop registry config file option in v4 then add it in a follow-up
  series along with image name validation and parsing using regex

* leave TypeError exceptions as-is in this series, improve user
  experience and error handling altogether in another follow-up

Best wishes,
Guillaume