[2/4] mk: remove broken check

Message ID 20181009021858.19216-3-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series support more ethdev iterator filters |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Thomas Monjalon Oct. 9, 2018, 2:18 a.m. UTC
  Next patch does not pass the experimental check
because it has an (unrelated) experimental function in his context.

Cc: Neil Horman <nhorman@tuxdriver.com>

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 mk/internal/rte.compile-pre.mk | 1 -
 1 file changed, 1 deletion(-)
  

Comments

Neil Horman Oct. 9, 2018, 11:43 a.m. UTC | #1
On Tue, Oct 09, 2018 at 04:18:56AM +0200, Thomas Monjalon wrote:
> Next patch does not pass the experimental check
> because it has an (unrelated) experimental function in his context.
> 
> Cc: Neil Horman <nhorman@tuxdriver.com>
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  mk/internal/rte.compile-pre.mk | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
> index a734cbbd0..acb9796fa 100644
> --- a/mk/internal/rte.compile-pre.mk
> +++ b/mk/internal/rte.compile-pre.mk
> @@ -75,7 +75,6 @@ C_TO_O_DO = @set -e; \
>  	echo $(C_TO_O_DISP); \
>  	$(C_TO_O) && \
>  	$(PMDINFO_TO_O) && \
> -	$(CHECK_EXPERIMENTAL) && \
>  	echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \
>  	sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call obj2dep,$(@)) && \
>  	rm -f $(call obj2dep,$(@)).tmp
> -- 
> 2.19.0
> 
> 

NACK.

Its a bit vague to get the meaning of your changelog, but based on the
context of the actual other patch I think what you're saying is that patch 3/4 in
this series has the __rte_experimental token in it as part of the context of a
diff, rather than as a change itself, and the check-experimental-syms script is
matching on that when it shouldn't.  The correct fix is to teach the difference
between changes and context to the check-experimental-syms.sh script, not to
break the whole thing by just removing the call to the script.


Neil
  
Thomas Monjalon Oct. 9, 2018, 11:53 a.m. UTC | #2
09/10/2018 13:43, Neil Horman:
> On Tue, Oct 09, 2018 at 04:18:56AM +0200, Thomas Monjalon wrote:
> > Next patch does not pass the experimental check
> > because it has an (unrelated) experimental function in his context.
> > 
> > Cc: Neil Horman <nhorman@tuxdriver.com>
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  mk/internal/rte.compile-pre.mk | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
> > index a734cbbd0..acb9796fa 100644
> > --- a/mk/internal/rte.compile-pre.mk
> > +++ b/mk/internal/rte.compile-pre.mk
> > @@ -75,7 +75,6 @@ C_TO_O_DO = @set -e; \
> >  	echo $(C_TO_O_DISP); \
> >  	$(C_TO_O) && \
> >  	$(PMDINFO_TO_O) && \
> > -	$(CHECK_EXPERIMENTAL) && \
> >  	echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \
> >  	sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call obj2dep,$(@)) && \
> >  	rm -f $(call obj2dep,$(@)).tmp
> 
> NACK.

Yes, of course.
I have added this patch in the series in order to allow building the patches.

> Its a bit vague to get the meaning of your changelog, but based on the
> context of the actual other patch I think what you're saying is that patch 3/4 in
> this series has the __rte_experimental token in it as part of the context of a
> diff, rather than as a change itself, and the check-experimental-syms script is
> matching on that when it shouldn't.  The correct fix is to teach the difference
> between changes and context to the check-experimental-syms.sh script, not to
> break the whole thing by just removing the call to the script.

I agree, but I don't understand how the patch context is related
to the checks done in buildtools.
Please could you fix it? It is blocking the integration of this series.

Thanks
  
Neil Horman Oct. 9, 2018, 6:11 p.m. UTC | #3
On Tue, Oct 09, 2018 at 01:53:46PM +0200, Thomas Monjalon wrote:
> 09/10/2018 13:43, Neil Horman:
> > On Tue, Oct 09, 2018 at 04:18:56AM +0200, Thomas Monjalon wrote:
> > > Next patch does not pass the experimental check
> > > because it has an (unrelated) experimental function in his context.
> > > 
> > > Cc: Neil Horman <nhorman@tuxdriver.com>
> > > 
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > >  mk/internal/rte.compile-pre.mk | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
> > > index a734cbbd0..acb9796fa 100644
> > > --- a/mk/internal/rte.compile-pre.mk
> > > +++ b/mk/internal/rte.compile-pre.mk
> > > @@ -75,7 +75,6 @@ C_TO_O_DO = @set -e; \
> > >  	echo $(C_TO_O_DISP); \
> > >  	$(C_TO_O) && \
> > >  	$(PMDINFO_TO_O) && \
> > > -	$(CHECK_EXPERIMENTAL) && \
> > >  	echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \
> > >  	sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call obj2dep,$(@)) && \
> > >  	rm -f $(call obj2dep,$(@)).tmp
> > 
> > NACK.
> 
> Yes, of course.
> I have added this patch in the series in order to allow building the patches.
> 
> > Its a bit vague to get the meaning of your changelog, but based on the
> > context of the actual other patch I think what you're saying is that patch 3/4 in
> > this series has the __rte_experimental token in it as part of the context of a
> > diff, rather than as a change itself, and the check-experimental-syms script is
> > matching on that when it shouldn't.  The correct fix is to teach the difference
> > between changes and context to the check-experimental-syms.sh script, not to
> > break the whole thing by just removing the call to the script.
> 
> I agree, but I don't understand how the patch context is related
> to the checks done in buildtools.
> Please could you fix it? It is blocking the integration of this series.
> 
Yeah, I'll get to it as soon as possible, but please lead with this rather than
just dismantling the feature :)

Neil

> Thanks
> 
> 
>
  

Patch

diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index a734cbbd0..acb9796fa 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -75,7 +75,6 @@  C_TO_O_DO = @set -e; \
 	echo $(C_TO_O_DISP); \
 	$(C_TO_O) && \
 	$(PMDINFO_TO_O) && \
-	$(CHECK_EXPERIMENTAL) && \
 	echo $(C_TO_O_CMD) > $(call obj2cmd,$(@)) && \
 	sed 's,'$@':,dep_'$@' =,' $(call obj2dep,$(@)).tmp > $(call obj2dep,$(@)) && \
 	rm -f $(call obj2dep,$(@)).tmp