kernelci.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] scripts: add TOML config to container tool
       [not found]     ` <CANiq72k6T9Y_Vubih6pFMQBxju2xQq0VQkCXT1WL0qSyCXKxyA@mail.gmail.com>
@ 2026-08-29  8:56       ` Guillaume Tucker
  2026-08-29 11:01         ` Miguel Ojeda
  2026-09-04 16:29         ` Nicolas Schier
  2026-08-29  9:02       ` Guillaume Tucker
  1 sibling, 2 replies; 8+ messages in thread
From: Guillaume Tucker @ 2026-08-29  8:56 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Nathan Chancellor, Jonathan Corbet, Onur Özkan,
	Nick Desaulniers, Miguel Ojeda, linux-doc, workflows,
	linux-kernel, linux-kbuild, automated-testing,
	kernelci lists.linux.dev

Hi Miguel,

On 28/08/2026 7:35 pm, Miguel Ojeda wrote:
> On Fri, Aug 28, 2026 at 7:25 PM Guillaume Tucker <gtucker@gtucker.io> wrote:
>>
>> 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
> 
> I didn't check at the moment, but at least back when the "(optional)"
> tag was removed, it was only required for certain configurations from
> what I understand (I am not sure why the tag was removed).
> 
> In any case, it is nice if we could just say that it is required -- it
> is quite useful to have around, especially due to its standard
> library.

OK so I'll leave this part as-is for now unless someone makes a
strong case for downgrading to v3.9, thanks for confirming.

> And regarding the minimum version, it would be nice to eventually have
> a single minimum for all Python scripts, no? Debian oldstable is
> already at 3.11, so I think we could perhaps bump the minimum already.
> And Debian stable is 3.13, which from your list would be enough for
> everything on your list, right? So we could perhaps set that as the
> actual minimum for everything in the future.

I was wondering about this too, especially since v3.10 is soon going
to reach end-of-life which means no more security patches.  In
principle newer, version should be backwards compatible so if we just
bumped the minimum to v3.13 older scripts that were written for v3.9
should still work - but it'd be nice to have a way to test that.

Well that's beyond this series anyway, maybe something for KernelCI?

Guillaume


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] scripts: add TOML config to container tool
       [not found]     ` <CANiq72k6T9Y_Vubih6pFMQBxju2xQq0VQkCXT1WL0qSyCXKxyA@mail.gmail.com>
  2026-08-29  8:56       ` [PATCH 1/2] scripts: add TOML config to container tool Guillaume Tucker
@ 2026-08-29  9:02       ` Guillaume Tucker
  2026-08-29 10:57         ` Miguel Ojeda
  1 sibling, 1 reply; 8+ messages in thread
From: Guillaume Tucker @ 2026-08-29  9:02 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Nathan Chancellor, Jonathan Corbet, Onur Özkan,
	Nick Desaulniers, Miguel Ojeda, linux-doc, workflows,
	linux-kernel, linux-kbuild, automated-testing,
	kernelci lists.linux.dev

On 28/08/2026 7:35 pm, Miguel Ojeda wrote:
> On Fri, Aug 28, 2026 at 7:25 PM Guillaume Tucker<gtucker@gtucker.io> wrote:
>> 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
> I didn't check at the moment, but at least back when the "(optional)"
> tag was removed, it was only required for certain configurations from
> what I understand (I am not sure why the tag was removed).

See "bc20c56e98e0 docs: changes: better document Python needs"

Also it's explained briefly in the docs here:

   https://www.kernel.org/doc/html/latest/process/changes.html#python

So I guess that if it's needed for at least one build configuration,
then it's not optional anymore.

Guillaume


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] scripts: add TOML config to container tool
  2026-08-29  9:02       ` Guillaume Tucker
@ 2026-08-29 10:57         ` Miguel Ojeda
  0 siblings, 0 replies; 8+ messages in thread
From: Miguel Ojeda @ 2026-08-29 10:57 UTC (permalink / raw)
  To: Guillaume Tucker
  Cc: Nathan Chancellor, Jonathan Corbet, Onur Özkan,
	Nick Desaulniers, Miguel Ojeda, linux-doc, workflows,
	linux-kernel, linux-kbuild, automated-testing,
	kernelci lists.linux.dev

On Sat, Aug 29, 2026 at 11:02 AM Guillaume Tucker <gtucker@gtucker.io> wrote:
>
> See "bc20c56e98e0 docs: changes: better document Python needs"
>
> Also it's explained briefly in the docs here:
>
>    https://www.kernel.org/doc/html/latest/process/changes.html#python
>
> So I guess that if it's needed for at least one build configuration,
> then it's not optional anymore.

Yeah, that is the commit I was referring to, but I am not sure what
that commit meant, i.e. if that were the case, then e.g. Rust would be
mandatory too since it is required for at least one build
configuration... :)

I guess the commit was trying to say "used in enough places" or
perhaps "the default config of at least one architecture" or "some
`make` targets" or a combination of all those, but it is not clear.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] scripts: add TOML config to container tool
  2026-08-29  8:56       ` [PATCH 1/2] scripts: add TOML config to container tool Guillaume Tucker
