Message ID | 20250117-b4-support-v2-1-7178f61d49e4@cherry.de |
---|---|
State | New |
Headers | show |
Series | [v2] add basic support for b4 contribution workflow | expand |
On Fri Jan 17, 2025 at 11:44 AM 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 Cc recipients > to always add, in our case the mailing list. > > This also adds a wrapper script that is called by b4 to figure out which > addresses to put as Cc recipients. Considering that patches to the doc/ > directory also need to be sent to the yocto-docs mailing list, this > wrapper handles that. A limitation of the script (lsdiff actually) is > that it doesn't know how to handle empty files, but those should be > of rather rare occurrences. > > While the wrapper script should be enough by itself to add the bitbake > mailing list to the Cc recipients, one still needs to manually run b4 > prep --auto-to-cc for that to happen. Therefore, let's add an explicit > send-series-cc so that at least that mailing list is always there. > > Because we currently do not have anything to check for patch validity, > remove requirement for b4 prep --check to be run before sending a patch > series, via disable-needs-checking in prep-pre-flight-checks. > > [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> > --- > This adds a basic config file for b4 to avoid to have to add the mailing > list by hand. > For documentation patches, the additional mailing list will be added > when running b4 prep --check, as is required for each new series managed > by b4. > > Note that b4 supports patchwork, c.f. > https://b4.docs.kernel.org/en/latest/config.html#patchwork-integration-settings > though I have no clue what it does with it as I am no maintainer, but > maybe something worth having a look at if some maintainer of this repo > wants to use b4 to merge stuff? I'll add my configuration here, maybe someone or myself can add some doc for it later, here or elsewhere. configuration I use: [b4] pw-url = https://patchwork.yoctoproject.org/ pw-key = <redacted> pw-project = docs pw-review-state = under-review pw-accept-state = accepted pw-discard-state = rejected For `pw-project`, it comes from the project name on patchwork, e.g.: https://patchwork.yoctoproject.org/project/docs/list ^^^^ The pw-key can be obtained from the user's personal settings. Then `b4 {am,shazam}` will automatically set the state to under-review. `b4 ty -d` will reject the patch. Sometimes I use `b4 ty --pw-set-state superseded -d ...` when I know another version of the patch/series was sent. Finally `b4 ty -a` will also set the accepted state along the generation of thank-you file. And overall it's much easier than clicking buttons on patchwork :) > --- > Changes in v2: > - add b4-wrapper for auto-detecting patches that are to be sent to the > docs mailing list as well, > - disable forced b4 prep --check for each b4-managed series, > - Link to v1: https://lore.kernel.org/r/20240524-b4-support-v1-1-0c4334c36cc7@cherry.de > --- > .b4-config | 4 ++++ > b4-wrapper.py | 38 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/.b4-config b/.b4-config > new file mode 100644 > index 0000000000000000000000000000000000000000..0b699f204c414099f2118ffdbc9f7b24454e4a4e > --- /dev/null > +++ b/.b4-config > @@ -0,0 +1,4 @@ > +[b4] > + send-series-cc = bitbake-devel@lists.openembedded.org > + send-auto-cc-cmd = ./b4-wrapper.py send-auto-cc-cmd > + prep-pre-flight-checks = disable-needs-checking > diff --git a/b4-wrapper.py b/b4-wrapper.py > new file mode 100755 > index 0000000000000000000000000000000000000000..ddc2597bc0374fa5124a83be94eeab52df6f38d3 > --- /dev/null > +++ b/b4-wrapper.py > @@ -0,0 +1,38 @@ > +#!/usr/bin/env python3 > +# > +# This script is to be called by b4: > +# - through b4.send-auto-cc-cmd with "send-auto-cc-cmd" as first argument, > +# > +# When send-auto-cc-cmd is passed: > +# > +# This returns the list of Cc recipients for a patch. > +# > +# This script takes as stdin a patch. > + > +import subprocess > +import sys > + > +cmd = sys.argv[1] > +if cmd != "send-auto-cc-cmd": > + sys.exit(-1) > + > +patch = sys.stdin.read() > + > +if subprocess.call(["which", "lsdiff"], stdout=subprocess.DEVNULL) != 0: > + print("lsdiff missing from host, please install patchutils") > + sys.exit(-1) > + > +print("bitbake-devel@lists.openembedded.org") > + > +files = subprocess.check_output(["lsdiff", "--strip-match=1", "--strip=1", "--include=doc/*"], > + input=patch, text=True) > +if len(files): > + print("docs@lists.yoctoproject.org") > +else: > +# Handle patches made with --no-prefix > + files = subprocess.check_output(["lsdiff", "--include=doc/*"], > + input=patch, text=True) > + if len(files): > + print("docs@lists.yoctoproject.org") > + > +sys.exit(0) > > --- > base-commit: 0329a7e3ac694737f2d2c1861f65492551360663 > change-id: 20240524-b4-support-e9855fc54d40 > > Best regards, Thanks for the script Quentin! I was thinking, for eocore we could even extend the list of cc from meta/conf/distro/include/maintainers.inc. Antonin
Hi Antonin, On 1/17/25 12:01 PM, Antonin Godard wrote: > On Fri Jan 17, 2025 at 11:44 AM 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 Cc recipients >> to always add, in our case the mailing list. >> >> This also adds a wrapper script that is called by b4 to figure out which >> addresses to put as Cc recipients. Considering that patches to the doc/ >> directory also need to be sent to the yocto-docs mailing list, this >> wrapper handles that. A limitation of the script (lsdiff actually) is >> that it doesn't know how to handle empty files, but those should be >> of rather rare occurrences. >> >> While the wrapper script should be enough by itself to add the bitbake >> mailing list to the Cc recipients, one still needs to manually run b4 >> prep --auto-to-cc for that to happen. Therefore, let's add an explicit >> send-series-cc so that at least that mailing list is always there. >> >> Because we currently do not have anything to check for patch validity, >> remove requirement for b4 prep --check to be run before sending a patch >> series, via disable-needs-checking in prep-pre-flight-checks. >> >> [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> >> --- >> This adds a basic config file for b4 to avoid to have to add the mailing >> list by hand. >> For documentation patches, the additional mailing list will be added >> when running b4 prep --check, as is required for each new series managed >> by b4. >> >> Note that b4 supports patchwork, c.f. >> https://b4.docs.kernel.org/en/latest/config.html#patchwork-integration-settings >> though I have no clue what it does with it as I am no maintainer, but >> maybe something worth having a look at if some maintainer of this repo >> wants to use b4 to merge stuff? > > I'll add my configuration here, maybe someone or myself can add some doc for it > later, here or elsewhere. > > configuration I use: > > [b4] > pw-url = https://patchwork.yoctoproject.org/ > pw-key = <redacted> > pw-project = docs > pw-review-state = under-review > pw-accept-state = accepted > pw-discard-state = rejected > > For `pw-project`, it comes from the project name on patchwork, e.g.: > > https://patchwork.yoctoproject.org/project/docs/list > ^^^^ > > The pw-key can be obtained from the user's personal settings. > > Then `b4 {am,shazam}` will automatically set the state to under-review. > > `b4 ty -d` will reject the patch. Sometimes I use `b4 ty --pw-set-state > superseded -d ...` when I know another version of the patch/series was sent. > > Finally `b4 ty -a` will also set the accepted state along the generation of > thank-you file. > > And overall it's much easier than clicking buttons on patchwork :) > Thanks! How does this work if I don't have pw-key (which will be the case for most/all contributors)? Does b4 fail or goes on like it would if we had no pw-* settings? Additionally, I'm pretty sure we do not want maintainers to mistakenly push their pw-key to the mailing list. So I guess this should be stored outside of the git repo and rather in .gitconfig, c.f. https://b4.docs.kernel.org/en/latest/config.html#configuration-options Should we add some documentation on how to do that for maintainers? Considering I cannot test this right now (i don't have patchwork set up, nor do I know if it makes sense to set it up if I'm not maintainer :) ), I think it'd make more sense to have someone able to test it to send a patch later to add support for that? What do you think? >> --- >> Changes in v2: >> - add b4-wrapper for auto-detecting patches that are to be sent to the >> docs mailing list as well, >> - disable forced b4 prep --check for each b4-managed series, >> - Link to v1: https://lore.kernel.org/r/20240524-b4-support-v1-1-0c4334c36cc7@cherry.de >> --- >> .b4-config | 4 ++++ >> b4-wrapper.py | 38 ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 42 insertions(+) >> >> diff --git a/.b4-config b/.b4-config >> new file mode 100644 >> index 0000000000000000000000000000000000000000..0b699f204c414099f2118ffdbc9f7b24454e4a4e >> --- /dev/null >> +++ b/.b4-config >> @@ -0,0 +1,4 @@ >> +[b4] >> + send-series-cc = bitbake-devel@lists.openembedded.org >> + send-auto-cc-cmd = ./b4-wrapper.py send-auto-cc-cmd >> + prep-pre-flight-checks = disable-needs-checking >> diff --git a/b4-wrapper.py b/b4-wrapper.py >> new file mode 100755 >> index 0000000000000000000000000000000000000000..ddc2597bc0374fa5124a83be94eeab52df6f38d3 >> --- /dev/null >> +++ b/b4-wrapper.py >> @@ -0,0 +1,38 @@ >> +#!/usr/bin/env python3 >> +# >> +# This script is to be called by b4: >> +# - through b4.send-auto-cc-cmd with "send-auto-cc-cmd" as first argument, >> +# >> +# When send-auto-cc-cmd is passed: >> +# >> +# This returns the list of Cc recipients for a patch. >> +# >> +# This script takes as stdin a patch. >> + >> +import subprocess >> +import sys >> + >> +cmd = sys.argv[1] >> +if cmd != "send-auto-cc-cmd": >> + sys.exit(-1) >> + >> +patch = sys.stdin.read() >> + >> +if subprocess.call(["which", "lsdiff"], stdout=subprocess.DEVNULL) != 0: >> + print("lsdiff missing from host, please install patchutils") >> + sys.exit(-1) >> + >> +print("bitbake-devel@lists.openembedded.org") >> + >> +files = subprocess.check_output(["lsdiff", "--strip-match=1", "--strip=1", "--include=doc/*"], >> + input=patch, text=True) >> +if len(files): >> + print("docs@lists.yoctoproject.org") >> +else: >> +# Handle patches made with --no-prefix >> + files = subprocess.check_output(["lsdiff", "--include=doc/*"], >> + input=patch, text=True) >> + if len(files): >> + print("docs@lists.yoctoproject.org") >> + >> +sys.exit(0) >> I forgot to add in the cover letter, but we could add a check to make sure a patch doesn't touch both documentation and code. But I'm not sure this is something we need to enforce, and maybe that's actually absolutely fine to have both in one commit? I can add that check though, so up to you. >> --- >> base-commit: 0329a7e3ac694737f2d2c1861f65492551360663 >> change-id: 20240524-b4-support-e9855fc54d40 >> >> Best regards, > > Thanks for the script Quentin! > > I was thinking, for eocore we could even extend the list of cc from > meta/conf/distro/include/maintainers.inc. > This is something Richard does not want :) I'm working on b4 support for poky/oe-core and yocto-docs, but I somehow stumbled upon a bug in b4 now :), c.f. https://lore.kernel.org/tools/a8bc0958-6c46-4622-96ef-b1e55c15f30f@cherry.de/T/#u Will send patches soon. poky will be fun to review :) Cheers, Quentin
Hi Quentin, On Fri Jan 17, 2025 at 12:43 PM CET, Quentin Schulz wrote: [...] >>> Note that b4 supports patchwork, c.f. >>> https://b4.docs.kernel.org/en/latest/config.html#patchwork-integration-settings >>> though I have no clue what it does with it as I am no maintainer, but >>> maybe something worth having a look at if some maintainer of this repo >>> wants to use b4 to merge stuff? >> >> I'll add my configuration here, maybe someone or myself can add some doc for it >> later, here or elsewhere. >> >> configuration I use: >> >> [b4] >> pw-url = https://patchwork.yoctoproject.org/ >> pw-key = <redacted> >> pw-project = docs >> pw-review-state = under-review >> pw-accept-state = accepted >> pw-discard-state = rejected >> >> For `pw-project`, it comes from the project name on patchwork, e.g.: >> >> https://patchwork.yoctoproject.org/project/docs/list >> ^^^^ >> >> The pw-key can be obtained from the user's personal settings. >> >> Then `b4 {am,shazam}` will automatically set the state to under-review. >> >> `b4 ty -d` will reject the patch. Sometimes I use `b4 ty --pw-set-state >> superseded -d ...` when I know another version of the patch/series was sent. >> >> Finally `b4 ty -a` will also set the accepted state along the generation of >> thank-you file. >> >> And overall it's much easier than clicking buttons on patchwork :) >> > > Thanks! > > How does this work if I don't have pw-key (which will be the case for > most/all contributors)? Does b4 fail or goes on like it would if we had > no pw-* settings? I just checked and if a pw-key is not set, b4 just does nothing related to patchwork (tested for shazam only). > Additionally, I'm pretty sure we do not want maintainers to mistakenly > push their pw-key to the mailing list. So I guess this should be stored > outside of the git repo and rather in .gitconfig, c.f. > https://b4.docs.kernel.org/en/latest/config.html#configuration-options > Should we add some documentation on how to do that for maintainers? True, in fact all of this was configured from .git/config on a per-repo basis. > Considering I cannot test this right now (i don't have patchwork set up, > nor do I know if it makes sense to set it up if I'm not maintainer :) ), > I think it'd make more sense to have someone able to test it to send a > patch later to add support for that? What do you think? I'm not sure where would be a good place for such documentation, as this is personal tooling and personal taste in the end. Antonin
Hi Antonin, On 1/17/25 1:08 PM, Antonin Godard wrote: > Hi Quentin, > > On Fri Jan 17, 2025 at 12:43 PM CET, Quentin Schulz wrote: > [...] >>>> Note that b4 supports patchwork, c.f. >>>> https://b4.docs.kernel.org/en/latest/config.html#patchwork-integration-settings >>>> though I have no clue what it does with it as I am no maintainer, but >>>> maybe something worth having a look at if some maintainer of this repo >>>> wants to use b4 to merge stuff? >>> >>> I'll add my configuration here, maybe someone or myself can add some doc for it >>> later, here or elsewhere. >>> >>> configuration I use: >>> >>> [b4] >>> pw-url = https://patchwork.yoctoproject.org/ >>> pw-key = <redacted> >>> pw-project = docs >>> pw-review-state = under-review >>> pw-accept-state = accepted >>> pw-discard-state = rejected >>> >>> For `pw-project`, it comes from the project name on patchwork, e.g.: >>> >>> https://patchwork.yoctoproject.org/project/docs/list >>> ^^^^ >>> >>> The pw-key can be obtained from the user's personal settings. >>> >>> Then `b4 {am,shazam}` will automatically set the state to under-review. >>> >>> `b4 ty -d` will reject the patch. Sometimes I use `b4 ty --pw-set-state >>> superseded -d ...` when I know another version of the patch/series was sent. >>> >>> Finally `b4 ty -a` will also set the accepted state along the generation of >>> thank-you file. >>> >>> And overall it's much easier than clicking buttons on patchwork :) >>> >> >> Thanks! >> >> How does this work if I don't have pw-key (which will be the case for >> most/all contributors)? Does b4 fail or goes on like it would if we had >> no pw-* settings? > > I just checked and if a pw-key is not set, b4 just does nothing related to > patchwork (tested for shazam only). > Thanks for testing! >> Additionally, I'm pretty sure we do not want maintainers to mistakenly >> push their pw-key to the mailing list. So I guess this should be stored >> outside of the git repo and rather in .gitconfig, c.f. >> https://b4.docs.kernel.org/en/latest/config.html#configuration-options >> Should we add some documentation on how to do that for maintainers? > > True, in fact all of this was configured from .git/config on a per-repo basis. > >> Considering I cannot test this right now (i don't have patchwork set up, >> nor do I know if it makes sense to set it up if I'm not maintainer :) ), >> I think it'd make more sense to have someone able to test it to send a >> patch later to add support for that? What do you think? > > I'm not sure where would be a good place for such documentation, as this is > personal tooling and personal taste in the end. > I believe those settings (except pw-key) would be identical for everybody, so having them in .b4-config within the repo makes sense to me. What do you think? In a README/CONTRIBUTION file and/or as a comment in .b4-config we can explain how to finish the maintainer setup for example. Cheers, Quentin
On 1/17/25 11:44 AM, 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 Cc recipients > to always add, in our case the mailing list. > > This also adds a wrapper script that is called by b4 to figure out which > addresses to put as Cc recipients. Considering that patches to the doc/ > directory also need to be sent to the yocto-docs mailing list, this > wrapper handles that. A limitation of the script (lsdiff actually) is > that it doesn't know how to handle empty files, but those should be > of rather rare occurrences. > > While the wrapper script should be enough by itself to add the bitbake > mailing list to the Cc recipients, one still needs to manually run b4 > prep --auto-to-cc for that to happen. Therefore, let's add an explicit > send-series-cc so that at least that mailing list is always there. > > Because we currently do not have anything to check for patch validity, > remove requirement for b4 prep --check to be run before sending a patch > series, via disable-needs-checking in prep-pre-flight-checks. > > [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> > --- > This adds a basic config file for b4 to avoid to have to add the mailing > list by hand. > For documentation patches, the additional mailing list will be added > when running b4 prep --check, as is required for each new series managed > by b4. > > Note that b4 supports patchwork, c.f. > https://b4.docs.kernel.org/en/latest/config.html#patchwork-integration-settings > though I have no clue what it does with it as I am no maintainer, but > maybe something worth having a look at if some maintainer of this repo > wants to use b4 to merge stuff? > --- > Changes in v2: > - add b4-wrapper for auto-detecting patches that are to be sent to the > docs mailing list as well, > - disable forced b4 prep --check for each b4-managed series, > - Link to v1: https://lore.kernel.org/r/20240524-b4-support-v1-1-0c4334c36cc7@cherry.de > --- > .b4-config | 4 ++++ > b4-wrapper.py | 38 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/.b4-config b/.b4-config > new file mode 100644 > index 0000000000000000000000000000000000000000..0b699f204c414099f2118ffdbc9f7b24454e4a4e > --- /dev/null > +++ b/.b4-config > @@ -0,0 +1,4 @@ > +[b4] > + send-series-cc = bitbake-devel@lists.openembedded.org > + send-auto-cc-cmd = ./b4-wrapper.py send-auto-cc-cmd > + prep-pre-flight-checks = disable-needs-checking > diff --git a/b4-wrapper.py b/b4-wrapper.py > new file mode 100755 > index 0000000000000000000000000000000000000000..ddc2597bc0374fa5124a83be94eeab52df6f38d3 > --- /dev/null > +++ b/b4-wrapper.py > @@ -0,0 +1,38 @@ > +#!/usr/bin/env python3 > +# Forgot to add an SPDX-License-Identifier here, I can send a v3 for that or simply pick between MIT and GPL-2.0-only and GPL-2.0-or-later which seem to be the most common licenses for python scripts in our projects, I don't have a personal preference :) For now, will wait for feedback first! Cheers, Quentin > +# This script is to be called by b4: > +# - through b4.send-auto-cc-cmd with "send-auto-cc-cmd" as first argument, > +# > +# When send-auto-cc-cmd is passed: > +# > +# This returns the list of Cc recipients for a patch. > +# > +# This script takes as stdin a patch. > + > +import subprocess > +import sys > + > +cmd = sys.argv[1] > +if cmd != "send-auto-cc-cmd": > + sys.exit(-1) > + > +patch = sys.stdin.read() > + > +if subprocess.call(["which", "lsdiff"], stdout=subprocess.DEVNULL) != 0: > + print("lsdiff missing from host, please install patchutils") > + sys.exit(-1) > + > +print("bitbake-devel@lists.openembedded.org") > + > +files = subprocess.check_output(["lsdiff", "--strip-match=1", "--strip=1", "--include=doc/*"], > + input=patch, text=True) > +if len(files): > + print("docs@lists.yoctoproject.org") > +else: > +# Handle patches made with --no-prefix > + files = subprocess.check_output(["lsdiff", "--include=doc/*"], > + input=patch, text=True) > + if len(files): > + print("docs@lists.yoctoproject.org") > + > +sys.exit(0) > > --- > base-commit: 0329a7e3ac694737f2d2c1861f65492551360663 > change-id: 20240524-b4-support-e9855fc54d40 > > Best regards,
diff --git a/.b4-config b/.b4-config new file mode 100644 index 0000000000000000000000000000000000000000..0b699f204c414099f2118ffdbc9f7b24454e4a4e --- /dev/null +++ b/.b4-config @@ -0,0 +1,4 @@ +[b4] + send-series-cc = bitbake-devel@lists.openembedded.org + send-auto-cc-cmd = ./b4-wrapper.py send-auto-cc-cmd + prep-pre-flight-checks = disable-needs-checking diff --git a/b4-wrapper.py b/b4-wrapper.py new file mode 100755 index 0000000000000000000000000000000000000000..ddc2597bc0374fa5124a83be94eeab52df6f38d3 --- /dev/null +++ b/b4-wrapper.py @@ -0,0 +1,38 @@ +#!/usr/bin/env python3 +# +# This script is to be called by b4: +# - through b4.send-auto-cc-cmd with "send-auto-cc-cmd" as first argument, +# +# When send-auto-cc-cmd is passed: +# +# This returns the list of Cc recipients for a patch. +# +# This script takes as stdin a patch. + +import subprocess +import sys + +cmd = sys.argv[1] +if cmd != "send-auto-cc-cmd": + sys.exit(-1) + +patch = sys.stdin.read() + +if subprocess.call(["which", "lsdiff"], stdout=subprocess.DEVNULL) != 0: + print("lsdiff missing from host, please install patchutils") + sys.exit(-1) + +print("bitbake-devel@lists.openembedded.org") + +files = subprocess.check_output(["lsdiff", "--strip-match=1", "--strip=1", "--include=doc/*"], + input=patch, text=True) +if len(files): + print("docs@lists.yoctoproject.org") +else: +# Handle patches made with --no-prefix + files = subprocess.check_output(["lsdiff", "--include=doc/*"], + input=patch, text=True) + if len(files): + print("docs@lists.yoctoproject.org") + +sys.exit(0)