[PATCH v2] add basic b4 config file

[PATCH v2] add basic b4 config file

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

b4[1] is a very nice tool for mail-based contribution. A config[2] file
exists to set up a few defaults. We can use it to set the To recipients
to always add, in our case the mailing list.

Because we do not have anything to check for now, disable needs-checking
so patches can be sent without running b4 prep --check.

Because we do not have any auto-to-cc support (and the implicit one
using scripts/get_maintainer.pl cannot work for us), also disable
needs-auto-to-cc so patches can be sent without running b4 prep
--auto-to-cc.

[1] https://pypi.org/project/b4/
[2] https://b4.docs.kernel.org/en/latest/config.html

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
I'm wondering if we couldn't add some per-patch check as well, checking
that the documentation builds for each for example. Could be added in a
later patch though. Any idea?
---
Changes in v2:
- add mailing list to To recipients instead of Cc recipients to match
  our contribution instructions,
- Link to v1: https://lore.kernel.org/r/20250123-b4-support-v1-1-03b4a18edfdf@cherry.de
---
 .b4-config | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.b4-config b/.b4-config
new file mode 100644
index 0000000000000000000000000000000000000000..77b28554b1952158bd8ee94a9c9439a60f3e80d2
--- /dev/null
+++ b/.b4-config
@@ -0,0 +1,3 @@
+[b4]
+  send-series-to = docs@lists.yoctoproject.org
+  prep-pre-flight-checks = disable-needs-auto-to-cc, disable-needs-checking

---
base-commit: 6b44257874858db3aa426d3e84a79c41cb4937a3
change-id: 20240524-b4-support-d5e749251bba

Best regards,
-- 
Quentin Schulz <quentin.schulz@cherry.de>

Re: [docs] [PATCH v2] add basic b4 config file

[Antonin Godard]

Hi Quentin,

On Wed Feb 5, 2025 at 3:02 PM CET, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
>
> b4[1] is a very nice tool for mail-based contribution. A config[2] file
> exists to set up a few defaults. We can use it to set the To recipients
> to always add, in our case the mailing list.
>
> Because we do not have anything to check for now, disable needs-checking
> so patches can be sent without running b4 prep --check.
>
> Because we do not have any auto-to-cc support (and the implicit one
> using scripts/get_maintainer.pl cannot work for us), also disable
> needs-auto-to-cc so patches can be sent without running b4 prep
> --auto-to-cc.
>
> [1] https://pypi.org/project/b4/
> [2] https://b4.docs.kernel.org/en/latest/config.html
>
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
> I'm wondering if we couldn't add some per-patch check as well, checking
> that the documentation builds for each for example. Could be added in a
> later patch though. Any idea?

I think this is a good idea although I think it would make more sense to build
the tip commit only, as doc builds are a bit long? Don't know if this is
possible with b4, though.

In any case we might also use your container scripts for this (which I will get
to soon hopefully!).

> ---
> Changes in v2:
> - add mailing list to To recipients instead of Cc recipients to match
>   our contribution instructions,
> - Link to v1: https://lore.kernel.org/r/20250123-b4-support-v1-1-03b4a18edfdf@cherry.de
> ---
>  .b4-config | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/.b4-config b/.b4-config
> new file mode 100644
> index 0000000000000000000000000000000000000000..77b28554b1952158bd8ee94a9c9439a60f3e80d2
> --- /dev/null
> +++ b/.b4-config
> @@ -0,0 +1,3 @@
> +[b4]
> +  send-series-to = docs@lists.yoctoproject.org
> +  prep-pre-flight-checks = disable-needs-auto-to-cc, disable-needs-checking
>
> ---
> base-commit: 6b44257874858db3aa426d3e84a79c41cb4937a3
> change-id: 20240524-b4-support-d5e749251bba
>
> Best regards,

Reviewed-by: Antonin Godard <antonin.godard@bootlin.com>

Thanks for this!
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH v2] add basic b4 config file

[Quentin Schulz]

Hi Antonin,

