[libvirt] [PATCH] build: simplify sanlock distribution

EXTRA_DIST is intended for files that must unconditionally be part of the tarball, so it should be outside HAVE_SANLOCK. Once that is fixed, then there is no need to cp an unchanged qemu-sanlock.conf from srcdir to builddir in a VPATH build, not to mention that $(builddir) is not supported in the automake in use in RHEL 5. * src/Makefile.am ($(builddir)/locking/%-sanlock.conf): Delete. (BUILT_SOURCES): No need to build qemu-sanlock.conf. (EXTRA_DIST): Always ship sanlock .aug and .conf files. --- As promised in: https://www.redhat.com/archives/libvir-list/2011-June/msg01448.html I'm starting a 'make distcheck' overnight to fully test this, but think that it will pass without issues. src/Makefile.am | 15 +++++---------- 1 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8f99cc2..bfbaa06 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1186,6 +1186,10 @@ libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE) +EXTRA_DIST += locking/sanlock.conf \ + locking/libvirt_sanlock.aug \ + locking/test_libvirt_sanlock.aug + if HAVE_SANLOCK lockdriverdir = $(libdir)/libvirt/lock-driver lockdriver_LTLIBRARIES = sanlock.la @@ -1199,20 +1203,11 @@ sanlock_la_LIBADD = -lsanlock \ augeas_DATA += locking/libvirt_sanlock.aug augeastest_DATA += locking/test_libvirt_sanlock.aug -EXTRA_DIST += locking/sanlock.conf \ - locking/libvirt_sanlock.aug \ - locking/test_libvirt_sanlock.aug - -$(builddir)/locking/%-sanlock.conf: $(srcdir)/locking/sanlock.conf - $(AM_V_GEN)mkdir locking ; \ - cp $< $@ - if WITH_QEMU conf_DATA += locking/qemu-sanlock.conf -BUILT_SOURCES += locking/qemu-sanlock.conf endif else -EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) locking/sanlock.conf +EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) endif noinst_LTLIBRARIES += libvirt-net-rpc.la libvirt-net-rpc-server.la libvirt-net-rpc-client.la -- 1.7.4.4

Eric Blake <eblake@redhat.com> writes:
EXTRA_DIST is intended for files that must unconditionally be part of the tarball, so it should be outside HAVE_SANLOCK. Once that is fixed, then there is no need to cp an unchanged qemu-sanlock.conf from srcdir to builddir in a VPATH build, not to mention that $(builddir) is not supported in the automake in use in RHEL 5.
The way that EXTRA_DIST honors Automake conditionals is, in my opinion, surprising. In some of my own projects, I've just added noinst_HEADERS += $(EXTRA_DIST) outside any Automake conditional to fix that. When you do that, any file mentioned in EXTRA_DIST is always distributed, regardless of conditionals. See also: http://lists.gnu.org/archive/html/automake/2009-07/msg00049.html http://lists.gnu.org/archive/html/automake/2009-07/msg00051.html -- Ben Pfaff http://benpfaff.org

