DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline
@ 2017-04-28  7:21 Yuanhan Liu
  2017-04-28  8:15 ` Thomas Monjalon
  2017-04-28 10:07 ` Thomas Monjalon
  0 siblings, 2 replies; 6+ messages in thread
From: Yuanhan Liu @ 2017-04-28  7:21 UTC (permalink / raw)
  To: dev; +Cc: Thomas Monjalon, 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>
---
 devtools/git-log-fixes.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/devtools/git-log-fixes.sh b/devtools/git-log-fixes.sh
index 3bf412e..7404946 100755
--- a/devtools/git-log-fixes.sh
+++ b/devtools/git-log-fixes.sh
@@ -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
-- 
1.9.0

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

* Re: [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline
  2017-04-28  7:21 [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline Yuanhan Liu
@ 2017-04-28  8:15 ` Thomas Monjalon
  2017-04-28  8:27   ` Yuanhan Liu
  2017-04-28 10:07 ` Thomas Monjalon
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2017-04-28  8:15 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev

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.

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

* Re: [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline
  2017-04-28  8:15 ` Thomas Monjalon
@ 2017-04-28  8:27   ` Yuanhan Liu
  2017-04-28  9:00     ` Thomas Monjalon
  0 siblings, 1 reply; 6+ messages in thread
From: Yuanhan Liu @ 2017-04-28  8:27 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

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

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

* Re: [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline
  2017-04-28  8:27   ` Yuanhan Liu
@ 2017-04-28  9:00     ` Thomas Monjalon
  2017-04-28  9:01       ` Yuanhan Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2017-04-28  9:00 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev

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.

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

* Re: [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline
  2017-04-28  9:00     ` Thomas Monjalon
@ 2017-04-28  9:01       ` Yuanhan Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Yuanhan Liu @ 2017-04-28  9:01 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

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

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

* Re: [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline
  2017-04-28  7:21 [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline Yuanhan Liu
  2017-04-28  8:15 ` Thomas Monjalon
@ 2017-04-28 10:07 ` Thomas Monjalon
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2017-04-28 10:07 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: dev

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

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

end of thread, other threads:[~2017-04-28 10:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-28  7:21 [dpdk-dev] [PATCH] devtools: list stable commits do not have fixline Yuanhan Liu
2017-04-28  8:15 ` Thomas Monjalon
2017-04-28  8:27   ` Yuanhan Liu
2017-04-28  9:00     ` Thomas Monjalon
2017-04-28  9:01       ` Yuanhan Liu
2017-04-28 10:07 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).