On 2/7/25 10:09 AM, Antonin Godard wrote:
> Hi Quentin,
> 
> On Wed Feb 5, 2025 at 3:02 PM CET, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>
>> b4[1] is a very nice tool for mail-based contribution. A config[2] file
>> exists to set up a few defaults. We can use it to set the To recipients
>> to always add, in our case the mailing list.
>>
>> Because we do not have anything to check for now, disable needs-checking
>> so patches can be sent without running b4 prep --check.
>>
>> Because we do not have any auto-to-cc support (and the implicit one
>> using scripts/get_maintainer.pl cannot work for us), also disable
>> needs-auto-to-cc so patches can be sent without running b4 prep
>> --auto-to-cc.
>>
>> [1] https://pypi.org/project/b4/
>> [2] https://b4.docs.kernel.org/en/latest/config.html
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>> ---
>> I'm wondering if we couldn't add some per-patch check as well, checking
>> that the documentation builds for each for example. Could be added in a
>> later patch though. Any idea?
> 
> I think this is a good idea although I think it would make more sense to build
> the tip commit only, as doc builds are a bit long? Don't know if this is

Ideally individual patches shouldn't break builds so that bisectability 
is guaranteed.

At the same time, prep-perpatch-check-cmd currently runs sequentially 
and Sphinx caches stuff between builds... so probably the first run 
would be the longest and subsequent ones would benefit from the cache 
(if we reuse the build output from previous patches).

> possible with b4, though.
> 

Not yet, see 
https://git.kernel.org/pub/scm/utils/b4/b4.git/tree/src/b4/ez.py#n1683

Konstantin is typically the only one really working on that, so I'm sure 
he would appreciate some patches :)

At the same time, I've done some shady stuff with b4 for poky where I 
check (rather **guess**) if the current patch is the last patch in a 
series. c.f. 
https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44

> In any case we might also use your container scripts for this (which I will get
> to soon hopefully!).
> 

That itself may be adding a lot of time to the test since we need to 
compile the container first. Additionally, remember that absolutely NO 
message should be output to stdout or stderr by the script otherwise the 
check is understood as a fail. I'm wondering if we couldn't add support 
for a PIPE between b4 and the scripts so that we can ask it to print 
stuff (e.g. "please be patient, this may take a while" and maybe even 
print some progress). For example, I briefly added some WIP support for 
patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time 
before returning something, not very user-friendly :/

Cheers,
Quentin

Re: [docs] [PATCH v2] add basic b4 config file

[Antonin Godard]

Hi Quentin,

On Fri Feb 7, 2025 at 10:26 AM CET, Quentin Schulz wrote:
> Hi Antonin,
>
> On 2/7/25 10:09 AM, Antonin Godard wrote:
>> Hi Quentin,
>> 
>> On Wed Feb 5, 2025 at 3:02 PM CET, Quentin Schulz wrote:
>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>
>>> b4[1] is a very nice tool for mail-based contribution. A config[2] file
>>> exists to set up a few defaults. We can use it to set the To recipients
>>> to always add, in our case the mailing list.
>>>
>>> Because we do not have anything to check for now, disable needs-checking
>>> so patches can be sent without running b4 prep --check.
>>>
>>> Because we do not have any auto-to-cc support (and the implicit one
>>> using scripts/get_maintainer.pl cannot work for us), also disable
>>> needs-auto-to-cc so patches can be sent without running b4 prep
>>> --auto-to-cc.
>>>
>>> [1] https://pypi.org/project/b4/
>>> [2] https://b4.docs.kernel.org/en/latest/config.html
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>>> ---
>>> I'm wondering if we couldn't add some per-patch check as well, checking
>>> that the documentation builds for each for example. Could be added in a
>>> later patch though. Any idea?
>> 
>> I think this is a good idea although I think it would make more sense to build
>> the tip commit only, as doc builds are a bit long? Don't know if this is
>
> Ideally individual patches shouldn't break builds so that bisectability 
> is guaranteed.
>
> At the same time, prep-perpatch-check-cmd currently runs sequentially 
> and Sphinx caches stuff between builds... so probably the first run 
> would be the longest and subsequent ones would benefit from the cache 
> (if we reuse the build output from previous patches).
>
>> possible with b4, though.
>> 
>
> Not yet, see 
> https://git.kernel.org/pub/scm/utils/b4/b4.git/tree/src/b4/ez.py#n1683
>
> Konstantin is typically the only one really working on that, so I'm sure 
> he would appreciate some patches :)
>
> At the same time, I've done some shady stuff with b4 for poky where I 
> check (rather **guess**) if the current patch is the last patch in a 
> series. c.f. 
> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
>
>> In any case we might also use your container scripts for this (which I will get
>> to soon hopefully!).
>> 
>
> That itself may be adding a lot of time to the test since we need to 
> compile the container first. Additionally, remember that absolutely NO 
> message should be output to stdout or stderr by the script otherwise the 
> check is understood as a fail. I'm wondering if we couldn't add support 
> for a PIPE between b4 and the scripts so that we can ask it to print 
> stuff (e.g. "please be patient, this may take a while" and maybe even 
> print some progress). For example, I briefly added some WIP support for 
> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time 
> before returning something, not very user-friendly :/

