[dpdk-dev] [PATCH 1/2] devtools: report the incorrect section when complaining

[dpdk-dev] [PATCH 1/2] devtools: report the incorrect section when complaining

[David Marchand]

It does not hurt reporting the incriminated section.

Before:
ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
section other than the EXPERIMENTAL section of the version map

After:
ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
+EXPERIMENTAL section other than the EXPERIMENTAL section of the
version map

Signed-off-by: David Marchand <david.marchand@redhat.com>
---

Used http://patchwork.dpdk.org/patch/48354/ to test.

---
 devtools/check-symbol-change.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 1d21e91..66741be 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -115,7 +115,7 @@ check_for_rule_violations()
 				if [ $? -ne 0 ]
 				then
 					echo -n "ERROR: symbol $symname "
-					echo -n "is added in a section "
+					echo -n "is added in $secname section "
 					echo -n "other than the EXPERIMENTAL "
 					echo "section of the version map"
 					ret=1
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/2] devtools: drop '+' from the section name

[David Marchand]

The incriminated commit did relax the condition to catch all sections
but dropped the + removal which can trigger false detection of the
special EXPERIMENTAL section when adding symbols and the section in the
same patch.

Fixes: 7281cf520f89 ("devtools: relax rule for identifying symbol section")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---

Used http://patchwork.dpdk.org/patch/48354/ to test.

---
 devtools/check-symbol-change.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 66741be..8b36ccd 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -31,6 +31,7 @@ build_map_changes()
 		# Triggering this rule sets in_sec to 1, which actives the
 		# symbol rule below
 		/^.*{/ {
+			gsub("+", "");
 			if (in_map == 1) {
 				sec=$(NF-1); in_sec=1;
 			}
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 1/2] devtools: report the incorrect section when complaining

[Neil Horman]

On Wed, Nov 28, 2018 at 11:28:52AM +0100, David Marchand wrote:
> It does not hurt reporting the incriminated section.
> 
> Before:
> ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
> section other than the EXPERIMENTAL section of the version map
> 
> After:
> ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
> +EXPERIMENTAL section other than the EXPERIMENTAL section of the
> version map
> 
nit: Its a bit odd in the changelog to have an example in which the incorect
section being reported matches the expected section.  I.e. its confusing to read
"... is added in +EXPERIMENTAL section other than the EXPERIMENTAL section".
Might be better to change the language of the report below and the example to be
something like:

ERROR: symbol <SYMBOL> is added in the <VERSION> section, but is expected to be
added in the EXPERIMENTAL section

ACK to the notion of reporting the offending section though.  Thats a good idea.

Neil

> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> 
> Used http://patchwork.dpdk.org/patch/48354/ to test.
> 
> ---
>  devtools/check-symbol-change.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
> index 1d21e91..66741be 100755
> --- a/devtools/check-symbol-change.sh
> +++ b/devtools/check-symbol-change.sh
> @@ -115,7 +115,7 @@ check_for_rule_violations()
>  				if [ $? -ne 0 ]
>  				then
>  					echo -n "ERROR: symbol $symname "
> -					echo -n "is added in a section "
> +					echo -n "is added in $secname section "
>  					echo -n "other than the EXPERIMENTAL "
>  					echo "section of the version map"
>  					ret=1
> -- 
> 1.8.3.1
> 
> 

Re: [dpdk-dev] [PATCH 1/2] devtools: report the incorrect section when complaining

[David Marchand]

On Wed, Nov 28, 2018 at 1:35 PM Neil Horman <nhorman@tuxdriver.com> wrote:

> On Wed, Nov 28, 2018 at 11:28:52AM +0100, David Marchand wrote:
> > It does not hurt reporting the incriminated section.
> >
> > Before:
> > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
> > section other than the EXPERIMENTAL section of the version map
> >
> > After:
> > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
> > +EXPERIMENTAL section other than the EXPERIMENTAL section of the
> > version map
> >
> nit: Its a bit odd in the changelog to have an example in which the
> incorect
> section being reported matches the expected section.  I.e. its confusing
> to read
> "... is added in +EXPERIMENTAL section other than the EXPERIMENTAL
> section".
> Might be better to change the language of the report below and the example
> to be
> something like:
>
> ERROR: symbol <SYMBOL> is added in the <VERSION> section, but is expected
> to be
> added in the EXPERIMENTAL section
>
> ACK to the notion of reporting the offending section though.  Thats a good
> idea.
>

Ok, updated for v2.

-- 
David Marchand

Re: [dpdk-dev] [PATCH 1/2] devtools: report the incorrect section when complaining

[Neil Horman]

On Wed, Nov 28, 2018 at 02:07:25PM +0100, David Marchand wrote:
> On Wed, Nov 28, 2018 at 1:35 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Wed, Nov 28, 2018 at 11:28:52AM +0100, David Marchand wrote:
> > > It does not hurt reporting the incriminated section.
> > >
> > > Before:
> > > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
> > > section other than the EXPERIMENTAL section of the version map
> > >
> > > After:
> > > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
> > > +EXPERIMENTAL section other than the EXPERIMENTAL section of the
> > > version map
> > >
> > nit: Its a bit odd in the changelog to have an example in which the
> > incorect
> > section being reported matches the expected section.  I.e. its confusing
> > to read
> > "... is added in +EXPERIMENTAL section other than the EXPERIMENTAL
> > section".
> > Might be better to change the language of the report below and the example
> > to be
> > something like:
> >
> > ERROR: symbol <SYMBOL> is added in the <VERSION> section, but is expected
> > to be
> > added in the EXPERIMENTAL section
> >
> > ACK to the notion of reporting the offending section though.  Thats a good
> > idea.
> >
> 
> Ok, updated for v2.
> 
Thanks!
Neil

> -- 
> David Marchand

[dpdk-dev] [PATCH v2 1/2] devtools: report the incorrect section when complaining

[David Marchand]

It does not hurt reporting the incriminated section.

Before:
ERROR: symbol rte_plop is added in a section other than the EXPERIMENTAL
section of the version map

After:
ERROR: symbol rte_plop is added in the DPDK_19.02 section, but is
expected to be added in the EXPERIMENTAL section of the version map

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
---
 devtools/check-symbol-change.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 1d21e91..4b8d9f3 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -115,8 +115,9 @@ check_for_rule_violations()
 				if [ $? -ne 0 ]
 				then
 					echo -n "ERROR: symbol $symname "
-					echo -n "is added in a section "
-					echo -n "other than the EXPERIMENTAL "
+					echo -n "is added in the $secname "
+					echo -n "section, but is expected to "
+					echo -n "be added in the EXPERIMENTAL "
 					echo "section of the version map"
 					ret=1
 				fi
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/2] devtools: drop '+' from the section name

