diff mbox series

[v2] Introduce a description of a new optional tag for Backports

Message ID 20200410164942.9747-1-sstabellini@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v2] Introduce a description of a new optional tag for Backports | expand

Commit Message

Stefano Stabellini April 10, 2020, 4:49 p.m. UTC
Create a new document under docs/process to describe our special tags.
For now, only add the new backport tag.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wl@xen.org>
CC: jbeulich@suse.com
CC: george.dunlap@citrix.com
CC: julien@xen.org
CC: lars.kurth@citrix.com
CC: andrew.cooper3@citrix.com
CC: konrad.wilk@oracle.com

---

This is the original thread: https://marc.info/?l=xen-devel&m=157324027614941

The backport tag was agreed upon. George requested the file to be
renamed to something more generic, where we could add more information
later.

I kept the original content and acked-by. I renamed the file to
tags.pandoc.
---
 docs/process/tags.pandoc | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 docs/process/tags.pandoc

Comments

Julien Grall April 10, 2020, 5:51 p.m. UTC | #1
Hi Stefano,

On 10/04/2020 17:49, Stefano Stabellini wrote:
> Create a new document under docs/process to describe our special tags.
> For now, only add the new backport tag.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Acked-by: Wei Liu <wl@xen.org>

Acked-by: Julien Grall <jgrall@amazon.com>

It would be good to get an ack by from Jan as he is the maintainer for 
stable's branch.

Cheers,

> CC: jbeulich@suse.com
> CC: george.dunlap@citrix.com
> CC: julien@xen.org
> CC: lars.kurth@citrix.com
> CC: andrew.cooper3@citrix.com
> CC: konrad.wilk@oracle.com
> 
> ---
> 
> This is the original thread: https://marc.info/?l=xen-devel&m=157324027614941
> 
> The backport tag was agreed upon. George requested the file to be
> renamed to something more generic, where we could add more information
> later.
> 
> I kept the original content and acked-by. I renamed the file to
> tags.pandoc.
> ---
>   docs/process/tags.pandoc | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>   create mode 100644 docs/process/tags.pandoc
> 
> diff --git a/docs/process/tags.pandoc b/docs/process/tags.pandoc
> new file mode 100644
> index 0000000000..e570efdcc8
> --- /dev/null
> +++ b/docs/process/tags.pandoc
> @@ -0,0 +1,23 @@
> +Backport Tag
> +------------
> +
> +A backport tag is an optional tag in the commit message to request a
> +given commit to be backported to the stable trees:
> +
> +    Backport: all
> +
> +It marks a commit for being a candidate for backports to all relevant
> +trees.
> +
> +    Backport: 4.9+
> +
> +It marks a commit for being a candidate for backports to all stable
> +trees from 4.9 onward.
> +
> +Maintainers request the Backport tag to be added on commit.
> +Contributors are also welcome to mark their patches with the Backport
> +tag when they deem appropriate. Maintainers will request for it to be
> +removed when that is not the case.
> +
> +Please note that the Backport tag is a **request** for backport, which
> +will still need to be evaluated by the stable tree maintainers.
>
Jan Beulich April 14, 2020, 7:52 a.m. UTC | #2
On 10.04.2020 18:49, Stefano Stabellini wrote:
> Create a new document under docs/process to describe our special tags.
> For now, only add the new backport tag.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Acked-by: Wei Liu <wl@xen.org>
> CC: jbeulich@suse.com
> CC: george.dunlap@citrix.com
> CC: julien@xen.org
> CC: lars.kurth@citrix.com
> CC: andrew.cooper3@citrix.com
> CC: konrad.wilk@oracle.com
> 
> ---
> 
> This is the original thread: https://marc.info/?l=xen-devel&m=157324027614941
> 
> The backport tag was agreed upon.

Well, sort of.

> George requested the file to be
> renamed to something more generic, where we could add more information
> later.
> 
> I kept the original content and acked-by. I renamed the file to
> tags.pandoc.
> ---
>  docs/process/tags.pandoc | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 docs/process/tags.pandoc
> 
> diff --git a/docs/process/tags.pandoc b/docs/process/tags.pandoc
> new file mode 100644
> index 0000000000..e570efdcc8
> --- /dev/null
> +++ b/docs/process/tags.pandoc
> @@ -0,0 +1,23 @@
> +Backport Tag
> +------------
> +
> +A backport tag is an optional tag in the commit message to request a
> +given commit to be backported to the stable trees:

Insert "fully maintained"?

