public inbox for docs@lists.yoctoproject.org
 help / color / mirror / Atom feed
* [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches
@ 2023-09-20 10:06 Roland Hieber
  2023-09-20 10:06 ` [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples Roland Hieber
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Roland Hieber @ 2023-09-20 10:06 UTC (permalink / raw)
  To: docs; +Cc: yocto, 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.

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
This is basically v2 of "[PATCH] contributor-guide: add docs for
Upstream-Status patch headers", Message-Id:
<20230919111549.997443-2-rhi@pengutronix.de>
<https://lists.yoctoproject.org/g/docs/topic/resend_patch/101455254>
rebased onto master-next, but since it looks so different now I made a
new v1 patch out of it.

 .../contributor-guide/recipe-style-guide.rst  | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index 99105179a6b9..52ab4523c49f 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -321,7 +321,17 @@ 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::
+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.
+
+Examples
+--------
+
+Here's an example of a patch that has been submitted upstream::
 
    rpm: Adjusted the foo setting in bar
 
@@ -336,3 +346,18 @@ For example, if the patch has been submitted upstream::
 
 A future update can change the value to ``Accepted`` or ``Denied`` as
 appropriate.
+
+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



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples
  2023-09-20 10:06 [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Roland Hieber
@ 2023-09-20 10:06 ` Roland Hieber
  2023-09-20 14:16   ` [docs] " Michael Opdenacker
  2023-09-20 10:06 ` [PATCH 3/4] contributor-guide: discourage marking patches as Inappropriate Roland Hieber
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Roland Hieber @ 2023-09-20 10:06 UTC (permalink / raw)
  To: docs; +Cc: yocto, Roland Hieber

Signed-off-by: Roland Hieber <rhi@pengutronix.de>
---
 .../contributor-guide/recipe-style-guide.rst     | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index 52ab4523c49f..4faadcd122d8 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -347,6 +347,22 @@ Here's an example of a patch that 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 a pull request::
+
+   include missing sys/file.h for LOCK_EX
+
+   Upstream-Status: Backport [https://github.com/systemd/systemd/pull/28651]
+
 This should be the header of patch that fixes CVE-2015-8370 in GRUB2::
 
    grub2: Fix CVE-2015-8370
-- 
2.39.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] contributor-guide: discourage marking patches as Inappropriate
  2023-09-20 10:06 [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Roland Hieber
  2023-09-20 10:06 ` [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples Roland Hieber
@ 2023-09-20 10:06 ` Roland Hieber
  2023-09-20 14:19   ` [docs] " Michael Opdenacker
  2023-09-20 10:06 ` [PATCH 4/4] contributor-guide: deprecate "Accepted" patch status Roland Hieber
  2023-09-20 14:11 ` [docs] [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Michael Opdenacker
  3 siblings, 1 reply; 11+ messages in thread
From: Roland Hieber @ 2023-09-20 10:06 UTC (permalink / raw)
  To: docs; +Cc: yocto, Roland Hieber, Alexander Kanavin

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>
---
 .../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 4faadcd122d8..bc14c58a9759 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



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] contributor-guide: deprecate "Accepted" patch status
  2023-09-20 10:06 [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Roland Hieber
  2023-09-20 10:06 ` [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples Roland Hieber
  2023-09-20 10:06 ` [PATCH 3/4] contributor-guide: discourage marking patches as Inappropriate Roland Hieber
@ 2023-09-20 10:06 ` Roland Hieber
  2023-09-20 14:25   ` [docs] " Michael Opdenacker
  2023-09-20 14:11 ` [docs] [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Michael Opdenacker
  3 siblings, 1 reply; 11+ messages in thread
From: Roland Hieber @ 2023-09-20 10:06 UTC (permalink / raw)
  To: docs; +Cc: yocto, Roland Hieber, Michael Opdenacker

This is in accordance with the Release Notes of the gatesgarth release:

  > 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>
---
 .../contributor-guide/recipe-style-guide.rst       | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
index bc14c58a9759..ab3b94f02e7e 100644
--- a/documentation/contributor-guide/recipe-style-guide.rst
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -277,13 +277,13 @@ 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 [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).
 
-``Backport``
-   Backported from new upstream version, because we are at a fixed version,
-   include upstream version info.
+   Note: historically, ``Accepted`` was another way to mark such patches, but
+   this status is now deprecated.
 
 ``Denied``
    Not accepted by upstream, include reason in patch.
@@ -342,7 +342,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



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [docs] [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches
  2023-09-20 10:06 [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Roland Hieber
                   ` (2 preceding siblings ...)
  2023-09-20 10:06 ` [PATCH 4/4] contributor-guide: deprecate "Accepted" patch status Roland Hieber
@ 2023-09-20 14:11 ` Michael Opdenacker
  3 siblings, 0 replies; 11+ messages in thread
From: Michael Opdenacker @ 2023-09-20 14:11 UTC (permalink / raw)
  To: Roland Hieber, docs; +Cc: yocto

Hi Roland

Many thanks for the update!

See my comments below...

On 20.09.23 at 12:06, Roland Hieber wrote:
> 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.
>
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> This is basically v2 of "[PATCH] contributor-guide: add docs for
> Upstream-Status patch headers", Message-Id:
> <20230919111549.997443-2-rhi@pengutronix.de>
> <https://lists.yoctoproject.org/g/docs/topic/resend_patch/101455254>
> rebased onto master-next, but since it looks so different now I made a
> new v1 patch out of it.
>
>   .../contributor-guide/recipe-style-guide.rst  | 27 ++++++++++++++++++-
>   1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
> index 99105179a6b9..52ab4523c49f 100644
> --- a/documentation/contributor-guide/recipe-style-guide.rst
> +++ b/documentation/contributor-guide/recipe-style-guide.rst
> @@ -321,7 +321,17 @@ 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::
> +CVE patches
> +-----------


I've got an issue with this... This makes the "CVE patches" section a 
subsection of "Patch Upstream Status".
Could you instead use?

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


s/*"CVE:"* tag/``CVE:``/
to match the way Upstream-Status was introduced

> +one CVE is fixed, separate them using spaces.
> +
> +Examples
> +--------
> +
> +Here's an example of a patch that has been submitted upstream::
>   
>      rpm: Adjusted the foo setting in bar
>   
> @@ -336,3 +346,18 @@ For example, if the patch has been submitted upstream::
>   
>   A future update can change the value to ``Accepted`` or ``Denied`` as
>   appropriate.
> +
> +This should be the header of patch that fixes CVE-2015-8370 in GRUB2::

s/of patch/of the patch/

We have a macro for CVEs:
s/CVE-2015-8370/:cve:`2015-8370`/

I know, you can't know this ;-)

> +
> +   grub2: Fix CVE-2015-8370


Could you add this section to another "Examples" subsection, dedicated 
to the "CVE:" tag? This way, each section (Upstream-Status and CVE) has 
its own examples subsection, and we don't have to create an "Examples" 
section which applies only by the last two sessions (a bit weird).

Thanks in advance,
Michael.

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [docs] [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples
  2023-09-20 10:06 ` [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples Roland Hieber
@ 2023-09-20 14:16   ` Michael Opdenacker
  2023-09-21  8:52     ` Roland Hieber
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Opdenacker @ 2023-09-20 14:16 UTC (permalink / raw)
  To: Roland Hieber, docs; +Cc: yocto


On 20.09.23 at 12:06, Roland Hieber wrote:
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
>   .../contributor-guide/recipe-style-guide.rst     | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
> index 52ab4523c49f..4faadcd122d8 100644
> --- a/documentation/contributor-guide/recipe-style-guide.rst
> +++ b/documentation/contributor-guide/recipe-style-guide.rst
> @@ -347,6 +347,22 @@ Here's an example of a patch that 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 a pull request::
> +
> +   include missing sys/file.h for LOCK_EX
> +
> +   Upstream-Status: Backport [https://github.com/systemd/systemd/pull/28651]

Could we instead use an example giving a commit instead of a pull 
request? With a pull request (like this one), it's hard to see what the 
final commit was, and we have to follow the link to double check whether 
the request was merged or not.

Thanks
Michael.

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [docs] [PATCH 3/4] contributor-guide: discourage marking patches as Inappropriate
  2023-09-20 10:06 ` [PATCH 3/4] contributor-guide: discourage marking patches as Inappropriate Roland Hieber
@ 2023-09-20 14:19   ` Michael Opdenacker
  2023-09-21  8:53     ` Roland Hieber
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Opdenacker @ 2023-09-20 14:19 UTC (permalink / raw)
  To: Roland Hieber, docs; +Cc: yocto, Alexander Kanavin


On 20.09.23 at 12:06, 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>
> ---
>   .../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 4faadcd122d8..bc14c58a9759 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).


There is a formatting issue here: the second line (starting with "fixed 
upstream") has an extra leading space compared to the first one, and 
this messes-up the way the paragraph is rendered. Check by yourself ;-)

Otherwise, the rest looks good to me.
Thanks
Michael.

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [docs] [PATCH 4/4] contributor-guide: deprecate "Accepted" patch status
  2023-09-20 10:06 ` [PATCH 4/4] contributor-guide: deprecate "Accepted" patch status Roland Hieber
@ 2023-09-20 14:25   ` Michael Opdenacker
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Opdenacker @ 2023-09-20 14:25 UTC (permalink / raw)
  To: Roland Hieber; +Cc: yocto, docs


On 20.09.23 at 12:06, Roland Hieber wrote:
> This is in accordance with the Release Notes of the gatesgarth release:
>
>    > 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>
> ---
>   .../contributor-guide/recipe-style-guide.rst       | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
> index bc14c58a9759..ab3b94f02e7e 100644
> --- a/documentation/contributor-guide/recipe-style-guide.rst
> +++ b/documentation/contributor-guide/recipe-style-guide.rst
> @@ -277,13 +277,13 @@ 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 [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).
>   
> -``Backport``
> -   Backported from new upstream version, because we are at a fixed version,
> -   include upstream version info.
> +   Note: historically, ``Accepted`` was another way to mark such patches, but
> +   this status is now deprecated.


I'd remove these last 2 lines, as we'd have to remove them one day 
anyway, to keep the manual going straight to the point.
The rest looks good to me otherwise.

I actually started to make the changes by myself, but then it was hard 
to share these changes with you and other reviewers. I hope this can 
help with future patches :)

Maybe in a V3 you can add what Alex explained about the "Pending" status.

Thanks for everything!
Cheers
Michael.

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [docs] [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples
  2023-09-20 14:16   ` [docs] " Michael Opdenacker
@ 2023-09-21  8:52     ` Roland Hieber
  2023-09-22  9:25       ` Michael Opdenacker
  0 siblings, 1 reply; 11+ messages in thread
From: Roland Hieber @ 2023-09-21  8:52 UTC (permalink / raw)
  To: michael.opdenacker; +Cc: docs, yocto

On Wed, Sep 20, 2023 at 04:16:12PM +0200, Michael Opdenacker via lists.yoctoproject.org wrote:
> 
> On 20.09.23 at 12:06, Roland Hieber wrote:
> > Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> > ---
> >   .../contributor-guide/recipe-style-guide.rst     | 16 ++++++++++++++++
> >   1 file changed, 16 insertions(+)
> > 
> > diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
> > index 52ab4523c49f..4faadcd122d8 100644
> > --- a/documentation/contributor-guide/recipe-style-guide.rst
> > +++ b/documentation/contributor-guide/recipe-style-guide.rst
> > @@ -347,6 +347,22 @@ Here's an example of a patch that 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 a pull request::
> > +
> > +   include missing sys/file.h for LOCK_EX
> > +
> > +   Upstream-Status: Backport [https://github.com/systemd/systemd/pull/28651]
> 
> Could we instead use an example giving a commit instead of a pull request?
> With a pull request (like this one), it's hard to see what the final commit
> was, and we have to follow the link to double check whether the request was
> merged or not.

Yes, that also makes more sense to me. But note that GitHub URLs
pointing to a commit also work if the commit in question was not yet
merged (or even if it is only available in a fork), so you'd still have
to click on the URL or check for the commit ID in the Git repo yourself
to see if it was merged (however this way you have the commit ID
directly in the patch).

 - Roland

-- 
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 |


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [docs] [PATCH 3/4] contributor-guide: discourage marking patches as Inappropriate
  2023-09-20 14:19   ` [docs] " Michael Opdenacker
@ 2023-09-21  8:53     ` Roland Hieber
  0 siblings, 0 replies; 11+ messages in thread
From: Roland Hieber @ 2023-09-21  8:53 UTC (permalink / raw)
  To: michael.opdenacker; +Cc: docs, yocto, Alexander Kanavin

On Wed, Sep 20, 2023 at 04:19:11PM +0200, Michael Opdenacker via lists.yoctoproject.org wrote:
> 
> On 20.09.23 at 12:06, 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>
> > ---
> >   .../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 4faadcd122d8..bc14c58a9759 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).
> 
> 
> There is a formatting issue here: the second line (starting with "fixed
> upstream") has an extra leading space compared to the first one, and this
> messes-up the way the paragraph is rendered. Check by yourself ;-)

Oh indeed. I'll chalk that up to not enough coffee… :-> Thanks for
catching it!

 - Roland

-- 
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 |


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [docs] [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples
  2023-09-21  8:52     ` Roland Hieber
@ 2023-09-22  9:25       ` Michael Opdenacker
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Opdenacker @ 2023-09-22  9:25 UTC (permalink / raw)
  To: Roland Hieber; +Cc: docs, yocto

Hi Roland

On 21.09.23 at 10:52, Roland Hieber wrote:
> On Wed, Sep 20, 2023 at 04:16:12PM +0200, Michael Opdenacker via lists.yoctoproject.org wrote:
>> Could we instead use an example giving a commit instead of a pull request?
>> With a pull request (like this one), it's hard to see what the final commit
>> was, and we have to follow the link to double check whether the request was
>> merged or not.
> Yes, that also makes more sense to me. But note that GitHub URLs
> pointing to a commit also work if the commit in question was not yet
> merged (or even if it is only available in a fork), so you'd still have
> to click on the URL or check for the commit ID in the Git repo yourself
> to see if it was merged (however this way you have the commit ID
> directly in the patch).


Oh right, this makes sense.
Thanks in advance for the updates to your patch series.

Cheers
Michael.

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



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-09-22  9:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-20 10:06 [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Roland Hieber
2023-09-20 10:06 ` [PATCH 2/4] contributor-guide: recipe-style-guide: add some more patch tagging examples Roland Hieber
2023-09-20 14:16   ` [docs] " Michael Opdenacker
2023-09-21  8:52     ` Roland Hieber
2023-09-22  9:25       ` Michael Opdenacker
2023-09-20 10:06 ` [PATCH 3/4] contributor-guide: discourage marking patches as Inappropriate Roland Hieber
2023-09-20 14:19   ` [docs] " Michael Opdenacker
2023-09-21  8:53     ` Roland Hieber
2023-09-20 10:06 ` [PATCH 4/4] contributor-guide: deprecate "Accepted" patch status Roland Hieber
2023-09-20 14:25   ` [docs] " Michael Opdenacker
2023-09-20 14:11 ` [docs] [PATCH 1/4] contributor-guide: recipe-style-guide: add section about CVE patches Michael Opdenacker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox