diff mbox series

[ovs-dev,v1] acinclude: Provide error info when linking fails with DPDK.

Message ID 20211110100744.204072-1-sunil.pai.g@intel.com
State Superseded
Headers show
Series [ovs-dev,v1] acinclude: Provide error info when linking fails with DPDK. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Pai G, Sunil Nov. 10, 2021, 10:07 a.m. UTC
Currently the configure script provides no infomation but to update
the pkg-config path on failure to link with DPDK.
Fix this to provide more information to the user on failure.

Before:
checking whether linking with dpdk works... no
configure: error: Could not find DPDK library in default search path,
update PKG_CONFIG_PATH for pkg-config to find the .pc file in
non-standard location

After:
checking whether linking with dpdk works... no
configure: error: configure:27158: gcc -o conftest -include rte_config.h ...
-Wl,--whole-archive -l:librte_bus_pci.a -l:my_lib.a ... -lunwind >&5
/usr/bin/ld: cannot find -l:my_lib.a

Signed-off-by: Sunil Pai G <sunil.pai.g@intel.com>
---
 acinclude.m4 | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Michael Santana Nov. 12, 2021, 4:43 p.m. UTC | #1
On 11/10/21 5:07 AM, Sunil Pai G wrote:
> Currently the configure script provides no infomation but to update
> the pkg-config path on failure to link with DPDK.
> Fix this to provide more information to the user on failure.
> 
> Before:
> checking whether linking with dpdk works... no
> configure: error: Could not find DPDK library in default search path,
> update PKG_CONFIG_PATH for pkg-config to find the .pc file in
> non-standard location
> 
> After:
> checking whether linking with dpdk works... no
> configure: error: configure:27158: gcc -o conftest -include rte_config.h ...
> -Wl,--whole-archive -l:librte_bus_pci.a -l:my_lib.a ... -lunwind >&5
> /usr/bin/ld: cannot find -l:my_lib.a
Let's add more details.

checking whether linking with dpdk works... no
+Printing the error that caused the linking failure:
configure: error: configure:27158: gcc -o conftest -include rte_config.h...
-Wl,--whole-archive -l:librte_bus_pci.a -l:my_lib.a ... -lunwind >&5
/usr/bin/ld: cannot find -l:my_lib.a
+End of log. In the case that this script could not find DPDK in the 
+default search path, update PKG_CONFIG_PATH for pkg-config to find the 
+.pc file in non-standard location. Otherwise check the error logs for 
+more details in the log file config.log

What do you think?
> 
> Signed-off-by: Sunil Pai G <sunil.pai.g@intel.com>
> ---
>   acinclude.m4 | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index dba365ea1..1af277447 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -475,7 +475,8 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>       # DPDK uses dlopen to load plugins.
>       OVS_FIND_DEPENDENCY([dlopen], [dl], [libdl])
>   
> -    AC_MSG_CHECKING([whether linking with dpdk works])
> +    DPDK_STRING="whether linking with dpdk works"
> +    AC_MSG_CHECKING([$DPDK_STRING])
>       LIBS="$DPDK_LIB $LIBS"
>       AC_LINK_IFELSE(
>         [AC_LANG_PROGRAM([#include <rte_config.h>
> @@ -485,10 +486,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>         [AC_MSG_RESULT([yes])
>          DPDKLIB_FOUND=true],
>         [AC_MSG_RESULT([no])
> +       # Fetch the cause of failure from config.log
> +       DPDK_LINK_ERROR=$(grep "$DPDK_STRING" -A2 config.log | tail -n2)
>          AC_MSG_ERROR(m4_normalize([
> -          Could not find DPDK library in default search path, update
> -          PKG_CONFIG_PATH for pkg-config to find the .pc file in
> -          non-standard location]))
> +          $DPDK_LINK_ERROR]))
>         ])
>   
>       CFLAGS="$ovs_save_CFLAGS"
>
Pai G, Sunil Nov. 12, 2021, 5:33 p.m. UTC | #2
Hi Michael,

Thanks for the review , responses inline.

> > Before:
> > checking whether linking with dpdk works... no
> > configure: error: Could not find DPDK library in default search path,
> > update PKG_CONFIG_PATH for pkg-config to find the .pc file in
> > non-standard location
> >
> > After:
> > checking whether linking with dpdk works... no
> > configure: error: configure:27158: gcc -o conftest -include rte_config.h ...
> > -Wl,--whole-archive -l:librte_bus_pci.a -l:my_lib.a ... -lunwind >&5
> > /usr/bin/ld: cannot find -l:my_lib.a
> Let's add more details.
> 
> checking whether linking with dpdk works... no
> +Printing the error that caused the linking failure:

