[dpdk-dev] devtools: list stable commits do not have fixline
Checks
Commit Message
Some commits for stable releases (with Cc stable tag) may not have the
fixline. For example:
http://dpdk.org/dev/patchwork/patch/23955/
It disables a feature we have implemented in last release. The feature
is done right. It's the QEMU implementaton being buggy, that we have to
disable it to workaround those buggy QEMU releases (v2.7 - v2.9). Without
such workaround, QEMU won't start when queue number >= 2.
That said, we also have to backport it to stable releases, though there
is no fixline (there was no DPDK bug to fix after all).
There should be similar cases like this. Thus, this patch makes
git-log-fixes.sh script also list those stable commits do not have
fixline.
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
devtools/git-log-fixes.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
28/04/2017 09:21, Yuanhan Liu:
> Some commits for stable releases (with Cc stable tag) may not have the
> fixline. For example:
> http://dpdk.org/dev/patchwork/patch/23955/
>
> It disables a feature we have implemented in last release. The feature
> is done right. It's the QEMU implementaton being buggy, that we have to
> disable it to workaround those buggy QEMU releases (v2.7 - v2.9). Without
> such workaround, QEMU won't start when queue number >= 2.
>
> That said, we also have to backport it to stable releases, though there
> is no fixline (there was no DPDK bug to fix after all).
How do we know where should it be backported?
It is fixing a bug with a correct implementation because of
a buggy dependency. But it is still a bug.
So I think we should put a Fixes: line.
>
> There should be similar cases like this. Thus, this patch makes
> git-log-fixes.sh script also list those stable commits do not have
> fixline.
I am against putting Cc: stable without Fixes: line.
However, this patch is harmless.
On Fri, Apr 28, 2017 at 10:15:46AM +0200, Thomas Monjalon wrote:
> 28/04/2017 09:21, Yuanhan Liu:
> > Some commits for stable releases (with Cc stable tag) may not have the
> > fixline. For example:
> > http://dpdk.org/dev/patchwork/patch/23955/
> >
> > It disables a feature we have implemented in last release. The feature
> > is done right. It's the QEMU implementaton being buggy, that we have to
> > disable it to workaround those buggy QEMU releases (v2.7 - v2.9). Without
> > such workaround, QEMU won't start when queue number >= 2.
> >
> > That said, we also have to backport it to stable releases, though there
> > is no fixline (there was no DPDK bug to fix after all).
>
> How do we know where should it be backported?
Good question. As a stable maintainer, I may not know. But the developer
should know. For such case, he may add something like:
Cc: stable@dpdk.org # for v17.02+
It's a trick used widely in kernel and QEMU community.
> It is fixing a bug with a correct implementation because of
> a buggy dependency. But it is still a bug.
> So I think we should put a Fixes: line.
I don't have strong objection to this. It just doesn't make too much
sense to me: there is no bug in the DPDK implementation after all.
But if you insist, I'm okay with it.
> >
> > There should be similar cases like this. Thus, this patch makes
> > git-log-fixes.sh script also list those stable commits do not have
> > fixline.
>
> I am against putting Cc: stable without Fixes: line.
General, yes. And luckily, that should be rare.
> However, this patch is harmless.
Yes. Moreover, we should not miss some important fixes with it.
--yliu
28/04/2017 10:27, Yuanhan Liu:
> On Fri, Apr 28, 2017 at 10:15:46AM +0200, Thomas Monjalon wrote:
> > 28/04/2017 09:21, Yuanhan Liu:
> > > Some commits for stable releases (with Cc stable tag) may not have the
> > > fixline. For example:
> > > http://dpdk.org/dev/patchwork/patch/23955/
> > >
> > > It disables a feature we have implemented in last release. The feature
> > > is done right. It's the QEMU implementaton being buggy, that we have to
> > > disable it to workaround those buggy QEMU releases (v2.7 - v2.9). Without
> > > such workaround, QEMU won't start when queue number >= 2.
> > >
> > > That said, we also have to backport it to stable releases, though there
> > > is no fixline (there was no DPDK bug to fix after all).
> >
> > How do we know where should it be backported?
>
> Good question. As a stable maintainer, I may not know. But the developer
> should know. For such case, he may add something like:
>
> Cc: stable@dpdk.org # for v17.02+
It breaks backport semi-automation.
> It's a trick used widely in kernel and QEMU community.
>
> > It is fixing a bug with a correct implementation because of
> > a buggy dependency. But it is still a bug.
> > So I think we should put a Fixes: line.
>
> I don't have strong objection to this. It just doesn't make too much
> sense to me: there is no bug in the DPDK implementation after all.
>
> But if you insist, I'm okay with it.
Yes I insist :)
It is fixing code to work with some dependencies.
> > > There should be similar cases like this. Thus, this patch makes
> > > git-log-fixes.sh script also list those stable commits do not have
> > > fixline.
> >
> > I am against putting Cc: stable without Fixes: line.
>
> General, yes. And luckily, that should be rare.
>
> > However, this patch is harmless.
>
> Yes. Moreover, we should not miss some important fixes with it.
On Fri, Apr 28, 2017 at 11:00:07AM +0200, Thomas Monjalon wrote:
> 28/04/2017 10:27, Yuanhan Liu:
> > On Fri, Apr 28, 2017 at 10:15:46AM +0200, Thomas Monjalon wrote:
> > > 28/04/2017 09:21, Yuanhan Liu:
> > > > Some commits for stable releases (with Cc stable tag) may not have the
> > > > fixline. For example:
> > > > http://dpdk.org/dev/patchwork/patch/23955/
> > > >
> > > > It disables a feature we have implemented in last release. The feature
> > > > is done right. It's the QEMU implementaton being buggy, that we have to
> > > > disable it to workaround those buggy QEMU releases (v2.7 - v2.9). Without
> > > > such workaround, QEMU won't start when queue number >= 2.
> > > >
> > > > That said, we also have to backport it to stable releases, though there
> > > > is no fixline (there was no DPDK bug to fix after all).
> > >
> > > How do we know where should it be backported?
> >
> > Good question. As a stable maintainer, I may not know. But the developer
> > should know. For such case, he may add something like:
> >
> > Cc: stable@dpdk.org # for v17.02+
>
> It breaks backport semi-automation.
But it should be (easily) fixed.
> > It's a trick used widely in kernel and QEMU community.
> >
> > > It is fixing a bug with a correct implementation because of
> > > a buggy dependency. But it is still a bug.
> > > So I think we should put a Fixes: line.
> >
> > I don't have strong objection to this. It just doesn't make too much
> > sense to me: there is no bug in the DPDK implementation after all.
> >
> > But if you insist, I'm okay with it.
>
> Yes I insist :)
> It is fixing code to work with some dependencies.
Okay.
Besides, okay to merge this patch? As you stated, it does no harm.
--yliu
28/04/2017 09:21, Yuanhan Liu:
> Some commits for stable releases (with Cc stable tag) may not have the
> fixline. For example:
> http://dpdk.org/dev/patchwork/patch/23955/
>
> It disables a feature we have implemented in last release. The feature
> is done right. It's the QEMU implementaton being buggy, that we have to
> disable it to workaround those buggy QEMU releases (v2.7 - v2.9). Without
> such workaround, QEMU won't start when queue number >= 2.
>
> That said, we also have to backport it to stable releases, though there
> is no fixline (there was no DPDK bug to fix after all).
>
> There should be similar cases like this. Thus, this patch makes
> git-log-fixes.sh script also list those stable commits do not have
> fixline.
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Applied, without the comments about the reasons for not adding a fixline,
because there should be always a fixline, as discussed in the thread.
Thanks
@@ -116,7 +116,8 @@ stable_tag () # <hash>
git log --oneline --reverse $range |
while read id headline ; do
origins=$(origin_filter $id)
- [ -n "$origins" ] || echo "$headline" | grep -q fix || continue
+ stable=$(stable_tag $id)
+ [ "$stable" = "S" ] || [ -n "$origins" ] || echo "$headline" | grep -q fix || continue
version=$(commit_version $id)
if [ -n "$origins" ] ; then
origver="$(origin_version $origins)"
@@ -126,6 +127,5 @@ while read id headline ; do
else
origver='N/A'
fi
- stable=$(stable_tag $id)
printf '%s %7s %s %s (%s)\n' $version $id $stable "$headline" "$origver"
done