> +    Backport: all
> +
> +It marks a commit for being a candidate for backports to all relevant
> +trees.

I'm unconvinced of the utility of this form - what "all" resolves to
changes over time. There's almost always a first version where a
particular issue was introduced. If we want this to be generally
useful, imo we shouldn't limit the scope of the tag to the upstream
maintained stable trees.

> +    Backport: 4.9+
> +
> +It marks a commit for being a candidate for backports to all stable
> +trees from 4.9 onward.
> +
> +Maintainers request the Backport tag to be added on commit.
> +Contributors are also welcome to mark their patches with the Backport
> +tag when they deem appropriate. Maintainers will request for it to be
> +removed when that is not the case.
> +
> +Please note that the Backport tag is a **request** for backport, which
> +will still need to be evaluated by the stable tree maintainers.

Now that we see more widespread use of the Fixes: tag, with there
being effectively some overlap between the information conveyed I
think there should be some mention of this. Not the least there's the
risk of the Backport: one to become stale when a flaky commit gets
backported - the Fixes: tag doesn't have this issue.

Jan
Stefano Stabellini April 14, 2020, 4:54 p.m. UTC | #3
On Tue, 14 Apr 2020, Jan Beulich wrote:
> On 10.04.2020 18:49, Stefano Stabellini wrote:
> > Create a new document under docs/process to describe our special tags.
> > For now, only add the new backport tag.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> > Acked-by: Wei Liu <wl@xen.org>
> > CC: jbeulich@suse.com
> > CC: george.dunlap@citrix.com
> > CC: julien@xen.org
> > CC: lars.kurth@citrix.com
> > CC: andrew.cooper3@citrix.com
> > CC: konrad.wilk@oracle.com
> > 
> > ---
> > 
> > This is the original thread: https://marc.info/?l=xen-devel&m=157324027614941
> > 
> > The backport tag was agreed upon.
> 
> Well, sort of.
> 
> > George requested the file to be
> > renamed to something more generic, where we could add more information
> > later.
> > 
> > I kept the original content and acked-by. I renamed the file to
> > tags.pandoc.
> > ---
> >  docs/process/tags.pandoc | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >  create mode 100644 docs/process/tags.pandoc
> > 
> > diff --git a/docs/process/tags.pandoc b/docs/process/tags.pandoc
> > new file mode 100644
> > index 0000000000..e570efdcc8
> > --- /dev/null
> > +++ b/docs/process/tags.pandoc
> > @@ -0,0 +1,23 @@
> > +Backport Tag
> > +------------
> > +
> > +A backport tag is an optional tag in the commit message to request a
> > +given commit to be backported to the stable trees:
> 
> Insert "fully maintained"?

Yep I'll add.


> > +    Backport: all
> > +
> > +It marks a commit for being a candidate for backports to all relevant
> > +trees.
> 
> I'm unconvinced of the utility of this form - what "all" resolves to
> changes over time. There's almost always a first version where a
> particular issue was introduced. If we want this to be generally
> useful, imo we shouldn't limit the scope of the tag to the upstream
> maintained stable trees.

The reason why I suggested also to have a "wildcard" version of this
tag, is that the person adding the tag (could be the contributor trying
to be helpful) might not know exactly to which stable trees the patch
should be backported to.

Writing this sentence, I realize that I really meant "any" rather than
"all". Would you prefer if I used "any"? Or we could even suggest to leave
it black like this:

  Backport:

But it looks a bit weird.


> > +    Backport: 4.9+
> > +
> > +It marks a commit for being a candidate for backports to all stable
> > +trees from 4.9 onward.
> > +
> > +Maintainers request the Backport tag to be added on commit.
> > +Contributors are also welcome to mark their patches with the Backport
> > +tag when they deem appropriate. Maintainers will request for it to be
> > +removed when that is not the case.
> > +
> > +Please note that the Backport tag is a **request** for backport, which
> > +will still need to be evaluated by the stable tree maintainers.
> 
> Now that we see more widespread use of the Fixes: tag, with there
> being effectively some overlap between the information conveyed I
> think there should be some mention of this. Not the least there's the
> risk of the Backport: one to become stale when a flaky commit gets
> backported - the Fixes: tag doesn't have this issue.

Yes, that's true, but "Fixes" cannot always be used. I can add a
statement like: "When possible use the Fixes tag."

Also, I can pull in the description of Fixes and add it to this file
too.


Fixes tag
---------

If your patch fixes a bug in a specific commit, e.g. you found an issue using
``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
the SHA-1 ID, and the one line summary.  Do not split the tag across multiple
lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify
parsing scripts.  For example::

	Fixes: 41548c5472a "mem_sharing: VM forking"