@ 2026-08-29 11:01         ` Miguel Ojeda
  2026-09-04 16:29         ` Nicolas Schier
  1 sibling, 0 replies; 8+ messages in thread
From: Miguel Ojeda @ 2026-08-29 11:01 UTC (permalink / raw)
  To: Guillaume Tucker
  Cc: Nathan Chancellor, Jonathan Corbet, Onur Özkan,
	Nick Desaulniers, Miguel Ojeda, linux-doc, workflows,
	linux-kernel, linux-kbuild, automated-testing,
	kernelci lists.linux.dev

On Sat, Aug 29, 2026 at 10:56 AM Guillaume Tucker <gtucker@gtucker.io> wrote:
>
> In
> principle newer, version should be backwards compatible so if we just
> bumped the minimum to v3.13 older scripts that were written for v3.9
> should still work - but it'd be nice to have a way to test that.

If they don't work today with e.g. v3.13, then that is something we
would need to fix regardless of the minimum.

Since some distributions offer the latest versions all the time, then
it is likely things generally work (or they are niche enough that
nobody notices).

But, yeah, it would be very nice to have tests on most scripts and
thus be able to run those with every version etc.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] scripts: add TOML config to container tool
       [not found]   ` <b45e7864-928c-4b3c-aa62-64f4f63d85c7@gtucker.io>
       [not found]     ` <CANiq72k6T9Y_Vubih6pFMQBxju2xQq0VQkCXT1WL0qSyCXKxyA@mail.gmail.com>
@ 2026-09-02 10:26     ` Guillaume Tucker
  2026-09-03 20:27       ` Nathan Chancellor
  1 sibling, 1 reply; 8+ messages in thread
From: Guillaume Tucker @ 2026-09-02 10:26 UTC (permalink / raw)
  To: Nathan Chancellor, Jonathan Corbet, Miguel Ojeda, Nicolas Schier
  Cc: linux-doc, workflows, linux-kernel, linux-kbuild,
	automated-testing, kernelci@lists.linux.dev, Nick Desaulniers,
	Onur Özkan

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@gtucker.io

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] scripts: add TOML config to container tool
  2026-09-02 10:26     ` Guillaume Tucker
@ 2026-09-03 20:27       ` Nathan Chancellor
  2026-09-07 11:28         ` Guillaume Tucker
  0 siblings, 1 reply; 8+ messages in thread
From: Nathan Chancellor @ 2026-09-03 20:27 UTC (permalink / raw)
  To: Guillaume Tucker
  Cc: Jonathan Corbet, Miguel Ojeda, Nicolas Schier, linux-doc,
	workflows, linux-kernel, linux-kbuild, automated-testing,
	kernelci@lists.linux.dev, Nick Desaulniers, Onur Özkan

On Wed, Sep 02, 2026 at 12:26:29PM +0200, Guillaume Tucker wrote:
> 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

Sure, I think this sounds fine. I tend to ignore existing issue comments
from Sashiko when it comes to accepting a series, I would always prefer
them to be handled in a separate series.

-- 
Cheers,
Nathan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] scripts: add TOML config to container tool
  2026-08-29  8:56       ` [PATCH 1/2] scripts: add TOML config to container tool Guillaume Tucker
  2026-08-29 11:01         ` Miguel Ojeda
@ 2026-09-04 16:29         ` Nicolas Schier
  1 sibling, 0 replies; 8+ messages in thread
From: Nicolas Schier @ 2026-09-04 16:29 UTC (permalink / raw)
  To: Guillaume Tucker
  Cc: Miguel Ojeda, Nathan Chancellor, Jonathan Corbet, Onur Özkan,
	Nick Desaulniers, Miguel Ojeda, linux-doc, workflows,
	linux-kernel, linux-kbuild, automated-testing,
	kernelci lists.linux.dev

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

On Sat, Aug 29, 2026 at 10:56:47AM +0200, Guillaume Tucker wrote:
> Hi Miguel,
> 
> On 28/08/2026 7:35 pm, Miguel Ojeda wrote:
> > On Fri, Aug 28, 2026 at 7:25 PM Guillaume Tucker <gtucker@gtucker.io> wrote:
> > > 
> > > 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
> > 
> > I didn't check at the moment, but at least back when the "(optional)"
> > tag was removed, it was only required for certain configurations from
> > what I understand (I am not sure why the tag was removed).
> > 
> > In any case, it is nice if we could just say that it is required -- it
> > is quite useful to have around, especially due to its standard
> > library.
> 
> OK so I'll leave this part as-is for now unless someone makes a
> strong case for downgrading to v3.9, thanks for confirming.
> 
> > And regarding the minimum version, it would be nice to eventually have
> > a single minimum for all Python scripts, no? Debian oldstable is
> > already at 3.11, so I think we could perhaps bump the minimum already.
> > And Debian stable is 3.13, which from your list would be enough for
> > everything on your list, right? So we could perhaps set that as the
> > actual minimum for everything in the future.
> 
> I was wondering about this too, especially since v3.10 is soon going
> to reach end-of-life which means no more security patches.  In
> principle newer, version should be backwards compatible so if we just
> bumped the minimum to v3.13 older scripts that were written for v3.9
> should still work - but it'd be nice to have a way to test that.
> 
> Well that's beyond this series anyway, maybe something for KernelCI?

If I filtered the repology data correctly, bumping the minimum Python 
version to v3.11 would be problematic for these (major) distributions:

  * Alpine Linux 3.17
  * CentOS 8
  * Debian 11
  * Devuan 4.0
  * OpenMandriva Cooker
  * OpenMandriva Rolling
  * Slackware 15.0
  * Ubuntu 20.04

With the end of security support for Python 3.10, I think that is 
acceptable for a minimum version bump.

Bumping the minimum version of a tool used in scattered places in the 
tree to a version that is just 3-4 years old should not be done too 
careless, but I think for Python, I'd second bumping to v3.11.

Kind regards,
Nicolas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] scripts: add TOML config to container tool
  2026-09-03 20:27       ` Nathan Chancellor
@ 2026-09-07 11:28         ` Guillaume Tucker
  0 siblings, 0 replies; 8+ messages in thread
From: Guillaume Tucker @ 2026-09-07 11:28 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jonathan Corbet, Miguel Ojeda, Nicolas Schier, linux-doc,
	workflows, linux-kernel, linux-kbuild, automated-testing,
	kernelci@lists.linux.dev, Nick Desaulniers, Onur Özkan

On 03/09/2026 22:27, Nathan Chancellor wrote:
> On Wed, Sep 02, 2026 at 12:26:29PM +0200, Guillaume Tucker wrote:
>> 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

Sorry I meant Python 3.11 as the minimum version for this series so
we can have TOML support from the standard library.

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

Done in v4 which I just sent, will send the follow-up once this
series is all done.

>> * leave TypeError exceptions as-is in this series, improve user
>>   experience and error handling altogether in another follow-up
> 
> Sure, I think this sounds fine. I tend to ignore existing issue comments
> from Sashiko when it comes to accepting a series, I would always prefer
> them to be handled in a separate series.

Thanks for confirming, makes sense.

Latest Sashiko review for the v4:

  https://sashiko.dev/#/patchset/68b14a76a4cb2e6437c869bab948983a98acffbd.1788779245.git.gtucker%40gtucker.io

The `env_file` comment is moot as the user should be able to trust
the config file provided.  It's exactly like the classic
docker-compose.yaml env_file use case in fact.

The other comment about injecting options in the image name is also
partly moot as the arguments passed to the subprocess wouldn't work,
still I'm going to be addressing this by validating the image name in
the follow-up series so that should then get resolved anyway.

Best wishes,
Guillaume


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-07 11:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <7d8ba914cd174d716340bb2b88723bef0f330627.1787565460.git.gtucker@gtucker.io>
     [not found] ` <apG3hwaPA8f7SJeI@levanger>
     [not found]   ` <b45e7864-928c-4b3c-aa62-64f4f63d85c7@gtucker.io>
     [not found]     ` <CANiq72k6T9Y_Vubih6pFMQBxju2xQq0VQkCXT1WL0qSyCXKxyA@mail.gmail.com>
2026-08-29  8:56       ` [PATCH 1/2] scripts: add TOML config to container tool Guillaume Tucker
2026-08-29 11:01         ` Miguel Ojeda
2026-09-04 16:29         ` Nicolas Schier
2026-08-29  9:02       ` Guillaume Tucker
2026-08-29 10:57         ` Miguel Ojeda
2026-09-02 10:26     ` Guillaume Tucker
2026-09-03 20:27       ` Nathan Chancellor
2026-09-07 11:28         ` Guillaume Tucker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).