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

["Antonin Godard" <antonin.godard@bootlin.com>]

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