Ok, all of that sounds like we could already have a first version of this
scripts that just builds the doc locally (no container), commit-per-commit.

Also, is there an option to opt out of this easily? I wouldn't want to
discourage people to contribute because they are unable to setup the docs build,
if you see what I mean (even though I really encourage them to do so, this saves
me time).

Random idea, but the script could read an env variable that switches to a
container build rather than a local build. So the default behavior is build to
build locally but we could always opt out of it. In any case, I would appreciate
a first version of the script that just does the build locally.

Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH v2] add basic b4 config file

[Quentin Schulz]

Hi Antonin,

On 2/7/25 10:35 AM, Antonin Godard wrote:
> Hi Quentin,
> 
> On Fri Feb 7, 2025 at 10:26 AM CET, Quentin Schulz wrote:
>> Hi Antonin,
>>
>> On 2/7/25 10:09 AM, Antonin Godard wrote:
>>> Hi Quentin,
>>>
>>> On Wed Feb 5, 2025 at 3:02 PM CET, Quentin Schulz wrote:
>>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>>
>>>> b4[1] is a very nice tool for mail-based contribution. A config[2] file
>>>> exists to set up a few defaults. We can use it to set the To recipients
>>>> to always add, in our case the mailing list.
>>>>
>>>> Because we do not have anything to check for now, disable needs-checking
>>>> so patches can be sent without running b4 prep --check.
>>>>
>>>> Because we do not have any auto-to-cc support (and the implicit one
>>>> using scripts/get_maintainer.pl cannot work for us), also disable
>>>> needs-auto-to-cc so patches can be sent without running b4 prep
>>>> --auto-to-cc.
>>>>
>>>> [1] https://pypi.org/project/b4/
>>>> [2] https://b4.docs.kernel.org/en/latest/config.html
>>>>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
>>>> ---
>>>> I'm wondering if we couldn't add some per-patch check as well, checking
>>>> that the documentation builds for each for example. Could be added in a
>>>> later patch though. Any idea?
>>>
>>> I think this is a good idea although I think it would make more sense to build
>>> the tip commit only, as doc builds are a bit long? Don't know if this is
>>
>> Ideally individual patches shouldn't break builds so that bisectability
>> is guaranteed.
>>
>> At the same time, prep-perpatch-check-cmd currently runs sequentially
>> and Sphinx caches stuff between builds... so probably the first run
>> would be the longest and subsequent ones would benefit from the cache
>> (if we reuse the build output from previous patches).
>>
>>> possible with b4, though.
>>>
>>
>> Not yet, see
>> https://git.kernel.org/pub/scm/utils/b4/b4.git/tree/src/b4/ez.py#n1683
>>
>> Konstantin is typically the only one really working on that, so I'm sure
>> he would appreciate some patches :)
>>
>> At the same time, I've done some shady stuff with b4 for poky where I
>> check (rather **guess**) if the current patch is the last patch in a
>> series. c.f.
>> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
>>
>>> In any case we might also use your container scripts for this (which I will get
>>> to soon hopefully!).
>>>
>>
>> That itself may be adding a lot of time to the test since we need to
>> compile the container first. Additionally, remember that absolutely NO
>> message should be output to stdout or stderr by the script otherwise the
>> check is understood as a fail. I'm wondering if we couldn't add support
>> for a PIPE between b4 and the scripts so that we can ask it to print
>> stuff (e.g. "please be patient, this may take a while" and maybe even
>> print some progress). For example, I briefly added some WIP support for
>> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time
>> before returning something, not very user-friendly :/
> 
> Ok, all of that sounds like we could already have a first version of this
> scripts that just builds the doc locally (no container), commit-per-commit.
> 
> Also, is there an option to opt out of this easily? I wouldn't want to
> discourage people to contribute because they are unable to setup the docs build,
> if you see what I mean (even though I really encourage them to do so, this saves
> me time).
> 

If you don't run b4 prep --check before b4 send, you'll be told you 
should be running it but you can still ignore it and send the patch series.

This message isn't printed (or at least that's the intent) if 
prep-pre-flight-checks has disable-needs-checking in it. But then people 
may not be aware that they have the ability to run this check before 
sending the patches.

> Random idea, but the script could read an env variable that switches to a
> container build rather than a local build. So the default behavior is build to
> build locally but we could always opt out of it. In any case, I would appreciate

The script should not output to stderr/stdout if the check isn't a fail. 
Outside of this limitation, it's just an executable started by b4, so we 
can pretty much do whatever we want.

Which distro do we want to use by default if OCI build is selected?
Do we want to allow the user to provide their own Make targets for the 
check (e.g. not use publish, but html, or something like that).

Also, I think we should use a temporary build directory for b4 prep 
--check so that it doesn't reuse the local build directory which may 
already be modified by the user, e.g. for debugging purposes.

Cheers,
Quentin

Re: [docs] [PATCH v2] add basic b4 config file

[Richard Purdie]

On Fri, 2025-02-07 at 10:26 +0100, Quentin Schulz via
lists.yoctoproject.org wrote:
> At the same time, I've done some shady stuff with b4 for poky where I
> check (rather **guess**) if the current patch is the last patch in a 
> series. c.f. 
> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
> 
> > In any case we might also use your container scripts for this
> > (which I will get
> > to soon hopefully!).
> > 
> 
> That itself may be adding a lot of time to the test since we need to 
> compile the container first. Additionally, remember that absolutely
> NO 
> message should be output to stdout or stderr by the script otherwise
> the 
> check is understood as a fail. I'm wondering if we couldn't add
> support 
> for a PIPE between b4 and the scripts so that we can ask it to print 
> stuff (e.g. "please be patient, this may take a while" and maybe even
> print some progress). For example, I briefly added some WIP support
> for 
> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time 
> before returning something, not very user-friendly :/

My view is that any preflight checks should be relatively fast. We
could ask someone setup an autobuilder in a container and run the whole
AB test matrix but that would be unfair and crazy! :)

I'm fine with having two levels of checks but I do think we need
something relatively quick by default else nobody will use it.

Cheers,

Richard

Re: [docs] [PATCH v2] add basic b4 config file

[Antonin Godard]

On Fri Feb 7, 2025 at 10:53 AM CET, Richard Purdie wrote:
> On Fri, 2025-02-07 at 10:26 +0100, Quentin Schulz via
> lists.yoctoproject.org wrote:
>> At the same time, I've done some shady stuff with b4 for poky where I
>> check (rather **guess**) if the current patch is the last patch in a 
>> series. c.f. 
>> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
>> 
>> > In any case we might also use your container scripts for this
>> > (which I will get
>> > to soon hopefully!).
>> > 
>> 
>> That itself may be adding a lot of time to the test since we need to 
>> compile the container first. Additionally, remember that absolutely
>> NO 
>> message should be output to stdout or stderr by the script otherwise
>> the 
>> check is understood as a fail. I'm wondering if we couldn't add
>> support 
>> for a PIPE between b4 and the scripts so that we can ask it to print 
>> stuff (e.g. "please be patient, this may take a while" and maybe even
>> print some progress). For example, I briefly added some WIP support
>> for 
>> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time 
>> before returning something, not very user-friendly :/
>
> My view is that any preflight checks should be relatively fast. We
> could ask someone setup an autobuilder in a container and run the whole
> AB test matrix but that would be unfair and crazy! :)
>
> I'm fine with having two levels of checks but I do think we need
> something relatively quick by default else nobody will use it.

After a clean build, for me it takes ~10 to 15 seconds to build the documentation
in html format. Most of the time is taken to index everything, so the output
format doesn't really matter.

So, quite a long build from my perspective. Not sure there's much we can do
about it, though.

We do have sphinx-lint, which takes ~half a second for variables.rst (likely the
biggest file here). So we could perhaps run that against each file the patch
series modifies (per-commit).
We would have to solve the existing issues, though! :)
There are also areas of improvements for this linter, for example enforcing
three spaces for indents, etc. I'm willing to put some time into it, if that's
an option we consider for b4 checking.

Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH v2] add basic b4 config file

