diff mbox series

[iproute2-next] configure: Allow command line override of toolchain

Message ID 20220228015435.1328-1-dsahern@kernel.org (mailing list archive)
State Accepted
Commit 386ae64c8312dd27b09508993a7c8386aff8b1d3
Delegated to: David Ahern
Headers show
Series [iproute2-next] configure: Allow command line override of toolchain | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

David Ahern Feb. 28, 2022, 1:54 a.m. UTC
Easy way to build for both gcc and clang.

Signed-off-by: David Ahern <dsahern@kernel.org>
---
 configure | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Stephen Hemminger Feb. 28, 2022, 5:11 p.m. UTC | #1
On Sun, 27 Feb 2022 18:54:35 -0700
David Ahern <dsahern@kernel.org> wrote:

> Easy way to build for both gcc and clang.
> 
> Signed-off-by: David Ahern <dsahern@kernel.org>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
patchwork-bot+netdevbpf@kernel.org Feb. 28, 2022, 5:50 p.m. UTC | #2
Hello:

This patch was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Sun, 27 Feb 2022 18:54:35 -0700 you wrote:
> Easy way to build for both gcc and clang.
> 
> Signed-off-by: David Ahern <dsahern@kernel.org>
> ---
>  configure | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Here is the summary with links:
  - [iproute2-next] configure: Allow command line override of toolchain
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=386ae64c8312

You are awesome, thank you!
Ido Schimmel March 2, 2022, 1:56 p.m. UTC | #3
On Sun, Feb 27, 2022 at 06:54:35PM -0700, David Ahern wrote:
> Easy way to build for both gcc and clang.
> 
> Signed-off-by: David Ahern <dsahern@kernel.org>
> ---
>  configure | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index 8ddff43c6a7d..13f2d1739b99 100755
> --- a/configure
> +++ b/configure
> @@ -19,10 +19,10 @@ check_toolchain()
>      : ${AR=ar}
>      : ${CC=gcc}
>      : ${YACC=bison}
> -    echo "PKG_CONFIG:=${PKG_CONFIG}" >>$CONFIG
> -    echo "AR:=${AR}" >>$CONFIG
> -    echo "CC:=${CC}" >>$CONFIG
> -    echo "YACC:=${YACC}" >>$CONFIG
> +    echo "PKG_CONFIG?=${PKG_CONFIG}" >>$CONFIG
> +    echo "AR?=${AR}" >>$CONFIG
> +    echo "CC?=${CC}" >>$CONFIG
> +    echo "YACC?=${YACC}" >>$CONFIG
>  }

David, are you sure this patch is needed? Even without it I can override
from the command line:

$ make V=1 CC=gcc

lib
make[1]: Entering directory '/home/idosch/code/iproute2/lib'
gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c
...

$ make V=1 CC=clang

lib
make[1]: Entering directory '/home/idosch/code/iproute2/lib'
clang -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c

Also created this test file:

$ cat bla.mk
CC := foo

all:
        echo $(origin CC)
        echo $(CC)

$ make -f bla.mk
echo file
file
echo foo
foo

$ make -f bla.mk CC=clang
echo command line
command line
echo clang
clang

$ make -v
GNU Make 4.3
Built for x86_64-redhat-linux-gnu
Copyright (C) 1988-2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

I'm asking because this change broke the build on our Fedora machines
and my laptop. None of them have 'yacc' installed, but they do have
'bison', which was used unconditionally before the change. After the
change, 'YACC' is set to 'yacc', as it's an implicit variable that
defaults to 'yacc':

https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html
David Ahern March 5, 2022, 6:11 p.m. UTC | #4
On 3/2/22 6:56 AM, Ido Schimmel wrote:
> David, are you sure this patch is needed? Even without it I can override
> from the command line:
> 
> $ make V=1 CC=gcc
> 
> lib
> make[1]: Entering directory '/home/idosch/code/iproute2/lib'
> gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c
> ...
> 
> $ make V=1 CC=clang
> 
> lib
> make[1]: Entering directory '/home/idosch/code/iproute2/lib'
> clang -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c
> 

interesting. As I recall the change was needed when I was testing
Stephen's patches for a clean compile with clang. Either way, the patch
was already merged.
Stephen Hemminger March 5, 2022, 7:32 p.m. UTC | #5
On Sat, 5 Mar 2022 11:11:43 -0700
David Ahern <dsahern@kernel.org> wrote:

> On 3/2/22 6:56 AM, Ido Schimmel wrote:
> > David, are you sure this patch is needed? Even without it I can override
> > from the command line:
> > 
> > $ make V=1 CC=gcc
> > 
> > lib
> > make[1]: Entering directory '/home/idosch/code/iproute2/lib'
> > gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c
> > ...
> > 
> > $ make V=1 CC=clang
> > 
> > lib
> > make[1]: Entering directory '/home/idosch/code/iproute2/lib'
> > clang -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c
> >   
> 
> interesting. As I recall the change was needed when I was testing
> Stephen's patches for a clean compile with clang. Either way, the patch
> was already merged.
> 

