From f6925237576e743d414fe59e373956763e70c6f5 Mon Sep 17 00:00:00 2001 From: Agathe Porte Date: Mon, 25 Nov 2024 11:44:17 +0100 Subject: [PATCH 1/5] Import ubuntu_acceptance_criteria Signed-off-by: Agathe Porte --- docs/reference/ubuntu_acceptance_criteria.rst | 296 ++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 docs/reference/ubuntu_acceptance_criteria.rst diff --git a/docs/reference/ubuntu_acceptance_criteria.rst b/docs/reference/ubuntu_acceptance_criteria.rst new file mode 100644 index 0000000..8376826 --- /dev/null +++ b/docs/reference/ubuntu_acceptance_criteria.rst @@ -0,0 +1,296 @@ +.. highlight:: email + +.. _ubuntu-patches-acceptance-criteria: + +Ubuntu patches acceptance criteria +================================== + +Generally, any patch is eligible for inclusion in the Ubuntu kernel, though some +criteria apply. + +Patch category +-------------- + +A patch falls into 3 categories: + +cherry-pick + The patch is part of the stable upstream for the desired kernel version, but + picking it with the stable upstream patches may be delayed and there’s a + request to have the specific patch as soon as possible. + +backport + The patch is upstream (either mainline_ or stable_), but part of a newer + kernel version, and the submitter asks for a backport to an older Ubuntu + kernel version. + +SAUCE + The patch is not upstream and/or never will. + +Usually, cherry-picks have the biggest rate of approval if it’s done correctly. +Backports and SAUCE patches are a bit tricky. In general, we avoid merging +those as much as possible. + +For a detailed description on how to format patches before submission, see this +`guideline document`_, while here is a quick (and far from complete) +introduction. + +.. _guideline document: https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat + +All patches +----------- + +Reasons why the Ubuntu Kernel Team won't approve a patch: + +Patch does not apply +^^^^^^^^^^^^^^^^^^^^ + +The patch should always be based on a recent kernel. Expect to resubmit +again if the tip changes and the patch has conflicts. + +SRU patches +----------- + +This section describes additional reasons why the Ubuntu Kernel Team won't +approve a SRU patch. + +Missing/bad BugLink +^^^^^^^^^^^^^^^^^^^ + +Patches and cover letter must have a link to a Launchpad bug as the first line +of the description. + +The link must be in the short form ``https://bugs.launchpad.net/bugs/XXXXXX``. + +Example:: + + Subject: [SRU][O/N][PATCH v2 0/1] ALSA: hda/realtek: fix mute/micmute LEDs for a HP EliteBook 645 G10 + + BugLink: https://bugs.launchpad.net/bugs/2087983 + + ... + +The link must not be in its long form +``https://bugs.launchpad.net/ubuntu/+source/linux/+bug/XXXXXX``. + +*Bad* example:: + + Subject: [SRU][O/N][PATCH v2 0/1] ALSA: hda/realtek: fix mute/micmute LEDs for a HP EliteBook 645 G10 + + BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2087983 + + ... + +Missing/bad Bug +^^^^^^^^^^^^^^^ + +The Launchpad bug should be targeted to the kernels and series that the patch +is aiming to lend. + +The bug description must follow the `SRU template`_. + +.. _SRU template: https://canonical-sru-docs.readthedocs-hosted.com/en/latest/reference/bug-template/ + +See `this example Launchpad bug`_. + +.. _this example Launchpad bug: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1995957> + +Missing/bad SRU cover letter +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The patch must come with a *cover letter* that has both a short link to the SRU +bug and a copy of the *SRU Justification* from the bug. It can be generated +using both the ``--cover-letter`` and the ``--compose`` options of the +git-cherry-pick_ command. + +Example cover letter:: + + Subject: [SRU][O/N][PATCH v2 0/1] ALSA: hda/realtek: fix mute/micmute LEDs for a HP EliteBook 645 G10 + + BugLink: https://bugs.launchpad.net/bugs/2087983 + + SRU Justification: + + [ Impact ] + + Mute/mic LEDs don't function on HP EliteBook 645 G10. + + [ Test Plan ] + + Test mute and mic LEDs with proposed kernel once patched. + + [ Where problems could occur ] + + Unknown regressions in the sound subsystem. + + Maksym Glubokiy (1): + ALSA: hda/realtek: fix mute/micmute LEDs for a HP EliteBook 645 G10 + + sound/pci/hda/patch_realtek.c | 1 + + 1 file changed, 1 insertion(+) + +Cherry-pick or backport +----------------------- + +This section describes additional reasons why the Ubuntu Kernel Team won't +approve a cherry-pick or backport patch. + +Not upstream +^^^^^^^^^^^^ + +The patch should be in the mainline_ or the stable_ tree. Having the patch in +a maintainer subtree is not enough, because the subtree might change. Having +the patch in linux-next_ is bare minimum. + +.. _mainline: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ +.. _stable: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ +.. _linux-next: https://www.kernel.org/doc/man-pages/linux-next.html + +Missing/bad upstream source +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +.. highlight:: text + +The patches should have a *cherry picked from* or *backported from* line with +the appropriate sha from the upstream. It can be generated using the +``-x`` option of the git-cherry-pick_ command. This line should appear just +before your *Signed-off-by*:: + + (cherry picked from commit 622f21994506e1dac7b8e4e362c8951426e032c5) + +:: + + (backported from commit 622f21994506e1dac7b8e4e362c8951426e032c5) + +In case the upstream source is linux-next, you should explicit it:: + + (cherry picked from commit 622f21994506e1dac7b8e4e362c8951426e032c5 linux-next) + +In case the provenance is anything else, you should explicit the source git +tree in full:: + + (cherry picked from commit 622f21994506e1dac7b8e4e362c8951426e032c5 git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git) + +.. highlight:: email + +Missing/bad Signed-off-by +^^^^^^^^^^^^^^^^^^^^^^^^^ + +The patches must have your Signed-off-by as the last line, after the upstream +cherry-picked line. It can be generated using the ``-s`` option of the +git-cherry-pick_ command. + +If the patch is from yourself and already has your SoB, a new SoB must be +added. + +Example:: + + Subject: [PATCH] ufs: ufs_sb_private_info: remove unused s_{2,3}apb fields + + BugLink: https://bugs.launchpad.net/ubuntu/oracular/+source/linux/+bug/2087853 + + These two fields are populated and stored as a "frequently used value" + in ufs_fill_super, but are not used afterwards in the driver. + + Moreover, one of the shifts triggers UBSAN: shift-out-of-bounds when + apbshift is 12 because 12 * 3 = 36 and 1 << 36 does not fit in the 32 + bit integer used to store the value. + + Closes: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2087853 + Signed-off-by: Agathe Porte + Signed-off-by: Al Viro + (cherry picked from commit 6cfe56fbad32c8c5b50e82d9109413566d691569 linux-next) + Signed-off-by: Agathe Porte + +.. _mainline: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ +.. _stable: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ +.. _linux-next: https://www.kernel.org/doc/man-pages/linux-next.html + +SAUCE +----- + +This section describes additional reasons why the Ubuntu Kernel Team won't +approve a SAUCE patch. + +Missing/bad SAUCE prefix +^^^^^^^^^^^^^^^^^^^^^^^^ + +The patches must have the ``UBUNTU: SAUCE:`` prefix. + +Example:: + + Subject: UBUNTU: SAUCE: wifi: ath11k: avoid deadlock during regulatory update in ath11k_regd_update() + + BugLink: https://bugs.launchpad.net/bugs/1995041 + + ... + + Signed-off-by: Aaron Ma + +Backport or SAUCE +------------------ + +This section describes additional reasons why the Ubuntu Kernel Team won't +approve a SAUCE or backport patch. + +Lack of Testing +^^^^^^^^^^^^^^^ + +It is very important for patches to have the upstream maintainer(s) review +and do wider testing on different types of hardware for various types of +scenarios. Even though the patch was tested by the submitter, the tests may +be limited to a specific use case and prone to breaking other parts of the +kernel affected by this change. In the case of backports, it was not tested +upstream for the specific kernel version, therefore it may cause issues. + +Maintenance +^^^^^^^^^^^ + +Maintaining a patch in our tree is not easy. Let’s say we include v0.54 of +some patch. Later, we want to sync up to the latest version of this patch. +It’s not easy to simply revert v0.54, because merges could have changed some +of the code. Not to mention, there are very few patches like this that +provide incremental changes between versions. + +Core code impact +^^^^^^^^^^^^^^^^ + +If our kernel contains multiple SAUCE patches or backports, it will diverge +from the upstream kernel. In case we need help from upstream to solve bugs, +we will have to first test if one of these patches does not cause the bug and +then ask the community for help. + +Merge conflict +^^^^^^^^^^^^^^ + +It may cause merge conflicts later when someone from upstream changes the +same piece of code. If the component is prone to frequent changes upstream, +we will have to deal with this a lot and it will require extra effort on our +side. + +Security concerns +^^^^^^^^^^^^^^^^^ + +It may open up unforeseen security issues. Not that this does not happen with +upstream code, but having the code there reaches a wider audience, and more +people are involved in mitigating the issue. + +Bug Prone +^^^^^^^^^ + +It may introduce new bugs that have a wider impact due to limited testing, +especially if the change affects a component used in many places. + +Quality +^^^^^^^ + +Not a very common reason, but the patch may not fit into our standards of +code quality or may not serve any real purpose. + +Lack of time +^^^^^^^^^^^^ + +Maintaining these patches, with all the arguments from above, will be +time-consuming on our side, and we don’t have the resources to both do this +and deliver a stable Linux OS + +.. _git-cherry-pick: https://manpages.ubuntu.com/manpages/trusty/en/man1/git-cherry-pick.1.html From 4fc0929ce0ccfc0ded2f43c213d1692b3f70c020 Mon Sep 17 00:00:00 2001 From: Agathe Porte Date: Mon, 25 Nov 2024 11:44:55 +0100 Subject: [PATCH 2/5] Rework ubuntu_acceptance_criteria to patch_acceptance_criteria Shorten some titles to make them fit in the ToC. Signed-off-by: Agathe Porte --- ...eria.rst => patch_acceptance_criteria.rst} | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) rename docs/reference/{ubuntu_acceptance_criteria.rst => patch_acceptance_criteria.rst} (94%) diff --git a/docs/reference/ubuntu_acceptance_criteria.rst b/docs/reference/patch_acceptance_criteria.rst similarity index 94% rename from docs/reference/ubuntu_acceptance_criteria.rst rename to docs/reference/patch_acceptance_criteria.rst index 8376826..39371d5 100644 --- a/docs/reference/ubuntu_acceptance_criteria.rst +++ b/docs/reference/patch_acceptance_criteria.rst @@ -2,8 +2,8 @@ .. _ubuntu-patches-acceptance-criteria: -Ubuntu patches acceptance criteria -================================== +Patch acceptance criteria +========================= Generally, any patch is eligible for inclusion in the Ubuntu kernel, though some criteria apply. @@ -53,8 +53,23 @@ SRU patches This section describes additional reasons why the Ubuntu Kernel Team won't approve a SRU patch. -Missing/bad BugLink -^^^^^^^^^^^^^^^^^^^ +Launchpad bug +^^^^^^^^^^^^^ + +Each patch must be related to a dedicated Launchpad bug. The bug should be +targeted to the kernels and series that the patch is aiming to lend. + +The bug description must follow the `SRU template`_. + +.. _SRU template: https://canonical-sru-docs.readthedocs-hosted.com/en/latest/reference/bug-template/ + +See `this example Launchpad bug`_. + +.. _this example Launchpad bug: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1995957> + + +BugLink +^^^^^^^ Patches and cover letter must have a link to a Launchpad bug as the first line of the description. @@ -80,22 +95,8 @@ The link must not be in its long form ... -Missing/bad Bug -^^^^^^^^^^^^^^^ - -The Launchpad bug should be targeted to the kernels and series that the patch -is aiming to lend. - -The bug description must follow the `SRU template`_. - -.. _SRU template: https://canonical-sru-docs.readthedocs-hosted.com/en/latest/reference/bug-template/ - -See `this example Launchpad bug`_. - -.. _this example Launchpad bug: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1995957> - -Missing/bad SRU cover letter -^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +SRU cover letter +^^^^^^^^^^^^^^^^ The patch must come with a *cover letter* that has both a short link to the SRU bug and a copy of the *SRU Justification* from the bug. It can be generated @@ -134,8 +135,8 @@ Cherry-pick or backport This section describes additional reasons why the Ubuntu Kernel Team won't approve a cherry-pick or backport patch. -Not upstream -^^^^^^^^^^^^ +Upstream +^^^^^^^^ The patch should be in the mainline_ or the stable_ tree. Having the patch in a maintainer subtree is not enough, because the subtree might change. Having @@ -145,8 +146,8 @@ the patch in linux-next_ is bare minimum. .. _stable: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ .. _linux-next: https://www.kernel.org/doc/man-pages/linux-next.html -Missing/bad upstream source -^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Source +^^^^^^ .. highlight:: text @@ -172,8 +173,8 @@ tree in full:: .. highlight:: email -Missing/bad Signed-off-by -^^^^^^^^^^^^^^^^^^^^^^^^^ +Signed-off-by +^^^^^^^^^^^^^ The patches must have your Signed-off-by as the last line, after the upstream cherry-picked line. It can be generated using the ``-s`` option of the @@ -211,8 +212,8 @@ SAUCE This section describes additional reasons why the Ubuntu Kernel Team won't approve a SAUCE patch. -Missing/bad SAUCE prefix -^^^^^^^^^^^^^^^^^^^^^^^^ +SAUCE prefix +^^^^^^^^^^^^ The patches must have the ``UBUNTU: SAUCE:`` prefix. @@ -232,8 +233,8 @@ Backport or SAUCE This section describes additional reasons why the Ubuntu Kernel Team won't approve a SAUCE or backport patch. -Lack of Testing -^^^^^^^^^^^^^^^ +Testing +^^^^^^^ It is very important for patches to have the upstream maintainer(s) review and do wider testing on different types of hardware for various types of From 0db2d98a36b559e5cc5821926ccc75f4d60150f5 Mon Sep 17 00:00:00 2001 From: Agathe Porte Date: Wed, 27 Nov 2024 14:25:33 +0100 Subject: [PATCH 3/5] patch_acceptance_criteria: replace some "must" with "should" After discussion with @smb some criteria are not hard requirements but have been used since some time. Change the wording to reflect that. Signed-off-by: Agathe Porte --- docs/reference/patch_acceptance_criteria.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/reference/patch_acceptance_criteria.rst b/docs/reference/patch_acceptance_criteria.rst index 39371d5..03277fd 100644 --- a/docs/reference/patch_acceptance_criteria.rst +++ b/docs/reference/patch_acceptance_criteria.rst @@ -71,8 +71,8 @@ See `this example Launchpad bug`_. BugLink ^^^^^^^ -Patches and cover letter must have a link to a Launchpad bug as the first line -of the description. +Patches and cover letter should have a link to a Launchpad bug as the first +line of the description. The link must be in the short form ``https://bugs.launchpad.net/bugs/XXXXXX``. @@ -98,8 +98,8 @@ The link must not be in its long form SRU cover letter ^^^^^^^^^^^^^^^^ -The patch must come with a *cover letter* that has both a short link to the SRU -bug and a copy of the *SRU Justification* from the bug. It can be generated +The patch should come with a *cover letter* that has both a short link to the +SRU bug and a copy of the *SRU Justification* from the bug. It can be generated using both the ``--cover-letter`` and the ``--compose`` options of the git-cherry-pick_ command. From 2540fbf76db0acfdaf700f60e887f17196feeafd Mon Sep 17 00:00:00 2001 From: Agathe Porte Date: Wed, 27 Nov 2024 14:26:46 +0100 Subject: [PATCH 4/5] patch_acceptance_criteria: add some cover-letter precisions Signed-off-by: Agathe Porte --- docs/reference/patch_acceptance_criteria.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/reference/patch_acceptance_criteria.rst b/docs/reference/patch_acceptance_criteria.rst index 03277fd..a37d056 100644 --- a/docs/reference/patch_acceptance_criteria.rst +++ b/docs/reference/patch_acceptance_criteria.rst @@ -129,6 +129,15 @@ Example cover letter:: sound/pci/hda/patch_realtek.c | 1 + 1 file changed, 1 insertion(+) +If the patchset is a new version of a previous patchset posted on the +mailing-list, the cover letter should explain what has changed for this new +submission. + +If the patchset involved some decisions that were not obvious, it should be +explained in the cover letter to ease the review of the patchset. If you choose +to send a SAUCE patch instead of the other options, the rationale should be +explained in the cover letter. + Cherry-pick or backport ----------------------- From 5da7f83c8b6ef264c57c94e7c330e678f4544756 Mon Sep 17 00:00:00 2001 From: Agathe Porte Date: Wed, 27 Nov 2024 14:30:53 +0100 Subject: [PATCH 5/5] patch_acceptance_criteria: add section for Ubuntu kernel source Signed-off-by: Agathe Porte --- docs/reference/patch_acceptance_criteria.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/reference/patch_acceptance_criteria.rst b/docs/reference/patch_acceptance_criteria.rst index a37d056..29cab8e 100644 --- a/docs/reference/patch_acceptance_criteria.rst +++ b/docs/reference/patch_acceptance_criteria.rst @@ -175,6 +175,11 @@ In case the upstream source is linux-next, you should explicit it:: (cherry picked from commit 622f21994506e1dac7b8e4e362c8951426e032c5 linux-next) +In case the upstream source is another Ubuntu kernel (even a SAUCE patch), you +can explicit it with the name of the source kernel:: + + (cherry picked from commit 622f21994506e1dac7b8e4e362c8951426e032c5 plucky:linux) + In case the provenance is anything else, you should explicit the source git tree in full::