[PATCH v2 1/5] contributor-guide: recipe-style-guide: add more patch tagging examples

[PATCH v2 1/5] contributor-guide: recipe-style-guide: add more patch tagging examples

[Roland Hieber]

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 * reorder before the patch adding the CVE section
 * use commit URL instead of pull request URL in Backport example

PATCH v1: https://lore.kernel.org/yocto-docs/20230920100647.1038583-2-rhi@pengutronix.de/
---
 .../contributor-guide/recipe-style-guide.rst  | 21 ++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index 99105179a6b9..0a2a3581a77e 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -321,7 +321,10 @@ the status should be changed to ``Submitted [where]``, and an additional
 ``Signed-off-by:`` line should be added to the patch by the person claiming
 responsibility for upstreaming.
 
-For example, if the patch has been submitted upstream::
+Examples
+--------
+
+Here's an example of a patch that has been submitted upstream::
 
    rpm: Adjusted the foo setting in bar
 
@@ -336,3 +339,19 @@ For example, if the patch has been submitted upstream::
 
 A future update can change the value to ``Accepted`` or ``Denied`` as
 appropriate.
+
+Another example of a patch that is specific to OpenEmbedded::
+
+   Do not treat warnings as errors
+
+   There are additional warnings found with musl which are
+   treated as errors and fails the build, we have more combinations
+   than upstream supports to handle.
+
+   Upstream-Status: Inappropriate [oe specific]
+
+Here's a patch that has been backported from an upstream commit::
+
+   include missing sys/file.h for LOCK_EX
+
+   Upstream-Status: Backport [https://github.com/systemd/systemd/commit/ac8db36cbc26694ee94beecc8dca208ec4b5fd45]
-- 
2.39.2

[PATCH v2 2/5] contributor-guide: recipe-style-guide: add section about CVE patches

[Roland Hieber]

This was previously included in the OpenEmbedded wiki page [1], but was
not ported along with the rest in commit 95c9a1e1e78bbfb82ade
(2023-09-12, Michael Opdenacker: "contributor-guide: recipe-style-guide:
add Upstream-Status").

  [1]: https://www.openembedded.org/index.php?title=Commit_Patch_Message_Guidelines&oldid=10935

Group the examples in their own sections (but name it differently from
any other section in this document so that sphinx doesn't generate a
warning about duplicate labels).

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 * reorder after the patch adding the general patch examples
 * put CVE examples into their own subsection
 * propagate CVE section one section level higher
 * use ``literal`` instead of *emphasized* formatting for CVE tag

PATCH v1: https://lore.kernel.org/yocto-docs/20230920100647.1038583-1-rhi@pengutronix.de/
---
 .../contributor-guide/recipe-style-guide.rst  | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index 0a2a3581a77e..b574e6cb53a1 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -355,3 +355,28 @@ Here's a patch that has been backported from an upstream commit::
    include missing sys/file.h for LOCK_EX
 
    Upstream-Status: Backport [https://github.com/systemd/systemd/commit/ac8db36cbc26694ee94beecc8dca208ec4b5fd45]
+
+CVE patches
+===========
+
+In order to have a better control of vulnerabilities, patches that fix CVEs must
+contain a ``CVE:`` tag. This tag list all CVEs fixed by the patch. If more than
+one CVE is fixed, separate them using spaces.
+
+CVE Examples
+------------
+
+This should be the header of patch that fixes CVE-2015-8370 in GRUB2::
+
+   grub2: Fix CVE-2015-8370
+
+   [No upstream tracking] -- https://bugzilla.redhat.com/show_bug.cgi?id=1286966
+
+   Back to 28; Grub2 Authentication
+
+   Two functions suffer from integer underflow fault; the grub_username_get() and grub_password_get()located in
+   grub-core/normal/auth.c and lib/crypto.c respectively. This can be exploited to obtain a Grub rescue shell.
+
+   Upstream-Status: Backport [http://git.savannah.gnu.org/cgit/grub.git/commit/?id=451d80e52d851432e109771bb8febafca7a5f1f2]
+   CVE: CVE-2015-8370
+   Signed-off-by: Joe Developer <joe.developer@example.com>
-- 
2.39.2

[PATCH v2 3/5] contributor-guide: discourage marking patches as Inappropriate

[Roland Hieber]

It was never really clear what all those reasons really meant, and every
patch submitted upstream liftens the maintenance on the Yocto side.
So remove the current list, and replace it with two reasons in which an
upstream submission likely won't benefit the upstream project.

Suggested-by: Alexander Kanavin <alex.kanavin@gmail.com>
Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 * fix list syntax

PATCH v1: https://lore.kernel.org/yocto-docs/20230920100647.1038583-3-rhi@pengutronix.de/
---
 .../contributor-guide/recipe-style-guide.rst  | 30 +++++++++----------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index b574e6cb53a1..77e8910dabbb 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -299,22 +299,20 @@ following status strings:
 
 ``Inappropriate [reason]``
    The patch is not appropriate for upstream, include a brief reason on the
-   same line enclosed with ``[]``. The reason can be:
-
-   -  ``not author`` (you are not the author and do not intend to upstream this,
-      the source must be listed in the comments)
-   -  ``native``
-   -  ``licensing``
-   -  ``configuration``
-   -  ``enable feature``
-   -  ``disable feature``
-   -  ``bugfix`` (add bug URL here)
-   -  ``embedded specific``
-   -  ``other`` (give details in comments)
-
-The various ``Inappropriate [reason]`` status items are meant to indicate that
-the person responsible for adding this patch to the system does not intend to
-upstream the patch for a specific reason.
+   same line enclosed with ``[]``. In the past, there were several different
+   reasons not to submit patches upstream, but we have to consider that every
+   non-upstreamed patch means a maintainance burden for recipe maintainers.
+   Currently, the only reasons to mark patches as inappropriate for upstream
+   submission are:
+
+   -  ``oe specific``: the issue is specific to how Yocto performs builds
+      or sets things up at runtime, and can be resolved only with a patch that
+      is not however relevant or appropriate for general upstream submission.
+   -  ``upstream ticket <link>``: the issue is not Yocto-specific and should be
+      fixed upstream, but the patch in its current form is not suitable for
+      merging upstream, and the author lacks sufficient expertise to develope a
+      proper patch. Instead the issue is handled via a bug report (include
+      link).
 
 Of course, if another person later takes care of submitting this patch upstream,
 the status should be changed to ``Submitted [where]``, and an additional
-- 
2.39.2

[PATCH v2 4/5] contributor-guide: deprecate "Accepted" patch status

[Roland Hieber]

This is in accordance with the gatesgarth Release Notes:

  > In the ``Upstream-Status`` header convention for patches,
  > ``Accepted`` has | been replaced with ``Backport`` as these almost
  > always mean the same thing i.e. the patch is already upstream and
  > may need to be removed in a future recipe upgrade. If you are adding
  > these headers to your own patches then use Backport to indicate that
  > the patch has been sent upstream.

  <https://docs.yoctoproject.org/migration-guides/migration-3.2.html#miscellaneous-changes>

Suggested-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 * remove note about historical Accepted status (feedback by Michael
   Opdenacker)

PATCH v1: https://lore.kernel.org/yocto-docs/20230920100647.1038583-4-rhi@pengutronix.de/
---
 .../contributor-guide/recipe-style-guide.rst        | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index 77e8910dabbb..c69e4a92c992 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -277,13 +277,10 @@ following status strings:
    Submitted to upstream, waiting for approval. Optionally include where
    it was submitted, such as the author, mailing list, etc.
 
-``Accepted``
-   Accepted in upstream, expect it to be removed at next update, include
-   expected version info.
-
-``Backport``
-   Backported from new upstream version, because we are at a fixed version,
-   include upstream version info.
+``Backport [version]``
+   Accepted upstream and included in the next release, or backported from newer
+   upstream version, because we are at a fixed version.
+   Include upstream version info (e.g. commit ID or next expected version).
 
 ``Denied``
    Not accepted by upstream, include reason in patch.
@@ -335,7 +332,7 @@ Here's an example of a patch that has been submitted upstream::
 
    Signed-off-by: Joe Developer <joe.developer@example.com>
 
-A future update can change the value to ``Accepted`` or ``Denied`` as
+A future update can change the value to ``Backport`` or ``Denied`` as
 appropriate.
 
 Another example of a patch that is specific to OpenEmbedded::
-- 
2.39.2

[PATCH v2 5/5] contributor-guide: style-guide: discourage using Pending patch status

[Roland Hieber]

Suggested-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Suggested-by: Alexander Kanavin <alex.kanavin@gmail.com>
Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
PATCH v2:
 * new patch in v2, based on feedback by Michael Opdenacker in v1
   https://lore.kernel.org/yocto-docs/12ef125b-5466-408c-ec8c-c5bef1510bc7@bootlin.com/
   and Alexander Kanavin in
   https://lore.kernel.org/yocto-docs/CANNYZj-7C_DeNgVCyRuHRCXZNX-2JPcnysKLn7gQq8w-4aAkNA@mail.gmail.com/
---
 documentation/contributor-guide/recipe-style-guide.rst | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index c69e4a92c992..c24ec267d394 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -271,7 +271,13 @@ Then, you should also add an ``Upstream-Status:`` tag containing one of the
 following status strings:
 
 ``Pending``
-   No determination has been made yet or not yet submitted to upstream.
+   No determination has been made yet, or patch has not yet been submitted to
+   upstream.
+
+   Keep in mind that every patch submitted upstream reduces the maintainance
+   burden in Yocto in the long run, so this patch status should only be used in
+   exceptional cases if there are genuine obstacles to submitting a patch
+   upstream; the reason for that should be included in the patch.
 
 ``Submitted [where]``
    Submitted to upstream, waiting for approval. Optionally include where
-- 
2.39.2

Re: [docs] [PATCH v2 3/5] contributor-guide: discourage marking patches as Inappropriate

[Michael Opdenacker]

On 22.09.23 at 12:00, Roland Hieber wrote:
> It was never really clear what all those reasons really meant, and every
> patch submitted upstream liftens the maintenance on the Yocto side.
> So remove the current list, and replace it with two reasons in which an
> upstream submission likely won't benefit the upstream project.
>
> Suggested-by: Alexander Kanavin <alex.kanavin@gmail.com>
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2:
>   * fix list syntax
>
> PATCH v1: https://lore.kernel.org/yocto-docs/20230920100647.1038583-3-rhi@pengutronix.de/
> ---
>   .../contributor-guide/recipe-style-guide.rst  | 30 +++++++++----------
>   1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
> index b574e6cb53a1..77e8910dabbb 100644
> --- a/documentation/contributor-guide/recipe-style-guide.rst
> +++ b/documentation/contributor-guide/recipe-style-guide.rst
> @@ -299,22 +299,20 @@ following status strings:
>   
>   ``Inappropriate [reason]``
>      The patch is not appropriate for upstream, include a brief reason on the
> -   same line enclosed with ``[]``. The reason can be:
> -
> -   -  ``not author`` (you are not the author and do not intend to upstream this,
> -      the source must be listed in the comments)
> -   -  ``native``
> -   -  ``licensing``
> -   -  ``configuration``
> -   -  ``enable feature``
> -   -  ``disable feature``
> -   -  ``bugfix`` (add bug URL here)
> -   -  ``embedded specific``
> -   -  ``other`` (give details in comments)
> -
> -The various ``Inappropriate [reason]`` status items are meant to indicate that
> -the person responsible for adding this patch to the system does not intend to
> -upstream the patch for a specific reason.
> +   same line enclosed with ``[]``. In the past, there were several different
> +   reasons not to submit patches upstream, but we have to consider that every
> +   non-upstreamed patch means a maintainance burden for recipe maintainers.
> +   Currently, the only reasons to mark patches as inappropriate for upstream
> +   submission are:
> +
> +   -  ``oe specific``: the issue is specific to how Yocto performs builds
> +      or sets things up at runtime, and can be resolved only with a patch that
> +      is not however relevant or appropriate for general upstream submission.
> +   -  ``upstream ticket <link>``: the issue is not Yocto-specific and should be
> +      fixed upstream, but the patch in its current form is not suitable for
> +      merging upstream, and the author lacks sufficient expertise to develope a
> +      proper patch. Instead the issue is handled via a bug report (include
> +      link).


For the above two paragraphs, I prefer to use "OpenEmbedded" instead of 
"Yocto", because that's the Contributor Guide for OE and Yocto.
I fixed it.
Thanks!
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH v2 3/5] contributor-guide: discourage marking patches as Inappropriate

[Michael Opdenacker]

On 22.09.23 at 15:11, Michael Opdenacker via lists.yoctoproject.org wrote:
>
> On 22.09.23 at 12:00, Roland Hieber wrote:
>> It was never really clear what all those reasons really meant, and every
>> patch submitted upstream liftens the maintenance on the Yocto side.
>> So remove the current list, and replace it with two reasons in which an
>> upstream submission likely won't benefit the upstream project.
>>
>> Suggested-by: Alexander Kanavin <alex.kanavin@gmail.com>
>> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
>> ---
>> PATCH v2:
>>   * fix list syntax
>>
>> PATCH v1: 
>> https://lore.kernel.org/yocto-docs/20230920100647.1038583-3-rhi@pengutronix.de/
>> ---
>>   .../contributor-guide/recipe-style-guide.rst  | 30 +++++++++----------
>>   1 file changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/documentation/contributor-guide/recipe-style-guide.rst 
>> b/documentation/contributor-guide/recipe-style-guide.rst
>> index b574e6cb53a1..77e8910dabbb 100644
>> --- a/documentation/contributor-guide/recipe-style-guide.rst
>> +++ b/documentation/contributor-guide/recipe-style-guide.rst
>> @@ -299,22 +299,20 @@ following status strings:
>>     ``Inappropriate [reason]``
>>      The patch is not appropriate for upstream, include a brief 
>> reason on the
>> -   same line enclosed with ``[]``. The reason can be:
>> -
>> -   -  ``not author`` (you are not the author and do not intend to 
>> upstream this,
>> -      the source must be listed in the comments)
>> -   -  ``native``
>> -   -  ``licensing``
>> -   -  ``configuration``
>> -   -  ``enable feature``
>> -   -  ``disable feature``
>> -   -  ``bugfix`` (add bug URL here)
>> -   -  ``embedded specific``
>> -   -  ``other`` (give details in comments)
>> -
>> -The various ``Inappropriate [reason]`` status items are meant to 
>> indicate that
>> -the person responsible for adding this patch to the system does not 
>> intend to
>> -upstream the patch for a specific reason.
>> +   same line enclosed with ``[]``. In the past, there were several 
>> different
>> +   reasons not to submit patches upstream, but we have to consider 
>> that every
>> +   non-upstreamed patch means a maintainance burden for recipe 
>> maintainers.
>> +   Currently, the only reasons to mark patches as inappropriate for 
>> upstream
>> +   submission are:
>> +
>> +   -  ``oe specific``: the issue is specific to how Yocto performs 
>> builds
>> +      or sets things up at runtime, and can be resolved only with a 
>> patch that
>> +      is not however relevant or appropriate for general upstream 
>> submission.
>> +   -  ``upstream ticket <link>``: the issue is not Yocto-specific 
>> and should be
>> +      fixed upstream, but the patch in its current form is not 
>> suitable for
>> +      merging upstream, and the author lacks sufficient expertise to 
>> develope a


s/develope/develop/
FIx too (forgot to report it).

Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH v2 5/5] contributor-guide: style-guide: discourage using Pending patch status

[Michael Opdenacker]

Hi Roland

On 22.09.23 at 12:00, Roland Hieber wrote:
> Suggested-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Suggested-by: Alexander Kanavin <alex.kanavin@gmail.com>
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> PATCH v2:
>   * new patch in v2, based on feedback by Michael Opdenacker in v1
>     https://lore.kernel.org/yocto-docs/12ef125b-5466-408c-ec8c-c5bef1510bc7@bootlin.com/
>     and Alexander Kanavin in
>     https://lore.kernel.org/yocto-docs/CANNYZj-7C_DeNgVCyRuHRCXZNX-2JPcnysKLn7gQq8w-4aAkNA@mail.gmail.com/
> ---
>   documentation/contributor-guide/recipe-style-guide.rst | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)


Many thanks for these updates!
For all of them: Reviewed-by: Michael Opdenacker 
<michael.opdenacker@bootlin.com>
Merged into "master-next".

Thanks to you, the Contributor Guide has made nice progress this week, 
and we now have a better and up to date reference for people adding 
patches to recipes.

Come back anytime :)