Not sure about this one.  Might be a bit too verbose ? I wonder if the error message (in the next line) is probably descriptive enough ? 

> configure: error: configure:27158: gcc -o conftest -include rte_config.h...
> -Wl,--whole-archive -l:librte_bus_pci.a -l:my_lib.a ... -lunwind >&5
> /usr/bin/ld: cannot find -l:my_lib.a


> +End of log. In the case that this script could not find DPDK in the
> +default search path, update PKG_CONFIG_PATH for pkg-config to find the
> +.pc file in non-standard location. Otherwise check the error logs for
> +more details in the log file config.log

Not sure if this could happen tbh. I tried keeping PKG_CONFIG_PATH empty deliberately to check and saw the standard error message from pkg-config:

checking whether dpdk is enabled... yes
checking for DPDK... no
configure: error: Package requirements (libdpdk) were not met:

No package 'libdpdk' found

Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.

Alternatively, you may set the environment variables DPDK_CFLAGS
and DPDK_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details. 


<snipped>

Thanks and regards,
Sunil
Ilya Maximets Dec. 3, 2021, 2:10 p.m. UTC | #3
On 11/10/21 11:07, Sunil Pai G wrote:
> Currently the configure script provides no infomation but to update
> the pkg-config path on failure to link with DPDK.
> Fix this to provide more information to the user on failure.
> 
> Before:
> checking whether linking with dpdk works... no
> configure: error: Could not find DPDK library in default search path,
> update PKG_CONFIG_PATH for pkg-config to find the .pc file in
> non-standard location
> 
> After:
> checking whether linking with dpdk works... no
> configure: error: configure:27158: gcc -o conftest -include rte_config.h ...
> -Wl,--whole-archive -l:librte_bus_pci.a -l:my_lib.a ... -lunwind >&5
> /usr/bin/ld: cannot find -l:my_lib.a

Hi, Sunil.

I don't think that printing parts of a config.log is a good thing
to do.  Primarily because if ./configure failed, users need to
look at config.log anyway.  That's a normal thing for all automake
based build systems.

One thing that we can improve is the error message.  Currently it
assumes that the path is not correct, but it should not exclude
the scenario where path is correct, but the library is broken, i.e.
has missing dependencies.

Maybe addition of a single "working" word to the message will be
enough:

"Could not find working DPDK library in default search path, ..."

Or something similar.

Best regards, Ilya Maximets.
Pai G, Sunil Dec. 6, 2021, 9:38 a.m. UTC | #4
Hi Ilya, 

> Hi, Sunil.
> 
> I don't think that printing parts of a config.log is a good thing to do.  Primarily
> because if ./configure failed, users need to look at config.log anyway.  That's
> a normal thing for all automake based build systems.
> 
> One thing that we can improve is the error message.  Currently it assumes
> that the path is not correct, but it should not exclude the scenario where
> path is correct, but the library is broken, i.e.
> has missing dependencies.
> 
> Maybe addition of a single "working" word to the message will be
> enough:
> 
> "Could not find working DPDK library in default search path, ..."
> 
> Or something similar.

Sure, sounds good. 
How about the following:

Failed to link to with DPDK, check config.log for more details on the error.
Could not find working DPDK library in default search path, update
PKG_CONFIG_PATH for pkg-config to find the .pc file in
non-standard location. 

> 
> Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/acinclude.m4 b/acinclude.m4
index dba365ea1..1af277447 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -475,7 +475,8 @@  AC_DEFUN([OVS_CHECK_DPDK], [
     # DPDK uses dlopen to load plugins.
     OVS_FIND_DEPENDENCY([dlopen], [dl], [libdl])
 
-    AC_MSG_CHECKING([whether linking with dpdk works])
+    DPDK_STRING="whether linking with dpdk works"
+    AC_MSG_CHECKING([$DPDK_STRING])
     LIBS="$DPDK_LIB $LIBS"
     AC_LINK_IFELSE(
       [AC_LANG_PROGRAM([#include <rte_config.h>
@@ -485,10 +486,10 @@  AC_DEFUN([OVS_CHECK_DPDK], [
       [AC_MSG_RESULT([yes])
        DPDKLIB_FOUND=true],
       [AC_MSG_RESULT([no])
+       # Fetch the cause of failure from config.log
+       DPDK_LINK_ERROR=$(grep "$DPDK_STRING" -A2 config.log | tail -n2)
        AC_MSG_ERROR(m4_normalize([
-          Could not find DPDK library in default search path, update
-          PKG_CONFIG_PATH for pkg-config to find the .pc file in
-          non-standard location]))
+          $DPDK_LINK_ERROR]))
       ])
 
     CFLAGS="$ovs_save_CFLAGS"