diff mbox series

devtools: fix symbols check

Message ID 20220308211859.3677476-1-thomas@monjalon.net (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series devtools: fix symbols check |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure

Commit Message

Thomas Monjalon March 8, 2022, 9:18 p.m. UTC 
  In some environments, the check of local symbols catch-all
was failing. Note: this script is called during the build.

The reason is that grep returns an error if nothing is found.
The option -e of the shell script makes this error fatal.
It is not always fatal because the grep is in a command substitution.

Fixes: b403498e1422 ("build: hide local symbols in shared libraries")

Reported-by: Usama Arif <usama.arif@bytedance.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/check-symbol-maps.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Monjalon March 8, 2022, 9:21 p.m. UTC | #1 
08/03/2022 22:18, Thomas Monjalon:
> In some environments, the check of local symbols catch-all
> was failing. Note: this script is called during the build.
> 
> The reason is that grep returns an error if nothing is found.
> The option -e of the shell script makes this error fatal.
> It is not always fatal because the grep is in a command substitution.
> 
> Fixes: b403498e1422 ("build: hide local symbols in shared libraries")
> 
> Reported-by: Usama Arif <usama.arif@bytedance.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Applied quickly to fix the build in some environments.
Liang Ma March 8, 2022, 9:27 p.m. UTC | #2 
On Tue, Mar 08, 2022 at 10:18:59PM +0100, Thomas Monjalon wrote:
> In some environments, the check of local symbols catch-all
> was failing. Note: this script is called during the build.
> 
> The reason is that grep returns an error if nothing is found.
> The option -e of the shell script makes this error fatal.
> It is not always fatal because the grep is in a command substitution.
> 
> Fixes: b403498e1422 ("build: hide local symbols in shared libraries")
> 
> Reported-by: Usama Arif <usama.arif@bytedance.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  devtools/check-symbol-maps.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
> index 8266fdf9ea..32e1fa5c8f 100755
> --- a/devtools/check-symbol-maps.sh
> +++ b/devtools/check-symbol-maps.sh
> @@ -53,7 +53,7 @@ if [ -n "$duplicate_symbols" ] ; then
>      ret=1
>  fi
>  
> -local_miss_maps=$(grep -L 'local: \*;' $@)
> +local_miss_maps=$(grep -L 'local: \*;' $@ || true)
reviewed-by Liang Ma <liangma@liangbit.com> 

I'm curious to know how can the CI passed with original patch..... 

>  if [ -n "$local_miss_maps" ] ; then
>      echo "Found maps without local catch-all:"
>      echo "$local_miss_maps"
> -- 
> 2.34.1
>
Thomas Monjalon March 8, 2022, 10:05 p.m. UTC | #3 
08/03/2022 22:27, Liang Ma:
> On Tue, Mar 08, 2022 at 10:18:59PM +0100, Thomas Monjalon wrote:
> > In some environments, the check of local symbols catch-all
> > was failing. Note: this script is called during the build.
> > 
> > The reason is that grep returns an error if nothing is found.
> > The option -e of the shell script makes this error fatal.
> > It is not always fatal because the grep is in a command substitution.
> > 
> > Fixes: b403498e1422 ("build: hide local symbols in shared libraries")
> > 
> > Reported-by: Usama Arif <usama.arif@bytedance.com>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  devtools/check-symbol-maps.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
> > index 8266fdf9ea..32e1fa5c8f 100755
> > --- a/devtools/check-symbol-maps.sh
> > +++ b/devtools/check-symbol-maps.sh
> > @@ -53,7 +53,7 @@ if [ -n "$duplicate_symbols" ] ; then
> >      ret=1
> >  fi
> >  
> > -local_miss_maps=$(grep -L 'local: \*;' $@)
> > +local_miss_maps=$(grep -L 'local: \*;' $@ || true)
> reviewed-by Liang Ma <liangma@liangbit.com> 
> 
> I'm curious to know how can the CI passed with original patch..... 

You are suggesting I did not check the CI.

The Intel check failed because of an apply issue,
so I was not able to see the compilation result:
http://mails.dpdk.org/archives/test-report/2022-March/267685.html

Later in the day, the UNH CI failed for the good reason
but it was too late, the patch was applied:
http://mails.dpdk.org/archives/test-report/2022-March/267723.html

That's what happens when I want to introduce a little additional check
at the last minute before a release candidate.
Liang Ma March 8, 2022, 11:02 p.m. UTC | #4 
On Tue, Mar 08, 2022 at 11:05:49PM +0100, Thomas Monjalon wrote:
> 08/03/2022 22:27, Liang Ma:
> > On Tue, Mar 08, 2022 at 10:18:59PM +0100, Thomas Monjalon wrote:
> > > In some environments, the check of local symbols catch-all
> > > was failing. Note: this script is called during the build.
> > > 
> > > The reason is that grep returns an error if nothing is found.
> > > The option -e of the shell script makes this error fatal.
> > > It is not always fatal because the grep is in a command substitution.
> > > 
> > > Fixes: b403498e1422 ("build: hide local symbols in shared libraries")
> > > 
> > > Reported-by: Usama Arif <usama.arif@bytedance.com>
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > >  devtools/check-symbol-maps.sh | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
> > > index 8266fdf9ea..32e1fa5c8f 100755
> > > --- a/devtools/check-symbol-maps.sh
> > > +++ b/devtools/check-symbol-maps.sh
> > > @@ -53,7 +53,7 @@ if [ -n "$duplicate_symbols" ] ; then
> > >      ret=1
> > >  fi
> > >  
> > > -local_miss_maps=$(grep -L 'local: \*;' $@)
> > > +local_miss_maps=$(grep -L 'local: \*;' $@ || true)
> > reviewed-by Liang Ma <liangma@liangbit.com> 
> > 
> > I'm curious to know how can the CI passed with original patch..... 
> 
> You are suggesting I did not check the CI.
> 
> The Intel check failed because of an apply issue,
> so I was not able to see the compilation result:
> http://mails.dpdk.org/archives/test-report/2022-March/267685.html
> 
> Later in the day, the UNH CI failed for the good reason
> but it was too late, the patch was applied:
> http://mails.dpdk.org/archives/test-report/2022-March/267723.html
> 
> That's what happens when I want to introduce a little additional check
> at the last minute before a release candidate.
No worry, I thought the CI has a bug.
> 
>
diff mbox series

Patch

diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
index 8266fdf9ea..32e1fa5c8f 100755
--- a/devtools/check-symbol-maps.sh
+++ b/devtools/check-symbol-maps.sh
@@ -53,7 +53,7 @@  if [ -n "$duplicate_symbols" ] ; then
     ret=1
 fi
 
-local_miss_maps=$(grep -L 'local: \*;' $@)
+local_miss_maps=$(grep -L 'local: \*;' $@ || true)
 if [ -n "$local_miss_maps" ] ; then
     echo "Found maps without local catch-all:"
     echo "$local_miss_maps"