Cheers
Michael.

-- 

Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH v2 5/5] contributor-guide: style-guide: discourage using Pending patch status

[Roland Hieber]

On Fri, Sep 22, 2023 at 03:14:25PM +0200, Michael Opdenacker via lists.yoctoproject.org wrote:
> Hi Roland
> 
> On 22.09.23 at 12:00, Roland Hieber wrote:
> > Suggested-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> > Suggested-by: Alexander Kanavin <alex.kanavin@gmail.com>
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> > PATCH v2:
> >   * new patch in v2, based on feedback by Michael Opdenacker in v1
> >     https://lore.kernel.org/yocto-docs/12ef125b-5466-408c-ec8c-c5bef1510bc7@bootlin.com/
> >     and Alexander Kanavin in
> >     https://lore.kernel.org/yocto-docs/CANNYZj-7C_DeNgVCyRuHRCXZNX-2JPcnysKLn7gQq8w-4aAkNA@mail.gmail.com/
> > ---
> >   documentation/contributor-guide/recipe-style-guide.rst | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> 
> Many thanks for these updates!
> For all of them: Reviewed-by: Michael Opdenacker
> <michael.opdenacker@bootlin.com>
> Merged into "master-next".
> 
> Thanks to you, the Contributor Guide has made nice progress this week, and
> we now have a better and up to date reference for people adding patches to
> recipes.

Whee \o/ And thanks for your fixups too!

 - Roland

> Come back anytime :)
> 
> Cheers
> Michael.
> 
> -- 
> 
> Michael Opdenacker, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#4296): https://lists.yoctoproject.org/g/docs/message/4296
> Mute This Topic: https://lists.yoctoproject.org/mt/101518215/5053951
> Group Owner: docs+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/docs/unsub [rhi@pengutronix.de]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |