| Message ID | 20250123-b4-support-v1-1-4841f52eccbb@cherry.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] add basic support for b4 contribution workflow | expand |
On Thu, 2025-01-23 at 16:44 +0100, Quentin Schulz via lists.yoctoproject.org 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 check that each > patch in the series is only for one project. Indeed, poky is actually > a "collection" of multiple repositories, namely BitBake, > OpenEmbedded-Core and the Yocto Docs. One patch should therefore not > make changes in multiple of those projects otherwise it cannot be > merged. > Additionally, a check is added to make sure that a series only touches > files from one project to avoid having to figure out which patch is to > be merged by which maintainer in which project repo. > > Moreover, it is not uncommon to have people develop patches for those > projects from within poky. This wrapper figures out which mailing lists > to send patches to based on the files that are modified in the series. > Considering that patches to the bitbake/doc/ directory need to be sent > to both the bitbake and 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. > > Note that this script requires hardcoding of paths that are handled by > different projects to map files to projects. Anything not mapped is > assumed part of OE-Core. > > [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 is marked as RFC because I believe there's some additional plumbing > required to make this work reliably. > > Indeed, OE-Core will also get its own .b4-config, > c.f. https://lore.kernel.org/openembedded-core/20250117-b4-support-v1-1-fe74a5f50eb6@cherry.de/T/#u > > So the same file will exist in both OE-Core and poky, at the same > location. The one from poky should always be this one (or with changes > contributed via the poky mailing list) and not overridden during a merge > by an update from the one in OE-Core. Not sure how to handle that yet > though, does anyone know how this merging of trees is done in poky and > what can be done to make this safe? I can sort that bit. Basically we mark the config file as not being synced by combo-layer. > There's also a decision that was made to not allow patches to different > projects in a series, but maybe that's not desired. > > Finally, I'm not sure about the appropriate copyright holder for the > patch and the license, I personally don't care, so up to you. Those look fine to me. > --- > .b4-config | 3 ++ > b4-wrapper.py | 162 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 165 insertions(+) I'm not against this but I would like to: a) Put the script into OE-Core, just so that we don't have "magic" things in poky. b) Perhaps name it something a little more poky specific c) Move it into the scripts subdir Cheers, Richard
Hi Richard, On 1/24/25 6:48 PM, Richard Purdie wrote: > On Thu, 2025-01-23 at 16:44 +0100, Quentin Schulz via lists.yoctoproject.org 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 check that each >> patch in the series is only for one project. Indeed, poky is actually >> a "collection" of multiple repositories, namely BitBake, >> OpenEmbedded-Core and the Yocto Docs. One patch should therefore not >> make changes in multiple of those projects otherwise it cannot be >> merged. >> Additionally, a check is added to make sure that a series only touches >> files from one project to avoid having to figure out which patch is to >> be merged by which maintainer in which project repo. >> >> Moreover, it is not uncommon to have people develop patches for those >> projects from within poky. This wrapper figures out which mailing lists >> to send patches to based on the files that are modified in the series. >> Considering that patches to the bitbake/doc/ directory need to be sent >> to both the bitbake and 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. >> >> Note that this script requires hardcoding of paths that are handled by >> different projects to map files to projects. Anything not mapped is >> assumed part of OE-Core. >> >> [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 not against this but I would like to: > > a) Put the script into OE-Core, just so that we don't have "magic" > things in poky. It is very poky-specific though. I don't have anything in mind right now but I could imagine we could need a b4 wrapper for openembedded-core at some point as well. b4-wrapper-oe-core.py and b4-wrapper-poky.py could coexistthough and be pointed at specifically by each repo's .b4-config. Don't mind it, just find it a bit odd. I haven't checked if b4 follows symlink, but if it can, would you want .b4-config.poky in OE-Core too and then we do the same dance as with the README.md in OE-Core/poky? > b) Perhaps name it something a little more poky specific b4-wrapper-poky.py? b4-wrapper-poky.py to match README.poky.md for example? My brain has already decided it's the weekend so not very creative right now :) Cheers, Quentin
On Fri, 2025-01-24 at 19:06 +0100, Quentin Schulz wrote: > Hi Richard, > > On 1/24/25 6:48 PM, Richard Purdie wrote: > > On Thu, 2025-01-23 at 16:44 +0100, Quentin Schulz via lists.yoctoproject.org 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 check that each > > > patch in the series is only for one project. Indeed, poky is actually > > > a "collection" of multiple repositories, namely BitBake, > > > OpenEmbedded-Core and the Yocto Docs. One patch should therefore not > > > make changes in multiple of those projects otherwise it cannot be > > > merged. > > > Additionally, a check is added to make sure that a series only touches > > > files from one project to avoid having to figure out which patch is to > > > be merged by which maintainer in which project repo. > > > > > > Moreover, it is not uncommon to have people develop patches for those > > > projects from within poky. This wrapper figures out which mailing lists > > > to send patches to based on the files that are modified in the series. > > > Considering that patches to the bitbake/doc/ directory need to be sent > > > to both the bitbake and 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. > > > > > > Note that this script requires hardcoding of paths that are handled by > > > different projects to map files to projects. Anything not mapped is > > > assumed part of OE-Core. > > > > > > [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 not against this but I would like to: > > > > a) Put the script into OE-Core, just so that we don't have "magic" > > things in poky. > > It is very poky-specific though. I don't have anything in mind right now > but I could imagine we could need a b4 wrapper for openembedded-core at > some point as well. b4-wrapper-oe-core.py and b4-wrapper-poky.py could > coexistthough and be pointed at specifically by each repo's .b4-config. > > Don't mind it, just find it a bit odd. It is a little unusual but I'd rather this than adding poky specific content. I'm find with either a poky specific script or a parameter to put it into "poky" mode. > I haven't checked if b4 follows symlink, but if it can, would you want > .b4-config.poky in OE-Core too and then we do the same dance as with the > README.md in OE-Core/poky? I think that would be confusing, I'm find with two different files and combo-layer can handle that easily. > > b) Perhaps name it something a little more poky specific > > b4-wrapper-poky.py? b4-wrapper-poky.py to match README.poky.md for > example? My brain has already decided it's the weekend so not very > creative right now :) Something like that. I'm also not feeling very creative atm! Cheers, Richard
diff --git a/.b4-config b/.b4-config new file mode 100644 index 0000000000000000000000000000000000000000..26c0dd54e8b1f96bf6d1bf65520c7ec5d72edaa2 --- /dev/null +++ b/.b4-config @@ -0,0 +1,3 @@ +[b4] + prep-perpatch-check-cmd = ./b4-wrapper.py prep-perpatch-check-cmd + send-auto-cc-cmd = ./b4-wrapper.py send-auto-cc-cmd diff --git a/b4-wrapper.py b/b4-wrapper.py new file mode 100755 index 0000000000000000000000000000000000000000..d64754cb02309a443eb9b7748877e62731b3ecfb --- /dev/null +++ b/b4-wrapper.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +# +# Copyright OpenEmbedded Contributors +# +# SPDX-License-Identifier: MIT +# +# This script is to be called by b4: +# - through the b4.prep-perpatch-check-cmd with "prep-perpatch-check-cmd" as +# first argument, +# - through b4.send-auto-cc-cmd with "send-auto-cc-cmd" as first argument, +# +# When prep-perpatch-check-cmd is passsed: +# +# This checks that a patch makes changes to at most one project in the poky +# combo repo (that is, out of yocto-docs, bitbake, openembedded-core combined +# into poky and the poky-specific files). +# +# Printing something to stdout in this file will result in b4 prep --check fail +# for the currently parsed patch. +# +# It checks that all patches in the series make changes to at most one project. +# +# 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 pathlib +import re +import subprocess +import sys + +cmd = sys.argv[1] + +patch = sys.stdin.readlines() + +# Subject field is used to identify the last patch as this script is called for +# each patch. We edit the same file in a series by using the References field +# unique identifier to check which projects are modified by earlier patches in +# the series. To avoid cluttering the disk, the last patch in the list removes +# that shared file. +re_subject = re.compile(r'^Subject:.*\[.*PATCH.*\s(\d+)/\1') +re_ref = re.compile(r'^References: <(.*)>$') + +subject = None +ref = None + +if subprocess.call(["which", "lsdiff"], stdout=subprocess.DEVNULL) != 0: + print("lsdiff missing from host, please install patchutils") + sys.exit(-1) + +try: + one_patch_series = False + for line in patch: + subject = re_subject.match(line) + if subject: + # Handle [PATCH 1/1] + if subject.group(1) == 1: + one_patch_series = True + break + if re.match(r'^Subject: .*\[.*PATCH[^/]*\]', line): + # Single patch is named [PATCH] but if there are prefix, it could be + # [PATCH prefix], so handle everything that doesn't have a / + # character which is used as separator between current patch number + # and total patch number + one_patch_series = True + break + + if cmd == "prep-perpatch-check-cmd" and not one_patch_series: + for line in patch: + ref = re_ref.match(line) + if ref: + break + + if not ref: + print("Failed to find ref to cover letter (References:)...") + sys.exit(-2) + + ref = ref.group(1) + series_check = pathlib.Path(f".tmp-{ref}") + + patch = "".join(patch) + + project_paths = { + "bitbake": ["bitbake/*"], + "yocto-docs": ["documentation/*"], + "poky": [ + "meta-poky/*", + "meta-yocto-bsp/*", + "README.hardware.md", + "README.poky.md", + ".b4-config", + "b4-wrapper.py", + ], + } + + if cmd == "send-auto-cc-cmd": + # Patches to BitBake documentation should also go to yocto-docs mailing list + project_paths["yocto-docs"] += ["bitbake/doc/*"] + + # List of projects touched by this patch + projs = [] + + for proj, proj_paths in project_paths.items(): + lsdiff_args = [f"--include={path}" for path in proj_paths] + files = subprocess.check_output(["lsdiff", "--strip-match=1", "--strip=1"] + lsdiff_args, + input=patch, text=True) + if len(files): + projs.append(proj) + continue + + # Handle patches made with --no-prefix + files = subprocess.check_output(["lsdiff"] + lsdiff_args, + input=patch, text=True) + if len(files): + projs.append(proj) + + if len(projs) == 0: + # Catch-all for everything not poky-specific or in bitbake/yocto-docs + projs.append("openembedded-core") + + if cmd == "prep-perpatch-check-cmd": + if len(projs) > 1: + print(f"Diff spans more than one project ({projs}), split into multiple commits...") + sys.exit(-3) + + # No need to check other patches in the series as there aren't any + if one_patch_series: + sys.exit(0) + + # This should be replaced once b4 supports prep-perseries-check-cmd (or something similar) + + if series_check.exists(): + # NOT race-free if b4 decides to parallelize prep-perpatch-check-cmd + series_projs = series_check.read_text().split('\n') + else: + series_projs = [] + + series_projs += projs + uniq_series_projs = set(series_projs) + # NOT race-free, if b4 decides to parallelize prep-perpatch-check-cmd + series_check.write_text('\n'.join(uniq_series_projs)) + + if len(uniq_series_projs) > 1: + print(f"Series spans more than one project ({', '.join(sorted(uniq_series_projs))}), split into multiple series...") + sys.exit(-4) + else: # send-auto-cc-cmd + ml_projs = { + "bitbake": "bitbake-devel@lists.openembedded.org", + "yocto-docs": "docs@lists.yoctoproject.org", + "poky": "poky@lists.yoctoproject.org", + "openembedded-core": "openembedded-core@lists.openembedded.org", + } + + print("\n".join([ml_projs[ml] for ml in projs])) + + sys.exit(0) +finally: + # Last patch in the series, cleanup tmp file + if subject and ref and series_check.exists(): + series_check.unlink()