The following ``git config`` settings can be used to add a pretty format for
outputting the above style in the ``git log`` or ``git show`` commands::

	[core]
		abbrev = 12
	[pretty]
		fixes = Fixes: %h (\"%s\")
Jan Beulich April 15, 2020, 6:23 a.m. UTC | #4
On 14.04.2020 18:54, Stefano Stabellini wrote:
> On Tue, 14 Apr 2020, Jan Beulich wrote:
>> On 10.04.2020 18:49, Stefano Stabellini wrote:
>>> Create a new document under docs/process to describe our special tags.
>>> For now, only add the new backport tag.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>> Acked-by: Wei Liu <wl@xen.org>
>>> CC: jbeulich@suse.com
>>> CC: george.dunlap@citrix.com
>>> CC: julien@xen.org
>>> CC: lars.kurth@citrix.com
>>> CC: andrew.cooper3@citrix.com
>>> CC: konrad.wilk@oracle.com
>>>
>>> ---
>>>
>>> This is the original thread: https://marc.info/?l=xen-devel&m=157324027614941
>>>
>>> The backport tag was agreed upon.
>>
>> Well, sort of.
>>
>>> George requested the file to be
>>> renamed to something more generic, where we could add more information
>>> later.
>>>
>>> I kept the original content and acked-by. I renamed the file to
>>> tags.pandoc.
>>> ---
>>>  docs/process/tags.pandoc | 23 +++++++++++++++++++++++
>>>  1 file changed, 23 insertions(+)
>>>  create mode 100644 docs/process/tags.pandoc
>>>
>>> diff --git a/docs/process/tags.pandoc b/docs/process/tags.pandoc
>>> new file mode 100644
>>> index 0000000000..e570efdcc8
>>> --- /dev/null
>>> +++ b/docs/process/tags.pandoc
>>> @@ -0,0 +1,23 @@
>>> +Backport Tag
>>> +------------
>>> +
>>> +A backport tag is an optional tag in the commit message to request a
>>> +given commit to be backported to the stable trees:
>>
>> Insert "fully maintained"?
> 
> Yep I'll add.
> 
> 
>>> +    Backport: all
>>> +
>>> +It marks a commit for being a candidate for backports to all relevant
>>> +trees.
>>
>> I'm unconvinced of the utility of this form - what "all" resolves to
>> changes over time. There's almost always a first version where a
>> particular issue was introduced. If we want this to be generally
>> useful, imo we shouldn't limit the scope of the tag to the upstream
>> maintained stable trees.
> 
> The reason why I suggested also to have a "wildcard" version of this
> tag, is that the person adding the tag (could be the contributor trying
> to be helpful) might not know exactly to which stable trees the patch
> should be backported to.
> 
> Writing this sentence, I realize that I really meant "any" rather than
> "all". Would you prefer if I used "any"? Or we could even suggest to leave
> it black like this:
> 
>   Backport:
> 
> But it looks a bit weird.

Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
"unknown"? Nevertheless, I still think people asking for a backport
should be nudged towards determining the applicable range; them not
doing so effectively pushes the burden to the general maintainers or
the stable tree ones, both of which scales less well. Omitting the
tag if they don't want to invest the time would to me then seem to
be the cleanest alternative. Albeit I'm sure views here will vary.

>>> +    Backport: 4.9+
>>> +
>>> +It marks a commit for being a candidate for backports to all stable
>>> +trees from 4.9 onward.
>>> +
>>> +Maintainers request the Backport tag to be added on commit.
>>> +Contributors are also welcome to mark their patches with the Backport
>>> +tag when they deem appropriate. Maintainers will request for it to be
>>> +removed when that is not the case.
>>> +
>>> +Please note that the Backport tag is a **request** for backport, which
>>> +will still need to be evaluated by the stable tree maintainers.
>>
>> Now that we see more widespread use of the Fixes: tag, with there
>> being effectively some overlap between the information conveyed I
>> think there should be some mention of this. Not the least there's the
>> risk of the Backport: one to become stale when a flaky commit gets
>> backported - the Fixes: tag doesn't have this issue.
> 
> Yes, that's true, but "Fixes" cannot always be used. I can add a
> statement like: "When possible use the Fixes tag."

Yes please.

> Also, I can pull in the description of Fixes and add it to this file
> too.

This would be much appreciated.

> Fixes tag
> ---------
> 
> If your patch fixes a bug in a specific commit, e.g. you found an issue using
> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
> the SHA-1 ID, and the one line summary.  Do not split the tag across multiple
> lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify
> parsing scripts.

I think this non-splitting rule, being applicable to all tags, might better
live prominently at the top of the file.

Jan

>  For example::
> 
> 	Fixes: 41548c5472a "mem_sharing: VM forking"
> 
> The following ``git config`` settings can be used to add a pretty format for
> outputting the above style in the ``git log`` or ``git show`` commands::
> 
> 	[core]
> 		abbrev = 12
> 	[pretty]
> 		fixes = Fixes: %h (\"%s\")
>
George Dunlap April 15, 2020, 9:43 a.m. UTC | #5
> On Apr 15, 2020, at 7:23 AM, Jan Beulich <JBeulich@suse.com> wrote:
> 
> On 14.04.2020 18:54, Stefano Stabellini wrote:
>> On Tue, 14 Apr 2020, Jan Beulich wrote:
>>> On 10.04.2020 18:49, Stefano Stabellini wrote:
>> 
[snip]
>>>> +    Backport: all
>>>> +
>>>> +It marks a commit for being a candidate for backports to all relevant
>>>> +trees.
>>> 
>>> I'm unconvinced of the utility of this form - what "all" resolves to
>>> changes over time. There's almost always a first version where a
>>> particular issue was introduced. If we want this to be generally
>>> useful, imo we shouldn't limit the scope of the tag to the upstream
>>> maintained stable trees.
>> 
>> The reason why I suggested also to have a "wildcard" version of this
>> tag, is that the person adding the tag (could be the contributor trying
>> to be helpful) might not know exactly to which stable trees the patch
>> should be backported to.
>> 
>> Writing this sentence, I realize that I really meant "any" rather than
>> "all". Would you prefer if I used "any"? Or we could even suggest to leave
>> it black like this:
>> 
>>  Backport:
>> 
>> But it looks a bit weird.
> 
> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
> "unknown"? Nevertheless, I still think people asking for a backport
> should be nudged towards determining the applicable range; them not
> doing so effectively pushes the burden to the general maintainers or
> the stable tree ones, both of which scales less well. Omitting the
> tag if they don't want to invest the time would to me then seem to
> be the cleanest alternative. Albeit I'm sure views here will vary.

FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me.

 -George
Julien Grall April 15, 2020, 9:49 a.m. UTC | #6
On 15/04/2020 10:43, George Dunlap wrote:
> 
> 
>> On Apr 15, 2020, at 7:23 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>
>> On 14.04.2020 18:54, Stefano Stabellini wrote:
>>> On Tue, 14 Apr 2020, Jan Beulich wrote:
>>>> On 10.04.2020 18:49, Stefano Stabellini wrote:
>>>
> [snip]
>>>>> +    Backport: all
>>>>> +
>>>>> +It marks a commit for being a candidate for backports to all relevant
>>>>> +trees.
>>>>
>>>> I'm unconvinced of the utility of this form - what "all" resolves to
>>>> changes over time. There's almost always a first version where a
>>>> particular issue was introduced. If we want this to be generally
>>>> useful, imo we shouldn't limit the scope of the tag to the upstream
>>>> maintained stable trees.
>>>
>>> The reason why I suggested also to have a "wildcard" version of this
>>> tag, is that the person adding the tag (could be the contributor trying
>>> to be helpful) might not know exactly to which stable trees the patch
>>> should be backported to.
>>>
>>> Writing this sentence, I realize that I really meant "any" rather than
>>> "all". Would you prefer if I used "any"? Or we could even suggest to leave
>>> it black like this:
>>>
>>>   Backport:
>>>
>>> But it looks a bit weird.
>>
>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
>> "unknown"? Nevertheless, I still think people asking for a backport
>> should be nudged towards determining the applicable range; them not
>> doing so effectively pushes the burden to the general maintainers or
>> the stable tree ones, both of which scales less well. Omitting the
>> tag if they don't want to invest the time would to me then seem to
>> be the cleanest alternative. Albeit I'm sure views here will vary.
> 
> FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me.

If you ask the contributor to do the work then you need to give guidance 
on the "older" version you can specify in Backport.

For instance, let say the bug was introduced in Xen 4.2. Are we allowing 
the user to specify Backport: 4.2+ or should it be 4.11+?

I would favor the former as this helps for downstream user which haven't 
yet moved to the supported stable tree.

Cheers,
George Dunlap April 15, 2020, 9:56 a.m. UTC | #7
> On Apr 15, 2020, at 10:49 AM, Julien Grall <julien@xen.org> wrote:
> 
> 
> 
> On 15/04/2020 10:43, George Dunlap wrote:
>>> On Apr 15, 2020, at 7:23 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>> 
>>> On 14.04.2020 18:54, Stefano Stabellini wrote:
>>>> On Tue, 14 Apr 2020, Jan Beulich wrote:
>>>>> On 10.04.2020 18:49, Stefano Stabellini wrote:
>>>> 
>> [snip]
>>>>>> +    Backport: all
>>>>>> +
>>>>>> +It marks a commit for being a candidate for backports to all relevant
>>>>>> +trees.
>>>>> 
>>>>> I'm unconvinced of the utility of this form - what "all" resolves to
>>>>> changes over time. There's almost always a first version where a
>>>>> particular issue was introduced. If we want this to be generally
>>>>> useful, imo we shouldn't limit the scope of the tag to the upstream
>>>>> maintained stable trees.
>>>> 
>>>> The reason why I suggested also to have a "wildcard" version of this
>>>> tag, is that the person adding the tag (could be the contributor trying
>>>> to be helpful) might not know exactly to which stable trees the patch
>>>> should be backported to.
>>>> 
>>>> Writing this sentence, I realize that I really meant "any" rather than
>>>> "all". Would you prefer if I used "any"? Or we could even suggest to leave
>>>> it black like this:
>>>> 
>>>>  Backport:
>>>> 
>>>> But it looks a bit weird.
>>> 
>>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
>>> "unknown"? Nevertheless, I still think people asking for a backport
>>> should be nudged towards determining the applicable range; them not
>>> doing so effectively pushes the burden to the general maintainers or
>>> the stable tree ones, both of which scales less well. Omitting the
>>> tag if they don't want to invest the time would to me then seem to
>>> be the cleanest alternative. Albeit I'm sure views here will vary.
>> FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me.
> 
> If you ask the contributor to do the work then you need to give guidance on the "older" version you can specify in Backport.
> 
> For instance, let say the bug was introduced in Xen 4.2. Are we allowing the user to specify Backport: 4.2+ or should it be 4.11+?
> 
> I would favor the former as this helps for downstream user which haven't yet moved to the supported stable tree.

I agree that specifying the oldest revision possible would be helpful.

However, I don’t think finding the absolute oldest revision should be *required* — imagine a bug that was introduced between 3.2 and 3.3.  It’s also perfectly fine if you go all the way back to 4.2 and stop because you get bored, not because you found out that 4.1 didn’t need it.

On the other hand, contributors should be expected to find out if it needs to be backported *at least* to fully-supported releases.

I think whatever text we come up with should probably say that contributors are “expected” (or “required”) to specify which currently-supported releases need the backport;  but “encouraged” to specify a release as far back as possible.

 -George
Jan Beulich April 15, 2020, 10:36 a.m. UTC | #8
On 15.04.2020 11:49, Julien Grall wrote:
> 
> 
> On 15/04/2020 10:43, George Dunlap wrote:
>>
>>
>>> On Apr 15, 2020, at 7:23 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>
>>> On 14.04.2020 18:54, Stefano Stabellini wrote:
>>>> On Tue, 14 Apr 2020, Jan Beulich wrote:
>>>>> On 10.04.2020 18:49, Stefano Stabellini wrote:
>>>>
>> [snip]
>>>>>> +    Backport: all
>>>>>> +
>>>>>> +It marks a commit for being a candidate for backports to all relevant
>>>>>> +trees.
>>>>>
>>>>> I'm unconvinced of the utility of this form - what "all" resolves to
>>>>> changes over time. There's almost always a first version where a
>>>>> particular issue was introduced. If we want this to be generally
>>>>> useful, imo we shouldn't limit the scope of the tag to the upstream
>>>>> maintained stable trees.
>>>>
>>>> The reason why I suggested also to have a "wildcard" version of this
>>>> tag, is that the person adding the tag (could be the contributor trying
>>>> to be helpful) might not know exactly to which stable trees the patch
>>>> should be backported to.
>>>>
>>>> Writing this sentence, I realize that I really meant "any" rather than
>>>> "all". Would you prefer if I used "any"? Or we could even suggest to leave
>>>> it black like this:
>>>>
>>>>   Backport:
>>>>
>>>> But it looks a bit weird.
>>>
>>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
>>> "unknown"? Nevertheless, I still think people asking for a backport
>>> should be nudged towards determining the applicable range; them not
>>> doing so effectively pushes the burden to the general maintainers or
>>> the stable tree ones, both of which scales less well. Omitting the
>>> tag if they don't want to invest the time would to me then seem to
>>> be the cleanest alternative. Albeit I'm sure views here will vary.
>>
>> FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me.
> 
> If you ask the contributor to do the work then you need to give guidance on the "older" version you can specify in Backport.
> 
> For instance, let say the bug was introduced in Xen 4.2. Are we allowing the user to specify Backport: 4.2+ or should it be 4.11+?
> 
> I would favor the former as this helps for downstream user which haven't yet moved to the supported stable tree.

In an earlier reply I did suggest the same, for the same reason.

Jan
Jan Beulich April 15, 2020, 10:44 a.m. UTC | #9
On 15.04.2020 11:56, George Dunlap wrote:
> 
> 
>> On Apr 15, 2020, at 10:49 AM, Julien Grall <julien@xen.org> wrote:
>>
>>
>>
>> On 15/04/2020 10:43, George Dunlap wrote:
>>>> On Apr 15, 2020, at 7:23 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>
>>>> On 14.04.2020 18:54, Stefano Stabellini wrote:
>>>>> On Tue, 14 Apr 2020, Jan Beulich wrote:
>>>>>> On 10.04.2020 18:49, Stefano Stabellini wrote:
>>>>>
>>> [snip]
>>>>>>> +    Backport: all
>>>>>>> +
>>>>>>> +It marks a commit for being a candidate for backports to all relevant
>>>>>>> +trees.
>>>>>>
>>>>>> I'm unconvinced of the utility of this form - what "all" resolves to
>>>>>> changes over time. There's almost always a first version where a
>>>>>> particular issue was introduced. If we want this to be generally
>>>>>> useful, imo we shouldn't limit the scope of the tag to the upstream
>>>>>> maintained stable trees.
>>>>>
>>>>> The reason why I suggested also to have a "wildcard" version of this
>>>>> tag, is that the person adding the tag (could be the contributor trying
>>>>> to be helpful) might not know exactly to which stable trees the patch
>>>>> should be backported to.
>>>>>
>>>>> Writing this sentence, I realize that I really meant "any" rather than
>>>>> "all". Would you prefer if I used "any"? Or we could even suggest to leave
>>>>> it black like this:
>>>>>
>>>>>  Backport:
>>>>>
>>>>> But it looks a bit weird.
>>>>
>>>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
>>>> "unknown"? Nevertheless, I still think people asking for a backport
>>>> should be nudged towards determining the applicable range; them not
>>>> doing so effectively pushes the burden to the general maintainers or
>>>> the stable tree ones, both of which scales less well. Omitting the
>>>> tag if they don't want to invest the time would to me then seem to
>>>> be the cleanest alternative. Albeit I'm sure views here will vary.
>>> FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me.
>>
>> If you ask the contributor to do the work then you need to give guidance on the "older" version you can specify in Backport.
>>
>> For instance, let say the bug was introduced in Xen 4.2. Are we allowing the user to specify Backport: 4.2+ or should it be 4.11+?
>>
>> I would favor the former as this helps for downstream user which haven't yet moved to the supported stable tree.
> 
> I agree that specifying the oldest revision possible would be helpful.
> 
> However, I don’t think finding the absolute oldest revision should be *required* — imagine a bug that was introduced between 3.2 and 3.3.  It’s also perfectly fine if you go all the way back to 4.2 and stop because you get bored, not because you found out that 4.1 didn’t need it.

In which case I'd like there to be a (canonical?) way of expressing
this, like in XSAs we say "at least back to" in such a case.

Jan
Stefano Stabellini April 16, 2020, 9:14 p.m. UTC | #10
On Wed, 15 Apr 2020, Jan Beulich wrote:
> On 15.04.2020 11:56, George Dunlap wrote:
> > 
> > 
> >> On Apr 15, 2020, at 10:49 AM, Julien Grall <julien@xen.org> wrote:
> >>
> >>
> >>
> >> On 15/04/2020 10:43, George Dunlap wrote:
> >>>> On Apr 15, 2020, at 7:23 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >>>>
> >>>> On 14.04.2020 18:54, Stefano Stabellini wrote:
> >>>>> On Tue, 14 Apr 2020, Jan Beulich wrote:
> >>>>>> On 10.04.2020 18:49, Stefano Stabellini wrote:
> >>>>>
> >>> [snip]
> >>>>>>> +    Backport: all
> >>>>>>> +
> >>>>>>> +It marks a commit for being a candidate for backports to all relevant
> >>>>>>> +trees.
> >>>>>>
> >>>>>> I'm unconvinced of the utility of this form - what "all" resolves to
> >>>>>> changes over time. There's almost always a first version where a
> >>>>>> particular issue was introduced. If we want this to be generally
> >>>>>> useful, imo we shouldn't limit the scope of the tag to the upstream
> >>>>>> maintained stable trees.
> >>>>>
> >>>>> The reason why I suggested also to have a "wildcard" version of this
> >>>>> tag, is that the person adding the tag (could be the contributor trying
> >>>>> to be helpful) might not know exactly to which stable trees the patch
> >>>>> should be backported to.
> >>>>>
> >>>>> Writing this sentence, I realize that I really meant "any" rather than
> >>>>> "all". Would you prefer if I used "any"? Or we could even suggest to leave
> >>>>> it black like this:
> >>>>>
> >>>>>  Backport:
> >>>>>
> >>>>> But it looks a bit weird.
> >>>>
> >>>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
> >>>> "unknown"? Nevertheless, I still think people asking for a backport
> >>>> should be nudged towards determining the applicable range; them not
> >>>> doing so effectively pushes the burden to the general maintainers or
> >>>> the stable tree ones, both of which scales less well. Omitting the
> >>>> tag if they don't want to invest the time would to me then seem to
> >>>> be the cleanest alternative. Albeit I'm sure views here will vary.
> >>> FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me.
> >>
> >> If you ask the contributor to do the work then you need to give guidance on the "older" version you can specify in Backport.
> >>
> >> For instance, let say the bug was introduced in Xen 4.2. Are we allowing the user to specify Backport: 4.2+ or should it be 4.11+?
> >>
> >> I would favor the former as this helps for downstream user which haven't yet moved to the supported stable tree.
> > 
> > I agree that specifying the oldest revision possible would be helpful.
> > 
> > However, I don’t think finding the absolute oldest revision should be *required* — imagine a bug that was introduced between 3.2 and 3.3.  It’s also perfectly fine if you go all the way back to 4.2 and stop because you get bored, not because you found out that 4.1 didn’t need it.

I dropped the definition of "Backport: all", and adopted George's
suggested wording:

  The backport requester is expected to specify which currently supported
  releases need the backport; but encouraged to specify a release as far
  back as possible which applies.


> In which case I'd like there to be a (canonical?) way of expressing
> this, like in XSAs we say "at least back to" in such a case.

I couldn't think of anything better than:

  Backport: 4.9+ # maybe older

We probably don't need to codify something like that in this document.


As an alternative we could perhaps reuse the "Backport: all" idea in a
different light for this new purpose.

I expect that in these cases where we don't know the oldest affected
tree, all the currently maintained stable trees will have to get the
backport. So maybe we could use one of the following:

  Backport: all
  Backport: oldest
  Backport: oldest-unknown

To express that the fix needs to be backported to *all* the currently
maintained stable trees, but there might be also other older
unmaintained trees that could be affected; we don't know for sure how
far back it should go.
Jan Beulich April 17, 2020, 6:51 a.m. UTC | #11
On 16.04.2020 23:14, Stefano Stabellini wrote:
> On Wed, 15 Apr 2020, Jan Beulich wrote:
>> On 15.04.2020 11:56, George Dunlap wrote:
>>>
>>>
>>>> On Apr 15, 2020, at 10:49 AM, Julien Grall <julien@xen.org> wrote:
>>>>
>>>>
>>>>
>>>> On 15/04/2020 10:43, George Dunlap wrote:
>>>>>> On Apr 15, 2020, at 7:23 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>
>>>>>> On 14.04.2020 18:54, Stefano Stabellini wrote:
>>>>>>> On Tue, 14 Apr 2020, Jan Beulich wrote:
>>>>>>>> On 10.04.2020 18:49, Stefano Stabellini wrote:
>>>>>>>
>>>>> [snip]
>>>>>>>>> +    Backport: all
>>>>>>>>> +
>>>>>>>>> +It marks a commit for being a candidate for backports to all relevant
>>>>>>>>> +trees.
>>>>>>>>
>>>>>>>> I'm unconvinced of the utility of this form - what "all" resolves to
>>>>>>>> changes over time. There's almost always a first version where a
>>>>>>>> particular issue was introduced. If we want this to be generally
>>>>>>>> useful, imo we shouldn't limit the scope of the tag to the upstream
>>>>>>>> maintained stable trees.
>>>>>>>
>>>>>>> The reason why I suggested also to have a "wildcard" version of this
>>>>>>> tag, is that the person adding the tag (could be the contributor trying
>>>>>>> to be helpful) might not know exactly to which stable trees the patch
>>>>>>> should be backported to.
>>>>>>>
>>>>>>> Writing this sentence, I realize that I really meant "any" rather than
>>>>>>> "all". Would you prefer if I used "any"? Or we could even suggest to leave
>>>>>>> it black like this:
>>>>>>>
>>>>>>>  Backport:
>>>>>>>
>>>>>>> But it looks a bit weird.
>>>>>>
>>>>>> Indeed. Instead of "all" or "any", how about "yes", "unspecified", or
>>>>>> "unknown"? Nevertheless, I still think people asking for a backport
>>>>>> should be nudged towards determining the applicable range; them not
>>>>>> doing so effectively pushes the burden to the general maintainers or
>>>>>> the stable tree ones, both of which scales less well. Omitting the
>>>>>> tag if they don't want to invest the time would to me then seem to
>>>>>> be the cleanest alternative. Albeit I'm sure views here will vary.
>>>>> FWIW asking people adding the tag to do the work of figuring out which versions to backport to makes sense to me.
>>>>
>>>> If you ask the contributor to do the work then you need to give guidance on the "older" version you can specify in Backport.
>>>>
>>>> For instance, let say the bug was introduced in Xen 4.2. Are we allowing the user to specify Backport: 4.2+ or should it be 4.11+?
>>>>
>>>> I would favor the former as this helps for downstream user which haven't yet moved to the supported stable tree.
>>>
>>> I agree that specifying the oldest revision possible would be helpful.
>>>
>>> However, I don’t think finding the absolute oldest revision should be *required* — imagine a bug that was introduced between 3.2 and 3.3.  It’s also perfectly fine if you go all the way back to 4.2 and stop because you get bored, not because you found out that 4.1 didn’t need it.
> 
> I dropped the definition of "Backport: all", and adopted George's
> suggested wording:
> 
>   The backport requester is expected to specify which currently supported
>   releases need the backport; but encouraged to specify a release as far
>   back as possible which applies.
> 
> 
>> In which case I'd like there to be a (canonical?) way of expressing
>> this, like in XSAs we say "at least back to" in such a case.
> 
> I couldn't think of anything better than:
> 
>   Backport: 4.9+ # maybe older
> 
> We probably don't need to codify something like that in this document.

The suggestion looks fine to me, and people using slightly varying
wording wouldn't be a problem either.

> As an alternative we could perhaps reuse the "Backport: all" idea in a
> different light for this new purpose.
> 
> I expect that in these cases where we don't know the oldest affected
> tree, all the currently maintained stable trees will have to get the
> backport. So maybe we could use one of the following:
> 
>   Backport: all
>   Backport: oldest
>   Backport: oldest-unknown
> 
> To express that the fix needs to be backported to *all* the currently
> maintained stable trees, but there might be also other older
> unmaintained trees that could be affected; we don't know for sure how
> far back it should go.

My prior objection to "all" remains - it changes over time what
"currently means", rendering the tag stale quite quickly. I think
that without even providing a suggested means to create such a tag
without an explicit version specified we underline the need to
figure out a baseline from where to apply the backport.

One more thing comes to mind that may want mentioning here: If
people request a backport, I think this should take as an
implication their willingness to actually be involved in doing
the actual backporting work. Typically it's pretty simple, but
every now and then quite a bit of effort is needed. It would be
nice if the stable tree maintainers could push over some of this
burden without the requester being caught by surprise.

Jan
diff mbox series

Patch

diff --git a/docs/process/tags.pandoc b/docs/process/tags.pandoc
new file mode 100644
index 0000000000..e570efdcc8
--- /dev/null
+++ b/docs/process/tags.pandoc
@@ -0,0 +1,23 @@ 
+Backport Tag
+------------
+
+A backport tag is an optional tag in the commit message to request a
+given commit to be backported to the stable trees:
+
+    Backport: all
+
+It marks a commit for being a candidate for backports to all relevant
+trees.
+
+    Backport: 4.9+
+
+It marks a commit for being a candidate for backports to all stable
+trees from 4.9 onward.
+
+Maintainers request the Backport tag to be added on commit.
+Contributors are also welcome to mark their patches with the Backport
+tag when they deem appropriate. Maintainers will request for it to be
+removed when that is not the case.
+
+Please note that the Backport tag is a **request** for backport, which
+will still need to be evaluated by the stable tree maintainers.