[Richard Purdie]

On Fri, 2025-02-07 at 11:28 +0100, Antonin Godard wrote:
> On Fri Feb 7, 2025 at 10:53 AM CET, Richard Purdie wrote:
> > On Fri, 2025-02-07 at 10:26 +0100, Quentin Schulz via
> > lists.yoctoproject.org wrote:
> > > At the same time, I've done some shady stuff with b4 for poky where I
> > > check (rather **guess**) if the current patch is the last patch in a 
> > > series. c.f. 
> > > https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
> > > 
> > > > In any case we might also use your container scripts for this
> > > > (which I will get
> > > > to soon hopefully!).
> > > > 
> > > 
> > > That itself may be adding a lot of time to the test since we need to 
> > > compile the container first. Additionally, remember that absolutely
> > > NO 
> > > message should be output to stdout or stderr by the script otherwise
> > > the 
> > > check is understood as a fail. I'm wondering if we couldn't add
> > > support 
> > > for a PIPE between b4 and the scripts so that we can ask it to print 
> > > stuff (e.g. "please be patient, this may take a while" and maybe even
> > > print some progress). For example, I briefly added some WIP support
> > > for 
> > > patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time 
> > > before returning something, not very user-friendly :/
> > 
> > My view is that any preflight checks should be relatively fast. We
> > could ask someone setup an autobuilder in a container and run the whole
> > AB test matrix but that would be unfair and crazy! :)
> > 
> > I'm fine with having two levels of checks but I do think we need
> > something relatively quick by default else nobody will use it.
> 
> After a clean build, for me it takes ~10 to 15 seconds to build the documentation
> in html format. Most of the time is taken to index everything, so the output
> format doesn't really matter.
> 
> So, quite a long build from my perspective. Not sure there's much we can do
> about it, though.

10-15s is fine but how much setup (time / network bandwidth / disk use)
is needed so the user can actually run that?

> We do have sphinx-lint, which takes ~half a second for variables.rst (likely the
> biggest file here). So we could perhaps run that against each file the patch
> series modifies (per-commit).
> We would have to solve the existing issues, though! :)
> There are also areas of improvements for this linter, for example enforcing
> three spaces for indents, etc. I'm willing to put some time into it, if that's
> an option we consider for b4 checking.

It is worth exploring, I just want to make sure we don't discourage contributions.

Cheers,

Richard

Re: [docs] [PATCH v2] add basic b4 config file

[Quentin Schulz]

Hi Antonin,

On 2/7/25 11:28 AM, Antonin Godard wrote:
> On Fri Feb 7, 2025 at 10:53 AM CET, Richard Purdie wrote:
>> On Fri, 2025-02-07 at 10:26 +0100, Quentin Schulz via
>> lists.yoctoproject.org wrote:
>>> At the same time, I've done some shady stuff with b4 for poky where I
>>> check (rather **guess**) if the current patch is the last patch in a
>>> series. c.f.
>>> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
>>>
>>>> In any case we might also use your container scripts for this
>>>> (which I will get
>>>> to soon hopefully!).
>>>>
>>>
>>> That itself may be adding a lot of time to the test since we need to
>>> compile the container first. Additionally, remember that absolutely
>>> NO
>>> message should be output to stdout or stderr by the script otherwise
>>> the
>>> check is understood as a fail. I'm wondering if we couldn't add
>>> support
>>> for a PIPE between b4 and the scripts so that we can ask it to print
>>> stuff (e.g. "please be patient, this may take a while" and maybe even
>>> print some progress). For example, I briefly added some WIP support
>>> for
>>> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time
>>> before returning something, not very user-friendly :/
>>
>> My view is that any preflight checks should be relatively fast. We
>> could ask someone setup an autobuilder in a container and run the whole
>> AB test matrix but that would be unfair and crazy! :)
>>
>> I'm fine with having two levels of checks but I do think we need
>> something relatively quick by default else nobody will use it.
> 
> After a clean build, for me it takes ~10 to 15 seconds to build the documentation
> in html format. Most of the time is taken to index everything, so the output
> format doesn't really matter.
> 
> So, quite a long build from my perspective. Not sure there's much we can do
> about it, though.
> 