[David Marchand]

The incriminated commit did relax the condition to catch all sections
but dropped the + removal which can triggers false detection of the
special EXPERIMENTAL section when adding symbols and the section in the
same patch.

Fixes: 7281cf520f89 ("devtools: relax rule for identifying symbol section")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 devtools/check-symbol-change.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 4b8d9f3..020da7e 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -31,6 +31,7 @@ build_map_changes()
 		# Triggering this rule sets in_sec to 1, which actives the
 		# symbol rule below
 		/^.*{/ {
+			gsub("+", "");
 			if (in_map == 1) {
 				sec=$(NF-1); in_sec=1;
 			}
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 2/2] devtools: drop '+' from the section name

[Neil Horman]

On Fri, Nov 30, 2018 at 01:32:02PM +0100, David Marchand wrote:
> The incriminated commit did relax the condition to catch all sections
> but dropped the + removal which can triggers false detection of the
> special EXPERIMENTAL section when adding symbols and the section in the
> same patch.
> 
> Fixes: 7281cf520f89 ("devtools: relax rule for identifying symbol section")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  devtools/check-symbol-change.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
> index 4b8d9f3..020da7e 100755
> --- a/devtools/check-symbol-change.sh
> +++ b/devtools/check-symbol-change.sh
> @@ -31,6 +31,7 @@ build_map_changes()
>  		# Triggering this rule sets in_sec to 1, which actives the
>  		# symbol rule below
>  		/^.*{/ {
> +			gsub("+", "");
>  			if (in_map == 1) {
>  				sec=$(NF-1); in_sec=1;
>  			}
> -- 
> 1.8.3.1
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Thanks!
Neil

Re: [dpdk-dev] [PATCH v2 2/2] devtools: drop '+' from the section name

[David Marchand]

Thomas,

On Fri, Nov 30, 2018 at 4:28 PM Neil Horman <nhorman@tuxdriver.com> wrote:

> On Fri, Nov 30, 2018 at 01:32:02PM +0100, David Marchand wrote:
> > The incriminated commit did relax the condition to catch all sections
> > but dropped the + removal which can triggers false detection of the
> > special EXPERIMENTAL section when adding symbols and the section in the
> > same patch.
> >
> > Fixes: 7281cf520f89 ("devtools: relax rule for identifying symbol
> section")
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
>

Can you take the two little patches from this patchset ?
I am pretty sure people will hit this issue with all these new apis coming
:-).

Thanks.

-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 2/2] devtools: drop '+' from the section name

[Ferruh Yigit]

On 11/30/2018 3:28 PM, Neil Horman wrote:
> On Fri, Nov 30, 2018 at 01:32:02PM +0100, David Marchand wrote:
>> The incriminated commit did relax the condition to catch all sections
>> but dropped the + removal which can triggers false detection of the
>> special EXPERIMENTAL section when adding symbols and the section in the
>> same patch.
>>
>> Fixes: 7281cf520f89 ("devtools: relax rule for identifying symbol section")
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Series applied, thanks.