[libvirt] [PATCH 0/3] build: drop some nested makefiles
This kills some nested makefiles. They don't hurt much but they are largely redudant so don't serve much usefulness either. After this we have a strict two level Makefile hierarchy, which is an easy rule of thumb to follow. The first two patches clean up blantantly redundant makesfiles Patch 3 is a bit trickier with the wireshark plugin, but it's also more useful to fix since now it can be built in parallel with the rest of the tools/ directory. Cole Robinson (3): build: Kill include/libvirt/Makefile.am build: Kill docs/schemas/Makefile.am build: Kill tools/wireshark Makefiles Makefile.am | 2 +- configure.ac | 7 ++---- docs/Makefile.am | 10 ++++---- docs/schemas/Makefile.am | 35 --------------------------- include/Makefile.am | 19 +++++++++++++-- include/libvirt/Makefile.am | 44 ---------------------------------- tools/Makefile.am | 52 ++++++++++++++++++++++++++++++++++++++--- tools/wireshark/Makefile.am | 23 ------------------ tools/wireshark/src/Makefile.am | 49 -------------------------------------- 9 files changed, 75 insertions(+), 166 deletions(-) delete mode 100644 docs/schemas/Makefile.am delete mode 100644 include/libvirt/Makefile.am delete mode 100644 tools/wireshark/Makefile.am delete mode 100644 tools/wireshark/src/Makefile.am -- 2.5.0
Move all the logic to include/Makefile.am, simplify it with a wildcard, then kill include/libvirt/Makefile.am --- configure.ac | 2 +- include/Makefile.am | 19 +++++++++++++++++-- include/libvirt/Makefile.am | 44 -------------------------------------------- 3 files changed, 18 insertions(+), 47 deletions(-) delete mode 100644 include/libvirt/Makefile.am diff --git a/configure.ac b/configure.ac index a46f9b3..787d0c6 100644 --- a/configure.ac +++ b/configure.ac @@ -2808,7 +2808,7 @@ AC_CONFIG_FILES([\ src/libvirt-lxc.pc \ libvirt.spec mingw-libvirt.spec \ po/Makefile.in \ - include/libvirt/Makefile include/libvirt/libvirt-common.h \ + include/libvirt/libvirt-common.h \ daemon/Makefile \ examples/Makefile \ tests/Makefile \ diff --git a/include/Makefile.am b/include/Makefile.am index 80361a7..1805700 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2011, 2013 Red Hat, Inc. +## Copyright (C) 2005-2011, 2013-2016 Red Hat, Inc. ## ## This library is free software; you can redistribute it and/or ## modify it under the terms of the GNU Lesser General Public @@ -16,4 +16,19 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>. -SUBDIRS=libvirt +virincdir = $(includedir)/libvirt + +allheaders = $(wildcard libvirt/*.h) +virinc_HEADERS = $(filter-out libvirt/libvirt-admin.h, $(allheaders)) + +EXTRA_DIST = libvirt/libvirt-common.h.in + +# Temporarily disabled, but we need it for building +EXTRA_DIST += libvirt/libvirt-admin.h + +all: + echo $(EXTRA_DIST) + echo $(virinc_HEADERS) + +install-exec-hook: + $(mkinstalldirs) $(DESTDIR)$(virincdir) diff --git a/include/libvirt/Makefile.am b/include/libvirt/Makefile.am deleted file mode 100644 index 5a4ada0..0000000 --- a/include/libvirt/Makefile.am +++ /dev/null @@ -1,44 +0,0 @@ -## Process this file with automake to produce Makefile.in - -## Copyright (C) 2005-2011, 2013-2015 Red Hat, Inc. -## -## This library is free software; you can redistribute it and/or -## modify it under the terms of the GNU Lesser General Public -## License as published by the Free Software Foundation; either -## version 2.1 of the License, or (at your option) any later version. -## -## This library is distributed in the hope that it will be useful, -## but WITHOUT ANY WARRANTY; without even the implied warranty of -## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -## Lesser General Public License for more details. -## -## You should have received a copy of the GNU Lesser General Public -## License along with this library. If not, see -## <http://www.gnu.org/licenses/>. - -virincdir = $(includedir)/libvirt - -virinc_HEADERS = libvirt.h \ - libvirt-common.h \ - libvirt-domain.h \ - libvirt-domain-snapshot.h \ - libvirt-event.h \ - libvirt-host.h \ - libvirt-interface.h \ - libvirt-network.h \ - libvirt-nodedev.h \ - libvirt-nwfilter.h \ - libvirt-secret.h \ - libvirt-storage.h \ - libvirt-stream.h \ - libvirt-lxc.h \ - libvirt-qemu.h \ - virterror.h - -install-exec-hook: - $(mkinstalldirs) $(DESTDIR)$(virincdir) - -EXTRA_DIST = libvirt-common.h.in - -# Temporarily disabled, but we need it for building -EXTRA_DIST += libvirt-admin.h -- 2.5.0
On Sun, Jan 10, 2016 at 06:06:21PM -0500, Cole Robinson wrote:
Move all the logic to include/Makefile.am, simplify it with a wildcard, then kill include/libvirt/Makefile.am --- configure.ac | 2 +- include/Makefile.am | 19 +++++++++++++++++-- include/libvirt/Makefile.am | 44 -------------------------------------------- 3 files changed, 18 insertions(+), 47 deletions(-) delete mode 100644 include/libvirt/Makefile.am
diff --git a/configure.ac b/configure.ac index a46f9b3..787d0c6 100644 --- a/configure.ac +++ b/configure.ac @@ -2808,7 +2808,7 @@ AC_CONFIG_FILES([\ src/libvirt-lxc.pc \ libvirt.spec mingw-libvirt.spec \ po/Makefile.in \ - include/libvirt/Makefile include/libvirt/libvirt-common.h \ + include/libvirt/libvirt-common.h \ daemon/Makefile \ examples/Makefile \ tests/Makefile \ diff --git a/include/Makefile.am b/include/Makefile.am index 80361a7..1805700 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2011, 2013 Red Hat, Inc. +## Copyright (C) 2005-2011, 2013-2016 Red Hat, Inc. ## ## This library is free software; you can redistribute it and/or ## modify it under the terms of the GNU Lesser General Public @@ -16,4 +16,19 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>.
-SUBDIRS=libvirt +virincdir = $(includedir)/libvirt + +allheaders = $(wildcard libvirt/*.h) +virinc_HEADERS = $(filter-out libvirt/libvirt-admin.h, $(allheaders)) + +EXTRA_DIST = libvirt/libvirt-common.h.in + +# Temporarily disabled, but we need it for building +EXTRA_DIST += libvirt/libvirt-admin.h + +all: + echo $(EXTRA_DIST) + echo $(virinc_HEADERS) +
Leftover from debugging? Oterwise looks fine.
On 01/11/2016 02:47 AM, Martin Kletzander wrote:
On Sun, Jan 10, 2016 at 06:06:21PM -0500, Cole Robinson wrote:
Move all the logic to include/Makefile.am, simplify it with a wildcard, then kill include/libvirt/Makefile.am --- configure.ac | 2 +- include/Makefile.am | 19 +++++++++++++++++-- include/libvirt/Makefile.am | 44 -------------------------------------------- 3 files changed, 18 insertions(+), 47 deletions(-) delete mode 100644 include/libvirt/Makefile.am
diff --git a/configure.ac b/configure.ac index a46f9b3..787d0c6 100644 --- a/configure.ac +++ b/configure.ac @@ -2808,7 +2808,7 @@ AC_CONFIG_FILES([\ src/libvirt-lxc.pc \ libvirt.spec mingw-libvirt.spec \ po/Makefile.in \ - include/libvirt/Makefile include/libvirt/libvirt-common.h \ + include/libvirt/libvirt-common.h \ daemon/Makefile \ examples/Makefile \ tests/Makefile \ diff --git a/include/Makefile.am b/include/Makefile.am index 80361a7..1805700 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in
-## Copyright (C) 2005-2011, 2013 Red Hat, Inc. +## Copyright (C) 2005-2011, 2013-2016 Red Hat, Inc. ## ## This library is free software; you can redistribute it and/or ## modify it under the terms of the GNU Lesser General Public @@ -16,4 +16,19 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>.
-SUBDIRS=libvirt +virincdir = $(includedir)/libvirt + +allheaders = $(wildcard libvirt/*.h) +virinc_HEADERS = $(filter-out libvirt/libvirt-admin.h, $(allheaders)) + +EXTRA_DIST = libvirt/libvirt-common.h.in + +# Temporarily disabled, but we need it for building +EXTRA_DIST += libvirt/libvirt-admin.h + +all: + echo $(EXTRA_DIST) + echo $(virinc_HEADERS) +
Leftover from debugging?
Yes, thanks for catching. I'll push the first 2 patches with that dropped - Cole
Move the logic to docs/Makefile.am, and simplify it with a wildcard expression. --- configure.ac | 1 - docs/Makefile.am | 10 ++++++---- docs/schemas/Makefile.am | 35 ----------------------------------- 3 files changed, 6 insertions(+), 40 deletions(-) delete mode 100644 docs/schemas/Makefile.am diff --git a/configure.ac b/configure.ac index 787d0c6..58807a8 100644 --- a/configure.ac +++ b/configure.ac @@ -2796,7 +2796,6 @@ AC_CONFIG_FILES([run], [chmod +x,-w run]) AC_CONFIG_FILES([\ Makefile src/Makefile include/Makefile docs/Makefile \ - docs/schemas/Makefile \ gnulib/lib/Makefile \ gnulib/tests/Makefile \ libvirt.pc \ diff --git a/docs/Makefile.am b/docs/Makefile.am index 13d7a5f..5f531ea 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -1,6 +1,6 @@ ## Process this file with automake to produce Makefile.in -## Copyright (C) 2005-2015 Red Hat, Inc. +## Copyright (C) 2005-2016 Red Hat, Inc. ## ## This library is free software; you can redistribute it and/or ## modify it under the terms of the GNU Lesser General Public @@ -16,8 +16,6 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>. -SUBDIRS= schemas - PERL = perl # The directory containing the source code (if it contains documentation). @@ -150,6 +148,9 @@ fig = \ migration-tunnel.fig \ migration-unmanaged-direct.fig +schemadir = $(pkgdatadir)/schemas +schema_DATA = $(wildcard schemas/*.rng) + EXTRA_DIST= \ apibuild.py genaclperms.pl \ site.xsl newapi.xsl news.xsl page.xsl \ @@ -160,7 +161,8 @@ EXTRA_DIST= \ $(patches) $(dot_php_in) $(dot_php_code_in) $(dot_php)\ $(internals_html_in) $(internals_html) \ sitemap.html.in aclperms.htmlinc \ - todo.pl hvsupport.pl todo.cfg-example + todo.pl hvsupport.pl todo.cfg-example \ + $(schema_DATA) acl_generated = aclperms.htmlinc diff --git a/docs/schemas/Makefile.am b/docs/schemas/Makefile.am deleted file mode 100644 index 0e14dc6..0000000 --- a/docs/schemas/Makefile.am +++ /dev/null @@ -1,35 +0,0 @@ -## Copyright (C) 2005-2011, 2013-2014 Red Hat, Inc. -## -## This library is free software; you can redistribute it and/or -## modify it under the terms of the GNU Lesser General Public -## License as published by the Free Software Foundation; either -## version 2.1 of the License, or (at your option) any later version. -## -## This library is distributed in the hope that it will be useful, -## but WITHOUT ANY WARRANTY; without even the implied warranty of -## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -## Lesser General Public License for more details. -## -## You should have received a copy of the GNU Lesser General Public -## License along with this library. If not, see -## <http://www.gnu.org/licenses/>. - -schemadir = $(pkgdatadir)/schemas -schema_DATA = \ - basictypes.rng \ - capability.rng \ - domain.rng \ - domaincaps.rng \ - domaincommon.rng \ - domainsnapshot.rng \ - interface.rng \ - network.rng \ - networkcommon.rng \ - nodedev.rng \ - nwfilter.rng \ - secret.rng \ - storagecommon.rng \ - storagepool.rng \ - storagevol.rng - -EXTRA_DIST = $(schema_DATA) -- 2.5.0
Just handle it all in tools/Makefile.am. I verified the generated output looks similar to the pre patch output, but I didn't test it. --- Makefile.am | 2 +- configure.ac | 4 +--- tools/Makefile.am | 52 ++++++++++++++++++++++++++++++++++++++--- tools/wireshark/Makefile.am | 23 ------------------ tools/wireshark/src/Makefile.am | 49 -------------------------------------- 5 files changed, 51 insertions(+), 79 deletions(-) delete mode 100644 tools/wireshark/Makefile.am delete mode 100644 tools/wireshark/src/Makefile.am diff --git a/Makefile.am b/Makefile.am index 708d051..ffe0517 100644 --- a/Makefile.am +++ b/Makefile.am @@ -20,7 +20,7 @@ LCOV = lcov GENHTML = genhtml SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \ - tests po examples tools/wireshark + tests po examples ACLOCAL_AMFLAGS = -I m4 diff --git a/configure.ac b/configure.ac index 58807a8..9398f03 100644 --- a/configure.ac +++ b/configure.ac @@ -2811,9 +2811,7 @@ AC_CONFIG_FILES([\ daemon/Makefile \ examples/Makefile \ tests/Makefile \ - tools/Makefile \ - tools/wireshark/Makefile \ - tools/wireshark/src/Makefile]) + tools/Makefile]) AC_OUTPUT AC_MSG_NOTICE([]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 73cad50..e5c186c 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -1,4 +1,5 @@ -## Copyright (C) 2005-2015 Red Hat, Inc. +## Copyright (C) 2005-2016 Red Hat, Inc. +## Copyright (C) 2013 Yuto KAWAMURA(kawamuray) <kawamuray.dadada@gmail.com> ## ## This library is free software; you can redistribute it and/or ## modify it under the terms of the GNU Lesser General Public @@ -56,7 +57,7 @@ EXTRA_DIST = \ virsh-volume.c - +CLEANFILES = DISTCLEANFILES = confdir = $(sysconfdir)/libvirt @@ -374,7 +375,52 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status mv $@-t $@ -CLEANFILES = $(bin_SCRIPTS) +EXTRA_DIST += \ + wireshark/util/genxdrstub.pl \ + wireshark/util/make-dissector-reg + +if WITH_WIRESHARK_DISSECTOR + +ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la +wireshark_src_libvirt_la_CPPFLAGS = \ + -I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS) +wireshark_src_libvirt_la_LDFLAGS = -avoid-version -module +wireshark_src_libvirt_la_SOURCES = \ + wireshark/src/packet-libvirt.h \ + wireshark/src/packet-libvirt.c \ + wireshark/src/plugin.c + +wireshark/src/packet-libvirt.c: wireshark/src/packet-libvirt.h \ + wireshark/src/libvirt/protocol.h + +wireshark/src/plugin.c: wireshark/src/packet-libvirt.c + cd wireshark/src && \ + $(abs_top_srcdir)/tools/wireshark/util/make-dissector-reg \ + . plugin packet-libvirt.c + +WS_DISSECTOR_PROTO_FILES = \ + $(abs_top_srcdir)/src/remote/remote_protocol.x \ + $(abs_top_srcdir)/src/remote/qemu_protocol.x \ + $(abs_top_srcdir)/src/remote/lxc_protocol.x \ + $(abs_top_srcdir)/src/rpc/virkeepaliveprotocol.x + +wireshark/src/libvirt/protocol.h: wireshark/util/genxdrstub.pl \ + $(WS_DISSECTOR_PROTO_FILES) + $(MKDIR_P) wireshark/src/libvirt + cd wireshark/src && \ + LIBVIRT_VERSION=$(LIBVIRT_VERSION) \ + $(PERL) $(abs_top_srcdir)/tools/wireshark/util/genxdrstub.pl \ + $(WS_DISSECTOR_PROTO_FILES) + +CLEANFILES += wireshark/src/plugin.c + +endif WITH_WIRESHARK_DISSECTOR + + +clean-local: + -rm -rf wireshark/src/libvirt + +CLEANFILES += $(bin_SCRIPTS) CLEANFILES += *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.i *.s MAINTAINERCLEANFILES = $(dist_man1_MANS) diff --git a/tools/wireshark/Makefile.am b/tools/wireshark/Makefile.am deleted file mode 100644 index 28e6ed8..0000000 --- a/tools/wireshark/Makefile.am +++ /dev/null @@ -1,23 +0,0 @@ -## Process this file with automake to produce Makefile.in - -# Copyright (C) 2013 Yuto KAWAMURA(kawamuray) <kawamuray.dadada@gmail.com> -# -# This library is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License as published by the Free Software Foundation; either -# version 2.1 of the License, or (at your option) any later version. -# -# This library is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public -# License along with this library. If not, see -# <http://www.gnu.org/licenses/>. -# -# Author: Yuto KAWAMURA(kawamuray) -if WITH_WIRESHARK_DISSECTOR -SUBDIRS = src -endif WITH_WIRESHARK_DISSECTOR -EXTRA_DIST = util/genxdrstub.pl util/make-dissector-reg diff --git a/tools/wireshark/src/Makefile.am b/tools/wireshark/src/Makefile.am deleted file mode 100644 index a7d775f..0000000 --- a/tools/wireshark/src/Makefile.am +++ /dev/null @@ -1,49 +0,0 @@ -## Process this file with automake to produce Makefile.in - -# Copyright (C) 2013 Yuto KAWAMURA(kawamuray) <kawamuray.dadada@gmail.com> -# -# This library is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License as published by the Free Software Foundation; either -# version 2.1 of the License, or (at your option) any later version. -# -# This library is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public -# License along with this library. If not, see -# <http://www.gnu.org/licenses/>. -# -# Author: Yuto KAWAMURA(kawamuray) - -INCLUDES = \ - -I$(top_srcdir) \ - -I$(top_srcdir)/src -I$(top_builddir)/src \ - -I$(top_srcdir)/include -I$(top_builddir)/include \ - -I$(top_srcdir)/gnulib/lib -I$(top_builddir)/gnulib/lib - -ws_plugin_LTLIBRARIES = libvirt.la -libvirt_la_SOURCES = packet-libvirt.h packet-libvirt.c plugin.c -libvirt_la_CPPFLAGS = $(WIRESHARK_DISSECTOR_CFLAGS) -libvirt_la_LDFLAGS = -avoid-version -module - -packet-libvirt.c: packet-libvirt.h libvirt/protocol.h - -plugin.c: packet-libvirt.c - $(srcdir)/../util/make-dissector-reg . plugin $< - -WS_DISSECTOR_PROTO_FILES = \ - $(top_srcdir)/src/remote/remote_protocol.x \ - $(top_srcdir)/src/remote/qemu_protocol.x \ - $(top_srcdir)/src/remote/lxc_protocol.x \ - $(top_srcdir)/src/rpc/virkeepaliveprotocol.x - -libvirt/protocol.h: $(srcdir)/../util/genxdrstub.pl $(WS_DISSECTOR_PROTO_FILES) - $(MKDIR_P) libvirt - LIBVIRT_VERSION=$(LIBVIRT_VERSION) \ - $(PERL) $(srcdir)/../util/genxdrstub.pl $(WS_DISSECTOR_PROTO_FILES) - -clean-local: - -rm -rf libvirt plugin.c -- 2.5.0
On 11.01.2016 00:06, Cole Robinson wrote:
Just handle it all in tools/Makefile.am. I verified the generated output looks similar to the pre patch output, but I didn't test it. --- Makefile.am | 2 +- configure.ac | 4 +--- tools/Makefile.am | 52 ++++++++++++++++++++++++++++++++++++++--- tools/wireshark/Makefile.am | 23 ------------------ tools/wireshark/src/Makefile.am | 49 -------------------------------------- 5 files changed, 51 insertions(+), 79 deletions(-) delete mode 100644 tools/wireshark/Makefile.am delete mode 100644 tools/wireshark/src/Makefile.am
ACK and safe for freeze. Michal
On 01/12/2016 11:06 AM, Michal Privoznik wrote:
On 11.01.2016 00:06, Cole Robinson wrote:
Just handle it all in tools/Makefile.am. I verified the generated output looks similar to the pre patch output, but I didn't test it. --- Makefile.am | 2 +- configure.ac | 4 +--- tools/Makefile.am | 52 ++++++++++++++++++++++++++++++++++++++--- tools/wireshark/Makefile.am | 23 ------------------ tools/wireshark/src/Makefile.am | 49 -------------------------------------- 5 files changed, 51 insertions(+), 79 deletions(-) delete mode 100644 tools/wireshark/Makefile.am delete mode 100644 tools/wireshark/src/Makefile.am
ACK and safe for freeze.
Michal
Thanks, pushed now - Cole
On Sun, Jan 10, 2016 at 06:06:20PM -0500, Cole Robinson wrote:
This kills some nested makefiles. They don't hurt much but they are largely redudant so don't serve much usefulness either. After this we have a strict two level Makefile hierarchy, which is an easy rule of thumb to follow.
The first two patches clean up blantantly redundant makesfiles Patch 3 is a bit trickier with the wireshark plugin, but it's also more useful to fix since now it can be built in parallel with the rest of the tools/ directory.
Cole Robinson (3): build: Kill include/libvirt/Makefile.am build: Kill docs/schemas/Makefile.am build: Kill tools/wireshark Makefiles
ACK to 1 and 2 (with 1 cleaned up according to the comment), but I can't really decide about the last one. We are handling quite a bit of path separators in rule names and that doesn't seem that nice. Anyway, I'm not against it, I would rather someone tested it before ACKing it. And since I can't currently build with wireshark dissector due to some non-compatible changes and me having installed supernew wireshark, so I'll leave it up to Michal^Wsomeone else.
participants (3)
-
Cole Robinson -
Martin Kletzander -
Michal Privoznik