Also, on which PC are you building that? Some people may compile on much 
less powerful PCs for example.

> We do have sphinx-lint, which takes ~half a second for variables.rst (likely the
> biggest file here). So we could perhaps run that against each file the patch
> series modifies (per-commit).

Any time you add linters, you take the risk of false positives. For 
public CI (e.g. GitLab CI), it is "fine" because you can tell people to 
ignore it. When run locally, that's a different story.

Finally, sphinx-lint only supports Python 3.8+ (and even dropped support 
for 3.8 in the main branch), we still want to build on Python 
3.6-systems, so that makes it an unlikely tool to use for us for the 
time being.

> We would have to solve the existing issues, though! :)
> There are also areas of improvements for this linter, for example enforcing
> three spaces for indents, etc. I'm willing to put some time into it, if that's
> an option we consider for b4 checking.
> 

Should we really be holding off b4 support for this?

We already cannot enforce requirements on running anything before 
sending patches with git send-email, supporting b4 doesn't need to be 
better from that side from the start, we can just start by supporting a 
different contribution workflow and build on that if and when desired.

Cheers,
Quentin

Re: [docs] [PATCH v2] add basic b4 config file

[Antonin Godard]

Hi Quentin,

On Fri Feb 7, 2025 at 11:48 AM CET, Quentin Schulz wrote:
> Hi Antonin,
>
> On 2/7/25 11:28 AM, Antonin Godard wrote:
>> On Fri Feb 7, 2025 at 10:53 AM CET, Richard Purdie wrote:
>>> On Fri, 2025-02-07 at 10:26 +0100, Quentin Schulz via
>>> lists.yoctoproject.org wrote:
>>>> At the same time, I've done some shady stuff with b4 for poky where I
>>>> check (rather **guess**) if the current patch is the last patch in a
>>>> series. c.f.
>>>> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
>>>>
>>>>> In any case we might also use your container scripts for this
>>>>> (which I will get
>>>>> to soon hopefully!).
>>>>>
>>>>
>>>> That itself may be adding a lot of time to the test since we need to
>>>> compile the container first. Additionally, remember that absolutely
>>>> NO
>>>> message should be output to stdout or stderr by the script otherwise
>>>> the
>>>> check is understood as a fail. I'm wondering if we couldn't add
>>>> support
>>>> for a PIPE between b4 and the scripts so that we can ask it to print
>>>> stuff (e.g. "please be patient, this may take a while" and maybe even
>>>> print some progress). For example, I briefly added some WIP support
>>>> for
>>>> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time
>>>> before returning something, not very user-friendly :/
>>>
>>> My view is that any preflight checks should be relatively fast. We
>>> could ask someone setup an autobuilder in a container and run the whole
>>> AB test matrix but that would be unfair and crazy! :)
>>>
>>> I'm fine with having two levels of checks but I do think we need
>>> something relatively quick by default else nobody will use it.
>> 
>> After a clean build, for me it takes ~10 to 15 seconds to build the documentation
>> in html format. Most of the time is taken to index everything, so the output
>> format doesn't really matter.
>> 
>> So, quite a long build from my perspective. Not sure there's much we can do
>> about it, though.
>> 
>
> Also, on which PC are you building that? Some people may compile on much 
> less powerful PCs for example.

You're right, I'd say I have a quite powerful PC, so I would imagine that number
could be at least doubled on a slow computer.

I think in the case where Sphinx isn't installed the script could point towards
instructions on how to install.
On top of that, the script could also print something like: "if you don't want
to run preflight checks, set 'prep-pre-flight-checks' to include
'disable-needs-checking' locally with <command/instruction>".

And so, with these two messages here, a user which sends a contribution and who
didn't compile the docs at all can choose to either take the time to do so, or
ignore and send the patch. What do you think?

>> We do have sphinx-lint, which takes ~half a second for variables.rst (likely the
>> biggest file here). So we could perhaps run that against each file the patch
>> series modifies (per-commit).
>
> Any time you add linters, you take the risk of false positives. For 
> public CI (e.g. GitLab CI), it is "fine" because you can tell people to 
> ignore it. When run locally, that's a different story.
>
> Finally, sphinx-lint only supports Python 3.8+ (and even dropped support 
> for 3.8 in the main branch), we still want to build on Python 
> 3.6-systems, so that makes it an unlikely tool to use for us for the 
> time being.

