[libvirt] [PATCH] build: fix up some compiler flags
Matthias noted that the line: virt_aa_helper_LDFLAGS = $(WARN_CFLAGS) looks inconsistent, so I did an audit. Currently, the set of compiler warning flags passed to gcc as $CC are equally permitted as the set of linker flags passed to gcc as $LD, so there was no problem with that usage. But if we ever get in a situation where $CC and $LD treat particular flags differently, using the right variable form will make it easier. In the process, I spotted a couple of typos that were omitting useful flags, as well as specifying a -l under the wrong variable. * acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Define WARN_LDFLAGS as an alias for WARN_CFLAGS. * tools/Makefile.am (virsh_LDFLAGS): Use more canonical spelling. * proxy/Makefile.am (libvirt_proxy_LDFLAGS): Likewise. Move library... (libvirt_proxy_LDADD): ...here. * src/Makefile.am (virt_aa_helper_LDFLAGS): Use more canonical spelling of WARN_LDFLAGS. (libvirt_parthelper_LDFLAGS, libvirt_lxc_LDFLAGS): Likewise. Use correct spelling of COVERAGE_LDFLAGS. Reported by Matthias Bolte. --- acinclude.m4 | 3 ++- proxy/Makefile.am | 4 ++-- src/Makefile.am | 6 +++--- tools/Makefile.am | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/acinclude.m4 b/acinclude.m4 index 7fa2d67..f048879 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -78,7 +78,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS) WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" - AC_SUBST(WARN_CFLAGS) + AC_SUBST([WARN_CFLAGS]) + AC_SUBST([WARN_LDFLAGS]) dnl Needed to keep compile quiet on python 2.4 COMPILER_FLAGS= diff --git a/proxy/Makefile.am b/proxy/Makefile.am index bee47d0..4716683 100644 --- a/proxy/Makefile.am +++ b/proxy/Makefile.am @@ -32,9 +32,9 @@ libvirt_proxy_SOURCES = libvirt_proxy.c \ @top_srcdir@/src/xen/xen_hypervisor.c \ @top_srcdir@/src/xen/sexpr.c \ @top_srcdir@/src/xen/xs_internal.c -libvirt_proxy_LDFLAGS = $(WARN_CFLAGS) $(XEN_LIBS) +libvirt_proxy_LDFLAGS = $(WARN_LDFLAGS) libvirt_proxy_DEPENDENCIES = -libvirt_proxy_LDADD = ../gnulib/lib/libgnu.la $(LIB_PTHREAD) +libvirt_proxy_LDADD = ../gnulib/lib/libgnu.la $(XEN_LIBS) $(LIB_PTHREAD) install-exec-hook: chmod u+s $(DESTDIR)$(libexecdir)/libvirt_proxy diff --git a/src/Makefile.am b/src/Makefile.am index 72f23a7..034feec 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1004,7 +1004,7 @@ if WITH_LIBVIRTD libexec_PROGRAMS += libvirt_parthelper libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) -libvirt_parthelper_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) +libvirt_parthelper_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS) libvirt_parthelper_LDADD = $(LIBPARTED_LIBS) libvirt_parthelper_CFLAGS = $(LIBPARTED_CFLAGS) endif @@ -1024,7 +1024,7 @@ libvirt_lxc_SOURCES = \ $(DOMAIN_CONF_SOURCES) \ $(CPU_CONF_SOURCES) \ $(NWFILTER_PARAM_CONF_SOURCES) -libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) +libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \ $(LIBXML_LIBS) $(NUMACTL_LIBS) $(LIB_PTHREAD) \ ../gnulib/lib/libgnu.la @@ -1044,7 +1044,7 @@ libexec_PROGRAMS += virt-aa-helper virt_aa_helper_SOURCES = $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) -virt_aa_helper_LDFLAGS = $(WARN_CFLAGS) +virt_aa_helper_LDFLAGS = $(WARN_LDFLAGS) virt_aa_helper_LDADD = \ $(LIBXML_LIBS) \ libvirt_conf.la \ diff --git a/tools/Makefile.am b/tools/Makefile.am index 33a3216..fd05e8b 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -34,14 +34,14 @@ virsh_SOURCES = \ console.c console.h \ virsh.c -virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) +virsh_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS) virsh_LDADD = \ $(STATIC_BINARIES) \ $(WARN_CFLAGS) \ ../src/libvirt.la \ ../gnulib/lib/libgnu.la \ $(VIRSH_LIBS) -virsh_CFLAGS = \ +virsh_CFLAGS = \ -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ -I../include -I$(top_srcdir)/include \ -I$(top_srcdir)/src \ -- 1.7.0.1
On Fri, May 14, 2010 at 04:58:14PM -0600, Eric Blake wrote:
Matthias noted that the line: virt_aa_helper_LDFLAGS = $(WARN_CFLAGS) looks inconsistent, so I did an audit.
Currently, the set of compiler warning flags passed to gcc as $CC are equally permitted as the set of linker flags passed to gcc as $LD, so there was no problem with that usage. But if we ever get in a situation where $CC and $LD treat particular flags differently, using the right variable form will make it easier.
In the process, I spotted a couple of typos that were omitting useful flags, as well as specifying a -l under the wrong variable.
* acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Define WARN_LDFLAGS as an alias for WARN_CFLAGS. * tools/Makefile.am (virsh_LDFLAGS): Use more canonical spelling. * proxy/Makefile.am (libvirt_proxy_LDFLAGS): Likewise. Move library... (libvirt_proxy_LDADD): ...here. * src/Makefile.am (virt_aa_helper_LDFLAGS): Use more canonical spelling of WARN_LDFLAGS. (libvirt_parthelper_LDFLAGS, libvirt_lxc_LDFLAGS): Likewise. Use correct spelling of COVERAGE_LDFLAGS. Reported by Matthias Bolte. --- acinclude.m4 | 3 ++- proxy/Makefile.am | 4 ++-- src/Makefile.am | 6 +++--- tools/Makefile.am | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4 index 7fa2d67..f048879 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -78,7 +78,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS)
WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" - AC_SUBST(WARN_CFLAGS) + AC_SUBST([WARN_CFLAGS]) + AC_SUBST([WARN_LDFLAGS])
Where is WARN_LDFLAGS actually defined ? This looks like it is just setting it to empty string ? Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 05/16/2010 03:24 AM, Daniel P. Berrange wrote:
* acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Define WARN_LDFLAGS as an alias for WARN_CFLAGS. +++ b/acinclude.m4 @@ -78,7 +78,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS)
WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" - AC_SUBST(WARN_CFLAGS) + AC_SUBST([WARN_CFLAGS]) + AC_SUBST([WARN_LDFLAGS])
Where is WARN_LDFLAGS actually defined ? This looks like it is just setting it to empty string ?
Good catch. I'm squashing this in; do you need to see the revised patch before giving an ACK? diff --git i/acinclude.m4 w/acinclude.m4 index f048879..8c97184 100644 --- i/acinclude.m4 +++ w/acinclude.m4 @@ -78,6 +78,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS) WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" + WARN_LDFLAGS=$WARN_CFLAGS AC_SUBST([WARN_CFLAGS]) AC_SUBST([WARN_LDFLAGS]) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, May 17, 2010 at 08:36:59AM -0600, Eric Blake wrote:
On 05/16/2010 03:24 AM, Daniel P. Berrange wrote:
* acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Define WARN_LDFLAGS as an alias for WARN_CFLAGS. +++ b/acinclude.m4 @@ -78,7 +78,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS)
WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" - AC_SUBST(WARN_CFLAGS) + AC_SUBST([WARN_CFLAGS]) + AC_SUBST([WARN_LDFLAGS])
Where is WARN_LDFLAGS actually defined ? This looks like it is just setting it to empty string ?
Good catch. I'm squashing this in; do you need to see the revised patch before giving an ACK?
diff --git i/acinclude.m4 w/acinclude.m4 index f048879..8c97184 100644 --- i/acinclude.m4 +++ w/acinclude.m4 @@ -78,6 +78,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS)
WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" + WARN_LDFLAGS=$WARN_CFLAGS AC_SUBST([WARN_CFLAGS]) AC_SUBST([WARN_LDFLAGS])
ACK, that's fine as an addition Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 05/17/2010 08:39 AM, Daniel P. Berrange wrote:
Where is WARN_LDFLAGS actually defined ? This looks like it is just setting it to empty string ?
Good catch. I'm squashing this in; do you need to see the revised patch before giving an ACK?
diff --git i/acinclude.m4 w/acinclude.m4 index f048879..8c97184 100644 --- i/acinclude.m4 +++ w/acinclude.m4 @@ -78,6 +78,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS)
WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" + WARN_LDFLAGS=$WARN_CFLAGS AC_SUBST([WARN_CFLAGS]) AC_SUBST([WARN_LDFLAGS])
ACK, that's fine as an addition
Thanks; pushed as amended. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake