[dpdk-stable] [PATCH 2/4] devtools: handle section suppression

[dpdk-stable] [PATCH 2/4] devtools: handle section suppression

[David Marchand]

Even if rare, the check script should handle removing a section.

Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
Cc: stable@dpdk.org

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 8da7650..d5fad04 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -32,6 +32,7 @@ build_map_changes()
 		# symbol rule below
 		/^.*{/ {
 			gsub("+", "");
+			gsub("-", "");
 			if (in_map == 1) {
 				sec=$(NF-1); in_sec=1;
 			}
-- 
1.8.3.1

Re: [dpdk-stable] [PATCH 2/4] devtools: handle section suppression

[Neil Horman]

On Fri, May 03, 2019 at 04:34:18PM +0200, David Marchand wrote:
> Even if rare, the check script should handle removing a section.
> 
> Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
> Cc: stable@dpdk.org
> 
> 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 8da7650..d5fad04 100755
> --- a/devtools/check-symbol-change.sh
> +++ b/devtools/check-symbol-change.sh
> @@ -32,6 +32,7 @@ build_map_changes()
>  		# symbol rule below
>  		/^.*{/ {
>  			gsub("+", "");
> +			gsub("-", "");
>  			if (in_map == 1) {
>  				sec=$(NF-1); in_sec=1;
>  			}
> -- 
> 1.8.3.1
> 
> 
Don't you also need to add some logic in the symbol detection match rule to
print an appropriate indicator that a symbol is being removed?  With just this
change, you will note that you are parsing a section, but you will never trigger
a symbol match

Neil

Re: [dpdk-stable] [PATCH 2/4] devtools: handle section suppression

[David Marchand]

On Fri, May 3, 2019 at 5:18 PM Neil Horman <nhorman@tuxdriver.com> wrote:

> On Fri, May 03, 2019 at 04:34:18PM +0200, David Marchand wrote:
> > Even if rare, the check script should handle removing a section.
> >
> > Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
> > Cc: stable@dpdk.org
> >
> > 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 8da7650..d5fad04 100755
> > --- a/devtools/check-symbol-change.sh
> > +++ b/devtools/check-symbol-change.sh
> > @@ -32,6 +32,7 @@ build_map_changes()
> >               # symbol rule below
> >               /^.*{/ {
> >                       gsub("+", "");
> > +                     gsub("-", "");
> >                       if (in_map == 1) {
> >                               sec=$(NF-1); in_sec=1;
> >                       }
> > --
> > 1.8.3.1
> >
> >
> Don't you also need to add some logic in the symbol detection match rule to
> print an appropriate indicator that a symbol is being removed?  With just
> this
> change, you will note that you are parsing a section, but you will never
> trigger
> a symbol match
>

I do remember seeing a warning about the "-EXPERIMENTAL" section.
And this is why I added this.
But since then I reorganised my series and eliminated some parts... so you
are most likely right.
I will double check and repost.


-- 
David Marchand

Re: [dpdk-stable] [PATCH 2/4] devtools: handle section suppression

[David Marchand]

On Fri, May 3, 2019 at 7:16 PM David Marchand <david.marchand@redhat.com>
wrote:

> On Fri, May 3, 2019 at 5:18 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
>> On Fri, May 03, 2019 at 04:34:18PM +0200, David Marchand wrote:
>> > Even if rare, the check script should handle removing a section.
>> >
>> > Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
>> > Cc: stable@dpdk.org
>> >
>> > 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 8da7650..d5fad04 100755
>> > --- a/devtools/check-symbol-change.sh
>> > +++ b/devtools/check-symbol-change.sh
>> > @@ -32,6 +32,7 @@ build_map_changes()
>> >               # symbol rule below
>> >               /^.*{/ {
>> >                       gsub("+", "");
>> > +                     gsub("-", "");
>> >                       if (in_map == 1) {
>> >                               sec=$(NF-1); in_sec=1;
>> >                       }
>> > --
>> > 1.8.3.1
>> >
>> >
>> Don't you also need to add some logic in the symbol detection match rule
>> to
>> print an appropriate indicator that a symbol is being removed?  With just
>> this
>> change, you will note that you are parsing a section, but you will never
>> trigger
>> a symbol match
>>
>
> I do remember seeing a warning about the "-EXPERIMENTAL" section.
> And this is why I added this.
> But since then I reorganised my series and eliminated some parts... so you
> are most likely right.
> I will double check and repost.
>
>
Yes, you are right, I moved this out of another patch that I ended up not
sending.
But it makes no sense by itself.
I will drop this in v2.

Any comments on the other patches ?
Thanks Neil.


-- 
David Marchand

Re: [dpdk-stable] [PATCH 2/4] devtools: handle section suppression

[Neil Horman]

On Mon, May 06, 2019 at 02:56:51PM +0200, David Marchand wrote:
> On Fri, May 3, 2019 at 7:16 PM David Marchand <david.marchand@redhat.com>
> wrote:
> 
> > On Fri, May 3, 2019 at 5:18 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> >> On Fri, May 03, 2019 at 04:34:18PM +0200, David Marchand wrote:
> >> > Even if rare, the check script should handle removing a section.
> >> >
> >> > Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
> >> > Cc: stable@dpdk.org
> >> >
> >> > 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 8da7650..d5fad04 100755
> >> > --- a/devtools/check-symbol-change.sh
> >> > +++ b/devtools/check-symbol-change.sh
> >> > @@ -32,6 +32,7 @@ build_map_changes()
> >> >               # symbol rule below
> >> >               /^.*{/ {
> >> >                       gsub("+", "");
> >> > +                     gsub("-", "");
> >> >                       if (in_map == 1) {
> >> >                               sec=$(NF-1); in_sec=1;
> >> >                       }
> >> > --
> >> > 1.8.3.1
> >> >
> >> >
> >> Don't you also need to add some logic in the symbol detection match rule
> >> to
> >> print an appropriate indicator that a symbol is being removed?  With just
> >> this
> >> change, you will note that you are parsing a section, but you will never
> >> trigger
> >> a symbol match
> >>
> >
> > I do remember seeing a warning about the "-EXPERIMENTAL" section.
> > And this is why I added this.
> > But since then I reorganised my series and eliminated some parts... so you
> > are most likely right.
> > I will double check and repost.
> >
> >
> Yes, you are right, I moved this out of another patch that I ended up not
> sending.
> But it makes no sense by itself.
> I will drop this in v2.
> 
> Any comments on the other patches ?
> Thanks Neil.
> 
No, I don't think I do, thanks for checking though!
Neil

> 
> -- 
> David Marchand

[dpdk-stable] [PATCH 3/4] devtools: fix symbol name in log message

[David Marchand]

We have an incorrect variable name in this log.

Fixes: 4bec48184e33 ("devtools: add checks for ABI symbol addition")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 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 d5fad04..9ada81e 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -98,7 +98,7 @@ check_for_rule_violations()
 			then
 				# Just inform the user of this occurrence, but
 				# don't flag it as an error
-				echo -n "INFO: symbol $syname is added but "
+				echo -n "INFO: symbol $symname is added but "
 				echo -n "patch has insuficient context "
 				echo -n "to determine the section name "
 				echo -n "please ensure the version is "
-- 
1.8.3.1

[dpdk-stable] [PATCH 4/4] devtools: fix direct additions to stable API

[David Marchand]

The incriminated commit broke the detection of new symbols skipping the
EXPERIMENTAL step before entering a stable abi section.
sed won't return an error, check a null output instead.

Fixes: 3630757803ab ("devtools: accept experimental symbol promotion")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 devtools/check-symbol-change.sh | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 9ada81e..116c311 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -111,15 +111,24 @@ check_for_rule_violations()
 
 			# A symbol can not enter a non experimental
 			# section directly
-			if [ $? -ne 0 ] && [ "$secname" != 'EXPERIMENTAL' ]
+			if [ -z "$oldsecname" ]
 			then
-				echo -n "ERROR: symbol $symname "
-				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
-				continue
+				if [ "$secname" = 'EXPERIMENTAL' ]
+				then
+					echo -n "INFO: symbol $symname has "
+					echo -n "been added to the "
+					echo -n "EXPERIMENTAL section of the "
+					echo "version map"
+					continue
+				else
+					echo -n "ERROR: symbol $symname "
+					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
+					continue
+				fi
 			fi
 
 			# This symbol is moving inside a section, nothing to do
-- 
1.8.3.1

Re: [dpdk-stable] [PATCH 4/4] devtools: fix direct additions to stable API

[Thomas Monjalon]

03/05/2019 16:34, David Marchand:
> The incriminated commit broke the detection of new symbols skipping the
> EXPERIMENTAL step before entering a stable abi section.
> sed won't return an error, check a null output instead.
> 
> Fixes: 3630757803ab ("devtools: accept experimental symbol promotion")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Series applied (without patch 2), thanks