Fair points!

>> We would have to solve the existing issues, though! :)
>> There are also areas of improvements for this linter, for example enforcing
>> three spaces for indents, etc. I'm willing to put some time into it, if that's
>> an option we consider for b4 checking.
>>
>
> Should we really be holding off b4 support for this?
>
> We already cannot enforce requirements on running anything before
> sending patches with git send-email, supporting b4 doesn't need to be
> better from that side from the start, we can just start by supporting a
> different contribution workflow and build on that if and when desired.

No, I think b4 support can be added without the pre-checks, of course.

Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH v2] add basic b4 config file

[Quentin Schulz]

On 2/7/25 12:21 PM, Antonin Godard wrote:
> Hi Quentin,
> 
> On Fri Feb 7, 2025 at 11:48 AM CET, Quentin Schulz wrote:
>> Hi Antonin,
>>
>> On 2/7/25 11:28 AM, Antonin Godard wrote:
>>> On Fri Feb 7, 2025 at 10:53 AM CET, Richard Purdie wrote:
>>>> On Fri, 2025-02-07 at 10:26 +0100, Quentin Schulz via
>>>> lists.yoctoproject.org wrote:
>>>>> At the same time, I've done some shady stuff with b4 for poky where I
>>>>> check (rather **guess**) if the current patch is the last patch in a
>>>>> series. c.f.
>>>>> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
>>>>>
>>>>>> In any case we might also use your container scripts for this
>>>>>> (which I will get
>>>>>> to soon hopefully!).
>>>>>>
>>>>>
>>>>> That itself may be adding a lot of time to the test since we need to
>>>>> compile the container first. Additionally, remember that absolutely
>>>>> NO
>>>>> message should be output to stdout or stderr by the script otherwise
>>>>> the
>>>>> check is understood as a fail. I'm wondering if we couldn't add
>>>>> support
>>>>> for a PIPE between b4 and the scripts so that we can ask it to print
>>>>> stuff (e.g. "please be patient, this may take a while" and maybe even
>>>>> print some progress). For example, I briefly added some WIP support
>>>>> for
>>>>> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time
>>>>> before returning something, not very user-friendly :/
>>>>
>>>> My view is that any preflight checks should be relatively fast. We
>>>> could ask someone setup an autobuilder in a container and run the whole
>>>> AB test matrix but that would be unfair and crazy! :)
>>>>
>>>> I'm fine with having two levels of checks but I do think we need
>>>> something relatively quick by default else nobody will use it.
>>>
>>> After a clean build, for me it takes ~10 to 15 seconds to build the documentation
>>> in html format. Most of the time is taken to index everything, so the output
>>> format doesn't really matter.
>>>
>>> So, quite a long build from my perspective. Not sure there's much we can do
>>> about it, though.
>>>
>>
>> Also, on which PC are you building that? Some people may compile on much
>> less powerful PCs for example.
> 
> You're right, I'd say I have a quite powerful PC, so I would imagine that number
> could be at least doubled on a slow computer.
> 
> I think in the case where Sphinx isn't installed the script could point towards
> instructions on how to install.

We'd need more than just Sphinx to be safe, but then we'd need to 
maintain this list of Python packages somehow and test their imports. 
Same for external dependencies (librsvg and the likes).

> On top of that, the script could also print something like: "if you don't want
> to run preflight checks, set 'prep-pre-flight-checks' to include
> 'disable-needs-checking' locally with <command/instruction>".
> 

I would advise against it, I don't think it makes a lot of sense to 
modify the .b4-config in the repo especially since b4 will refuse 
sending patches if your local git is dirty. They can simply ignore the 
"you didn't run b4 prep --check" message if they want to. This is how it 
looks when you have a prep-perpatch-check-cmd but haven't run it yet:

$ b4 send --no-sign
Converted the branch to 3 messages
---
Some pre-flight checks are failing:
   - Run local checks : b4 prep --check
---
Press Enter to ignore and send anyway or Ctrl-C to abort and fix

> And so, with these two messages here, a user which sends a contribution and who
> didn't compile the docs at all can choose to either take the time to do so, or
> ignore and send the patch. What do you think?
> 

We could document properly in the docs contribution section and/or in 
the git repo README what the expectations are wrt running the checks?

Cheers,
Quentin

Re: [docs] [PATCH v2] add basic b4 config file

