diff mbox

[ovs-dev] configure: Fix check for rte_config.h to handle cross-compilation.

Message ID 20170415041455.4680-1-blp@ovn.org
State Superseded
Headers show

Commit Message

Ben Pfaff April 15, 2017, 4:14 a.m. UTC 
The check for rte_config.h in acinclude.m4 used AC_CHECK_FILE, but this
macro is intended to check for a file on the host system, not the build
system, which means that it fails unconditionally in a cross-compilation
environment.  However, the intended check here is for a header file,
which is part of the build system.  To check for part of the build system,
we can just use "test", so this commit makes that change.

Reported-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329994.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 acinclude.m4 | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Ben Pfaff May 1, 2017, 5:30 p.m. UTC | #1 
Hemant, does this fix the problem you reported?

On Fri, Apr 14, 2017 at 09:14:55PM -0700, Ben Pfaff wrote:
> The check for rte_config.h in acinclude.m4 used AC_CHECK_FILE, but this
> macro is intended to check for a file on the host system, not the build
> system, which means that it fails unconditionally in a cross-compilation
> environment.  However, the intended check here is for a header file,
> which is part of the build system.  To check for part of the build system,
> we can just use "test", so this commit makes that change.
> 
> Reported-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329994.html
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>  acinclude.m4 | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 744d8f89525c..842469455914 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -180,9 +180,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>          DPDK_INCLUDE="$with_dpdk/include"
>          # If 'with_dpdk' is passed install directory, point to headers
>          # installed in $DESTDIR/$prefix/include/dpdk
> -        AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [],
> -                      [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
> -                                     [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
> +	if test ! -e "$DPDK_INCLUDE/rte_config.h" && \
> +	   test -e "$DPDK_INCLUDE/dpdk/rte_config.h"; then
> +	   DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h
> +	fi
>          DPDK_LIB_DIR="$with_dpdk/lib"
>          ;;
>      esac
> -- 
> 2.10.2
>
Ben Pfaff July 6, 2017, 9:36 p.m. UTC | #2 
Can someone review this please?

On Mon, May 01, 2017 at 10:30:27AM -0700, Ben Pfaff wrote:
> Hemant, does this fix the problem you reported?
> 
> On Fri, Apr 14, 2017 at 09:14:55PM -0700, Ben Pfaff wrote:
> > The check for rte_config.h in acinclude.m4 used AC_CHECK_FILE, but this
> > macro is intended to check for a file on the host system, not the build
> > system, which means that it fails unconditionally in a cross-compilation
> > environment.  However, the intended check here is for a header file,
> > which is part of the build system.  To check for part of the build system,
> > we can just use "test", so this commit makes that change.
> > 
> > Reported-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329994.html
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > ---
> >  acinclude.m4 | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 744d8f89525c..842469455914 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -180,9 +180,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> >          DPDK_INCLUDE="$with_dpdk/include"
> >          # If 'with_dpdk' is passed install directory, point to headers
> >          # installed in $DESTDIR/$prefix/include/dpdk
> > -        AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [],
> > -                      [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
> > -                                     [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
> > +	if test ! -e "$DPDK_INCLUDE/rte_config.h" && \
> > +	   test -e "$DPDK_INCLUDE/dpdk/rte_config.h"; then
> > +	   DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h
> > +	fi
> >          DPDK_LIB_DIR="$with_dpdk/lib"
> >          ;;
> >      esac
> > -- 
> > 2.10.2
> >
Darrell Ball July 6, 2017, 9:48 p.m. UTC | #3 
On 7/6/17, 2:36 PM, "ovs-dev-bounces@openvswitch.org on behalf of Ben Pfaff" <ovs-dev-bounces@openvswitch.org on behalf of blp@ovn.org> wrote:

    Can someone review this please?
    
    On Mon, May 01, 2017 at 10:30:27AM -0700, Ben Pfaff wrote:
    > Hemant, does this fix the problem you reported?
    > 
    > On Fri, Apr 14, 2017 at 09:14:55PM -0700, Ben Pfaff wrote:
    > > The check for rte_config.h in acinclude.m4 used AC_CHECK_FILE, but this
    > > macro is intended to check for a file on the host system, not the build
    > > system, which means that it fails unconditionally in a cross-compilation
    > > environment.  However, the intended check here is for a header file,
    > > which is part of the build system.  To check for part of the build system,
    > > we can just use "test", so this commit makes that change.
    > > 
    > > Reported-by: Hemant Agrawal <hemant.agrawal@nxp.com>
    > > Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DMarch_329994.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=9SEip3J9loq15hpoutcyARnoiI9CS9RRVfT0oFE5C8k&s=dOQYeMGyHRUBxuPzkJLkr2oUHFTObc-qjwZhHCDFTrU&e= 
    > > Signed-off-by: Ben Pfaff <blp@ovn.org>
    > > ---
    > >  acinclude.m4 | 7 ++++---
    > >  1 file changed, 4 insertions(+), 3 deletions(-)
    > > 
    > > diff --git a/acinclude.m4 b/acinclude.m4
    > > index 744d8f89525c..842469455914 100644
    > > --- a/acinclude.m4
    > > +++ b/acinclude.m4
    > > @@ -180,9 +180,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
    > >          DPDK_INCLUDE="$with_dpdk/include"
    > >          # If 'with_dpdk' is passed install directory, point to headers
    > >          # installed in $DESTDIR/$prefix/include/dpdk
    > > -        AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [],
    > > -                      [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
    > > -                                     [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
    > > +	if test ! -e "$DPDK_INCLUDE/rte_config.h" && \
    > > +	   test -e "$DPDK_INCLUDE/dpdk/rte_config.h"; then
    > > +	   DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h

Did you mean 
DPDK_INCLUDE=$DPDK_INCLUDE/dpdk
rather than
DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h 
?


    > > +	fi
    > >          DPDK_LIB_DIR="$with_dpdk/lib"
    > >          ;;
    > >      esac
    > > -- 
    > > 2.10.2
    > > 
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=9SEip3J9loq15hpoutcyARnoiI9CS9RRVfT0oFE5C8k&s=5mgaD8RH43sN-x5q37TpOgF-sG-M7rA0u3U4L9eOrP0&e=
Aaron Conole July 7, 2017, 1:11 p.m. UTC | #4 
Ben Pfaff <blp@ovn.org> writes:

> Can someone review this please?
>
> On Mon, May 01, 2017 at 10:30:27AM -0700, Ben Pfaff wrote:
>> Hemant, does this fix the problem you reported?
>> 
>> On Fri, Apr 14, 2017 at 09:14:55PM -0700, Ben Pfaff wrote:
>> > The check for rte_config.h in acinclude.m4 used AC_CHECK_FILE, but this
>> > macro is intended to check for a file on the host system, not the build
>> > system, which means that it fails unconditionally in a cross-compilation
>> > environment.  However, the intended check here is for a header file,
>> > which is part of the build system.  To check for part of the build system,
>> > we can just use "test", so this commit makes that change.
>> > 
>> > Reported-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329994.html
>> > Signed-off-by: Ben Pfaff <blp@ovn.org>
>> > ---
>> >  acinclude.m4 | 7 ++++---
>> >  1 file changed, 4 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/acinclude.m4 b/acinclude.m4
>> > index 744d8f89525c..842469455914 100644
>> > --- a/acinclude.m4
>> > +++ b/acinclude.m4
>> > @@ -180,9 +180,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>> >          DPDK_INCLUDE="$with_dpdk/include"
>> >          # If 'with_dpdk' is passed install directory, point to headers
>> >          # installed in $DESTDIR/$prefix/include/dpdk
>> > -        AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [],
>> > -                      [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
>> > -                                     [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
>> > +	if test ! -e "$DPDK_INCLUDE/rte_config.h" && \
>> > +	   test -e "$DPDK_INCLUDE/dpdk/rte_config.h"; then
>> > +	   DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h

As Darrell pointed out, the DPDK_INCLUDE directive needs to trim the header
file.

>> > +	fi

Also, this leads with tabs, and the rest of the file leads with spaces.

I found an old discussion about the AC_CHECK_FILE limitation at
https://lists.gnu.org/archive/html/autoconf/2000-10/msg00018.html
and the recommendation ends up being "use test, AC_CHECK_FILE is
overkill," which makes me think AC_CHECK_FILE is meant for checking
runtime files - not build time files.

>> >          DPDK_LIB_DIR="$with_dpdk/lib"
>> >          ;;
>> >      esac
>> > -- 
>> > 2.10.2
>> > 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff July 7, 2017, 4:17 p.m. UTC | #5 
On Thu, Jul 06, 2017 at 09:48:58PM +0000, Darrell Ball wrote:
> 
> 
> On 7/6/17, 2:36 PM, "ovs-dev-bounces@openvswitch.org on behalf of Ben Pfaff" <ovs-dev-bounces@openvswitch.org on behalf of blp@ovn.org> wrote:
> 
>     Can someone review this please?
>     
>     On Mon, May 01, 2017 at 10:30:27AM -0700, Ben Pfaff wrote:
>     > Hemant, does this fix the problem you reported?
>     > 
>     > On Fri, Apr 14, 2017 at 09:14:55PM -0700, Ben Pfaff wrote:
>     > > The check for rte_config.h in acinclude.m4 used AC_CHECK_FILE, but this
>     > > macro is intended to check for a file on the host system, not the build
>     > > system, which means that it fails unconditionally in a cross-compilation
>     > > environment.  However, the intended check here is for a header file,
>     > > which is part of the build system.  To check for part of the build system,
>     > > we can just use "test", so this commit makes that change.
>     > > 
>     > > Reported-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>     > > Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DMarch_329994.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=9SEip3J9loq15hpoutcyARnoiI9CS9RRVfT0oFE5C8k&s=dOQYeMGyHRUBxuPzkJLkr2oUHFTObc-qjwZhHCDFTrU&e= 
>     > > Signed-off-by: Ben Pfaff <blp@ovn.org>
>     > > ---
>     > >  acinclude.m4 | 7 ++++---
>     > >  1 file changed, 4 insertions(+), 3 deletions(-)
>     > > 
>     > > diff --git a/acinclude.m4 b/acinclude.m4
>     > > index 744d8f89525c..842469455914 100644
>     > > --- a/acinclude.m4
>     > > +++ b/acinclude.m4
>     > > @@ -180,9 +180,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>     > >          DPDK_INCLUDE="$with_dpdk/include"
>     > >          # If 'with_dpdk' is passed install directory, point to headers
>     > >          # installed in $DESTDIR/$prefix/include/dpdk
>     > > -        AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [],
>     > > -                      [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
>     > > -                                     [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
>     > > +	if test ! -e "$DPDK_INCLUDE/rte_config.h" && \
>     > > +	   test -e "$DPDK_INCLUDE/dpdk/rte_config.h"; then
>     > > +	   DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h
> 
> Did you mean 
> DPDK_INCLUDE=$DPDK_INCLUDE/dpdk
> rather than
> DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h 
> ?

Oops, you're right.  Fixed in v2:
        https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335079.html
Ben Pfaff July 7, 2017, 4:17 p.m. UTC | #6 
On Fri, Jul 07, 2017 at 09:11:27AM -0400, Aaron Conole wrote:
> Ben Pfaff <blp@ovn.org> writes:
> 
> > Can someone review this please?
> >
> > On Mon, May 01, 2017 at 10:30:27AM -0700, Ben Pfaff wrote:
> >> Hemant, does this fix the problem you reported?
> >> 
> >> On Fri, Apr 14, 2017 at 09:14:55PM -0700, Ben Pfaff wrote:
> >> > The check for rte_config.h in acinclude.m4 used AC_CHECK_FILE, but this
> >> > macro is intended to check for a file on the host system, not the build
> >> > system, which means that it fails unconditionally in a cross-compilation
> >> > environment.  However, the intended check here is for a header file,
> >> > which is part of the build system.  To check for part of the build system,
> >> > we can just use "test", so this commit makes that change.
> >> > 
> >> > Reported-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329994.html
> >> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> >> > ---
> >> >  acinclude.m4 | 7 ++++---
> >> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >> > 
> >> > diff --git a/acinclude.m4 b/acinclude.m4
> >> > index 744d8f89525c..842469455914 100644
> >> > --- a/acinclude.m4
> >> > +++ b/acinclude.m4
> >> > @@ -180,9 +180,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> >> >          DPDK_INCLUDE="$with_dpdk/include"
> >> >          # If 'with_dpdk' is passed install directory, point to headers
> >> >          # installed in $DESTDIR/$prefix/include/dpdk
> >> > -        AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [],
> >> > -                      [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
> >> > -                                     [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
> >> > +	if test ! -e "$DPDK_INCLUDE/rte_config.h" && \
> >> > +	   test -e "$DPDK_INCLUDE/dpdk/rte_config.h"; then
> >> > +	   DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h
> 
> As Darrell pointed out, the DPDK_INCLUDE directive needs to trim the header
> file.

Thanks, fixed in v2.

> >> > +	fi
> 
> Also, this leads with tabs, and the rest of the file leads with spaces.

Fixed in v2:
        https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335079.html

> I found an old discussion about the AC_CHECK_FILE limitation at
> https://lists.gnu.org/archive/html/autoconf/2000-10/msg00018.html
> and the recommendation ends up being "use test, AC_CHECK_FILE is
> overkill," which makes me think AC_CHECK_FILE is meant for checking
> runtime files - not build time files.

Yes, this is a reasonable restatement of the rationale for this change.
diff mbox

Patch

diff --git a/acinclude.m4 b/acinclude.m4
index 744d8f89525c..842469455914 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -180,9 +180,10 @@  AC_DEFUN([OVS_CHECK_DPDK], [
         DPDK_INCLUDE="$with_dpdk/include"
         # If 'with_dpdk' is passed install directory, point to headers
         # installed in $DESTDIR/$prefix/include/dpdk
-        AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h], [],
-                      [AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],
-                                     [DPDK_INCLUDE=$DPDK_INCLUDE/dpdk], [])])
+	if test ! -e "$DPDK_INCLUDE/rte_config.h" && \
+	   test -e "$DPDK_INCLUDE/dpdk/rte_config.h"; then
+	   DPDK_INCLUDE=$DPDK_INCLUDE/dpdk/rte_config.h
+	fi
         DPDK_LIB_DIR="$with_dpdk/lib"
         ;;
     esac