Right, it maybe necessary for configure. I was using clang after already having done
configure step on my system (with gcc).
Ido Schimmel March 7, 2022, 3:09 p.m. UTC | #6
On Sat, Mar 05, 2022 at 11:11:43AM -0700, David Ahern wrote:
> On 3/2/22 6:56 AM, Ido Schimmel wrote:
> > David, are you sure this patch is needed? Even without it I can override
> > from the command line:
> > 
> > $ make V=1 CC=gcc
> > 
> > lib
> > make[1]: Entering directory '/home/idosch/code/iproute2/lib'
> > gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c
> > ...
> > 
> > $ make V=1 CC=clang
> > 
> > lib
> > make[1]: Entering directory '/home/idosch/code/iproute2/lib'
> > clang -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -pipe -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -DHAVE_SETNS -DHAVE_HANDLE_AT -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -DNEED_STRLCPY -DHAVE_LIBCAP -fPIC   -c -o libgenl.o libgenl.c
> > 
> 
> interesting. As I recall the change was needed when I was testing
> Stephen's patches for a clean compile with clang. Either way, the patch
> was already merged.

I realize it was already merged (wasn't asking for academic purposes),
but rather wanted you to verify that the patch is not needed on your end
so that I could revert it. I can build with gcc/clang even without the
patch. With the patch, the build is broken on Fedora as "yacc" is not a
build dependency [1]. Verified this with a clean install of Fedora 35:
Can't build iproute with this patch after running "dnf builddep
iproute". Builds fine without it.

[1] https://src.fedoraproject.org/rpms/iproute/blob/rawhide/f/iproute.spec#_22
David Ahern March 8, 2022, 4:26 p.m. UTC | #7
On 3/7/22 8:09 AM, Ido Schimmel wrote:
> 
> I realize it was already merged (wasn't asking for academic purposes),
> but rather wanted you to verify that the patch is not needed on your end
> so that I could revert it. I can build with gcc/clang even without the
> patch. With the patch, the build is broken on Fedora as "yacc" is not a
> build dependency [1]. Verified this with a clean install of Fedora 35:
> Can't build iproute with this patch after running "dnf builddep
> iproute". Builds fine without it.
> 
> [1] https://src.fedoraproject.org/rpms/iproute/blob/rawhide/f/iproute.spec#_22

I booted a Fedora 35 VM and see what you mean. I can not find the
difference as to why Ubuntu 20.04 is ok and Fedora 35 is not, so I
reverted the patch.
Ido Schimmel March 8, 2022, 4:41 p.m. UTC | #8
On Tue, Mar 08, 2022 at 09:26:49AM -0700, David Ahern wrote:
> On 3/7/22 8:09 AM, Ido Schimmel wrote:
> > 
> > I realize it was already merged (wasn't asking for academic purposes),
> > but rather wanted you to verify that the patch is not needed on your end
> > so that I could revert it. I can build with gcc/clang even without the
> > patch. With the patch, the build is broken on Fedora as "yacc" is not a
> > build dependency [1]. Verified this with a clean install of Fedora 35:
> > Can't build iproute with this patch after running "dnf builddep
> > iproute". Builds fine without it.
> > 
> > [1] https://src.fedoraproject.org/rpms/iproute/blob/rawhide/f/iproute.spec#_22
> 
> I booted a Fedora 35 VM and see what you mean. I can not find the
> difference as to why Ubuntu 20.04 is ok and Fedora 35 is not, so I
> reverted the patch.

I tested with Ubuntu 21.10 yesterday and it worked for me as well.
There 'yacc' eventually points to '/usr/bin/bison.yacc'.

Anyway, thanks for taking care of that
diff mbox series

Patch

diff --git a/configure b/configure
index 8ddff43c6a7d..13f2d1739b99 100755
--- a/configure
+++ b/configure
@@ -19,10 +19,10 @@  check_toolchain()
     : ${AR=ar}
     : ${CC=gcc}
     : ${YACC=bison}
-    echo "PKG_CONFIG:=${PKG_CONFIG}" >>$CONFIG
-    echo "AR:=${AR}" >>$CONFIG
-    echo "CC:=${CC}" >>$CONFIG
-    echo "YACC:=${YACC}" >>$CONFIG
+    echo "PKG_CONFIG?=${PKG_CONFIG}" >>$CONFIG
+    echo "AR?=${AR}" >>$CONFIG
+    echo "CC?=${CC}" >>$CONFIG
+    echo "YACC?=${YACC}" >>$CONFIG
 }
 
 check_atm()