On Tue, Jun 28, 2011 at 10:22:17PM -0600, Eric Blake wrote:
EXTRA_DIST is intended for files that must unconditionally be part of the tarball, so it should be outside HAVE_SANLOCK. Once that is fixed, then there is no need to cp an unchanged qemu-sanlock.conf from srcdir to builddir in a VPATH build, not to mention that $(builddir) is not supported in the automake in use in RHEL 5.
* src/Makefile.am ($(builddir)/locking/%-sanlock.conf): Delete. (BUILT_SOURCES): No need to build qemu-sanlock.conf. (EXTRA_DIST): Always ship sanlock .aug and .conf files. ---
As promised in: https://www.redhat.com/archives/libvir-list/2011-June/msg01448.html
I'm starting a 'make distcheck' overnight to fully test this, but think that it will pass without issues.
src/Makefile.am | 15 +++++---------- 1 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index 8f99cc2..bfbaa06 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1186,6 +1186,10 @@ libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
+EXTRA_DIST += locking/sanlock.conf \ + locking/libvirt_sanlock.aug \ + locking/test_libvirt_sanlock.aug + if HAVE_SANLOCK lockdriverdir = $(libdir)/libvirt/lock-driver lockdriver_LTLIBRARIES = sanlock.la @@ -1199,20 +1203,11 @@ sanlock_la_LIBADD = -lsanlock \ augeas_DATA += locking/libvirt_sanlock.aug augeastest_DATA += locking/test_libvirt_sanlock.aug
-EXTRA_DIST += locking/sanlock.conf \ - locking/libvirt_sanlock.aug \ - locking/test_libvirt_sanlock.aug - -$(builddir)/locking/%-sanlock.conf: $(srcdir)/locking/sanlock.conf - $(AM_V_GEN)mkdir locking ; \ - cp $< $@
How does 'qemu-sanlock.conf' get created now ? This rule is what actually tells make how to create 'qemu-sanlock.conf' from 'sanlock.conf' Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 06/29/2011 03:38 AM, Daniel P. Berrange wrote:
- -$(builddir)/locking/%-sanlock.conf: $(srcdir)/locking/sanlock.conf - $(AM_V_GEN)mkdir locking ; \ - cp $< $@
How does 'qemu-sanlock.conf' get created now ? This rule is what actually tells make how to create 'qemu-sanlock.conf' from 'sanlock.conf'
Serves me right for coding too late at night. I guess you have a point - you want to ship sanlock.conf as a template, as well as install multiple <HV>-sanlock.conf files, one for each <HV> that supports sanlock (right now, just qemu), while still storing only one file in libvirt.git. I missed the '%-' in the rule in my first read of it. In which case, my patch is wrong. But the existing code is also wrong - the EXTRA_DIST still needs to be moved outside of HAVE_SANLOCK, and the 'mkdir' needs to instead be '$(MKDIR_P)' for parallel builds. v2 coming up. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

EXTRA_DIST files should unconditionally be part of the tarball, rather than depending on the presence of sanlock-devel. Meanwhile, parallel builds could fail if we don't use mkdir -p. * src/Makefile.am (EXTRA_DIST): Always ship sanlock .aug and template .conf files. (%-sanlock.conf): Use MKDIR_P. --- src/Makefile.am | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8f99cc2..956bc0b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1186,6 +1186,10 @@ libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE) +EXTRA_DIST += locking/sanlock.conf \ + locking/libvirt_sanlock.aug \ + locking/test_libvirt_sanlock.aug + if HAVE_SANLOCK lockdriverdir = $(libdir)/libvirt/lock-driver lockdriver_LTLIBRARIES = sanlock.la @@ -1199,12 +1203,8 @@ sanlock_la_LIBADD = -lsanlock \ augeas_DATA += locking/libvirt_sanlock.aug augeastest_DATA += locking/test_libvirt_sanlock.aug -EXTRA_DIST += locking/sanlock.conf \ - locking/libvirt_sanlock.aug \ - locking/test_libvirt_sanlock.aug - $(builddir)/locking/%-sanlock.conf: $(srcdir)/locking/sanlock.conf - $(AM_V_GEN)mkdir locking ; \ + $(AM_V_GEN)$(MKDIR_P) locking ; \ cp $< $@ if WITH_QEMU @@ -1212,7 +1212,7 @@ conf_DATA += locking/qemu-sanlock.conf BUILT_SOURCES += locking/qemu-sanlock.conf endif else -EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) locking/sanlock.conf +EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES) endif noinst_LTLIBRARIES += libvirt-net-rpc.la libvirt-net-rpc-server.la libvirt-net-rpc-client.la -- 1.7.4.4

On 06/29/2011 11:31 AM, Eric Blake wrote:
EXTRA_DIST files should unconditionally be part of the tarball, rather than depending on the presence of sanlock-devel.
Meanwhile, parallel builds could fail if we don't use mkdir -p.
* src/Makefile.am (EXTRA_DIST): Always ship sanlock .aug and template .conf files. (%-sanlock.conf): Use MKDIR_P. --- src/Makefile.am | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
ACK.

On 06/30/2011 05:09 PM, Laine Stump wrote:
On 06/29/2011 11:31 AM, Eric Blake wrote:
EXTRA_DIST files should unconditionally be part of the tarball, rather than depending on the presence of sanlock-devel.
Meanwhile, parallel builds could fail if we don't use mkdir -p.
* src/Makefile.am (EXTRA_DIST): Always ship sanlock .aug and template .conf files. (%-sanlock.conf): Use MKDIR_P. --- src/Makefile.am | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
ACK.
Thanks; applied. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Ben Pfaff
-
Daniel P. Berrange
-
Eric Blake
-
Laine Stump