[Antonin Godard]

Hi Quentin,

On Fri Feb 7, 2025 at 12:35 PM CET, Quentin Schulz wrote:
> On 2/7/25 12:21 PM, Antonin Godard wrote:
>> Hi Quentin,
>> 
>> On Fri Feb 7, 2025 at 11:48 AM CET, Quentin Schulz wrote:
>>> Hi Antonin,
>>>
>>> On 2/7/25 11:28 AM, Antonin Godard wrote:
>>>> On Fri Feb 7, 2025 at 10:53 AM CET, Richard Purdie wrote:
>>>>> On Fri, 2025-02-07 at 10:26 +0100, Quentin Schulz via
>>>>> lists.yoctoproject.org wrote:
>>>>>> At the same time, I've done some shady stuff with b4 for poky where I
>>>>>> check (rather **guess**) if the current patch is the last patch in a
>>>>>> series. c.f.
>>>>>> https://git.openembedded.org/openembedded-core/tree/scripts/b4-wrapper-poky.py#n44
>>>>>>
>>>>>>> In any case we might also use your container scripts for this
>>>>>>> (which I will get
>>>>>>> to soon hopefully!).
>>>>>>>
>>>>>>
>>>>>> That itself may be adding a lot of time to the test since we need to
>>>>>> compile the container first. Additionally, remember that absolutely
>>>>>> NO
>>>>>> message should be output to stdout or stderr by the script otherwise
>>>>>> the
>>>>>> check is understood as a fail. I'm wondering if we couldn't add
>>>>>> support
>>>>>> for a PIPE between b4 and the scripts so that we can ask it to print
>>>>>> stuff (e.g. "please be patient, this may take a while" and maybe even
>>>>>> print some progress). For example, I briefly added some WIP support
>>>>>> for
>>>>>> patchtest as prep-perpatch-cmd in OE-Core, but it takes a long time
>>>>>> before returning something, not very user-friendly :/
>>>>>
>>>>> My view is that any preflight checks should be relatively fast. We
>>>>> could ask someone setup an autobuilder in a container and run the whole
>>>>> AB test matrix but that would be unfair and crazy! :)
>>>>>
>>>>> I'm fine with having two levels of checks but I do think we need
>>>>> something relatively quick by default else nobody will use it.
>>>>
>>>> After a clean build, for me it takes ~10 to 15 seconds to build the documentation
>>>> in html format. Most of the time is taken to index everything, so the output
>>>> format doesn't really matter.
>>>>
>>>> So, quite a long build from my perspective. Not sure there's much we can do
>>>> about it, though.
>>>>
>>>
>>> Also, on which PC are you building that? Some people may compile on much
>>> less powerful PCs for example.
>> 
>> You're right, I'd say I have a quite powerful PC, so I would imagine that number
>> could be at least doubled on a slow computer.
>> 
>> I think in the case where Sphinx isn't installed the script could point towards
>> instructions on how to install.
>
> We'd need more than just Sphinx to be safe, but then we'd need to 
> maintain this list of Python packages somehow and test their imports. 
> Same for external dependencies (librsvg and the likes).
>
>> On top of that, the script could also print something like: "if you don't want
>> to run preflight checks, set 'prep-pre-flight-checks' to include
>> 'disable-needs-checking' locally with <command/instruction>".
>> 
>
> I would advise against it, I don't think it makes a lot of sense to 
> modify the .b4-config in the repo especially since b4 will refuse 
> sending patches if your local git is dirty. They can simply ignore the 
> "you didn't run b4 prep --check" message if they want to. This is how it 
> looks when you have a prep-perpatch-check-cmd but haven't run it yet:
>
> $ b4 send --no-sign
> Converted the branch to 3 messages
> ---
> Some pre-flight checks are failing:
>    - Run local checks : b4 prep --check
> ---
> Press Enter to ignore and send anyway or Ctrl-C to abort and fix

Ah ok, yes, this is enough I guess.

>> And so, with these two messages here, a user which sends a contribution and who
>> didn't compile the docs at all can choose to either take the time to do so, or
>> ignore and send the patch. What do you think?
>> 
>
> We could document properly in the docs contribution section and/or in 
> the git repo README what the expectations are wrt running the checks?

Yes, I think we can document it. The contributor guide sound like a good option
to me.

Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com