| Message ID | 20250205-b4-support-v2-1-b0ffa83bdefb@cherry.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] add basic b4 config file | expand |
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
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
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
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
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
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
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
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
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
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
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
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