[libvirt] [PATCH] build: Fix build with old automake
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82). Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Makefile.am b/src/Makefile.am index d4d7b2b..fce9056 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -16,6 +16,10 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>. +# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd) + # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets # that actually use them. Also keep GETTEXT_CPPFLAGS at the end. -- 1.9.0
On 14.3.2014 11:43, Jiri Denemark wrote:
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/Makefile.am b/src/Makefile.am index d4d7b2b..fce9056 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -16,6 +16,10 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>.
+# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd) + # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets # that actually use them. Also keep GETTEXT_CPPFLAGS at the end.
Tested on RHEL-5 and it works, ACK Pavel
On 03/14/2014 04:43 AM, Jiri Denemark wrote:
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/Makefile.am b/src/Makefile.am index d4d7b2b..fce9056 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -16,6 +16,10 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>.
+# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd)
Luckily we require GNU make, so this works. It would be nice, however, if src/Makefile.am and tests/Makefile.am shared the same formulas; right now, tests/Makefile.am uses the more portable (but slower): AM_CFLAGS = ... -Dabs_builddir="\"`pwd`\"" \ -Dabs_srcdir="\"`cd '$(srcdir)'; pwd`\"" \ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Mar 14, 2014 at 07:54:58 -0600, Eric Blake wrote:
On 03/14/2014 04:43 AM, Jiri Denemark wrote:
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/Makefile.am b/src/Makefile.am index d4d7b2b..fce9056 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -16,6 +16,10 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>.
+# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd)
Luckily we require GNU make, so this works. It would be nice, however, if src/Makefile.am and tests/Makefile.am shared the same formulas; right now, tests/Makefile.am uses the more portable (but slower):
AM_CFLAGS = ... -Dabs_builddir="\"`pwd`\"" \ -Dabs_srcdir="\"`cd '$(srcdir)'; pwd`\"" \
Which is unusable in this case, because we don't need to pass the variables to the compiler. It's make itself that needs to consume the variables: $(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@ Jirka
On 03/14/2014 12:55 PM, Jiri Denemark wrote:
On Fri, Mar 14, 2014 at 07:54:58 -0600, Eric Blake wrote:
On 03/14/2014 04:43 AM, Jiri Denemark wrote:
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
+# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd)
Hmm, just noticed another thing - with NEWER automake, we are now less efficient (calling out to $(shell) overwrites the value that is already provided for free without a subprocess by newer automake). Does it work if you use: abs_builddir ?= $(shell pwd)
Luckily we require GNU make, so this works. It would be nice, however, if src/Makefile.am and tests/Makefile.am shared the same formulas; right now, tests/Makefile.am uses the more portable (but slower):
AM_CFLAGS = ... -Dabs_builddir="\"`pwd`\"" \ -Dabs_srcdir="\"`cd '$(srcdir)'; pwd`\"" \
Which is unusable in this case, because we don't need to pass the variables to the compiler. It's make itself that needs to consume the variables:
$(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@
The $(abs_srcdir) could be inlined with ``, but you are right that the use of $(abs_builddir) has to be expanded up front for make to track the right file in its dependency. So ACK to this patch if the switch to ?= instead of = works. It still might be nice to update tests/Makefile.am in a followup to use $(shell) instead of `` (since $(shell) is more efficient), but that's not for this patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Mar 17, 2014 at 09:33:11 -0600, Eric Blake wrote:
On 03/14/2014 12:55 PM, Jiri Denemark wrote:
On Fri, Mar 14, 2014 at 07:54:58 -0600, Eric Blake wrote:
On 03/14/2014 04:43 AM, Jiri Denemark wrote:
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
+# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd)
Hmm, just noticed another thing - with NEWER automake, we are now less efficient (calling out to $(shell) overwrites the value that is already provided for free without a subprocess by newer automake). Does it work if you use:
abs_builddir ?= $(shell pwd)
Automake does a fair amount of magic here so abs_builddir = $(shel ...) does not actually overwrite the old value because there is no old value :-) Automake just does not put it's own abs_builddir definition in that case. If I switch to abs_builddir ?= $(shell ...), automake adds it's own definition so it seems it could work except that it doesn't. With "=", automake adds the definition above the other variable definition and namely above libvirt_cpu_la_DEPENDENCIES which uses it: abs_builddir = $(shell ...) ... libvirt_cpu_la_DEPENDENCIES = $(abs_builddir)/... ... rules When I switch to "?=", automake apparently does not recognize it as a variable definition and puts it between the block with variable definitions and rules. Thus the result is: abs_builddir = /some/build/path ... libvirt_cpu_la_DEPENDENCIES = $(abs_builddir)/... ... abs_builddir ?= $(shell ...) ... rules which of course does not work with old automake which does not add the first abs_builddir definition there and thus libvirt_cpu_la_DEPENDENCIES sees $(abs_builddir) empty. Anyway, making two extra shell commands once per build does not seem like anything we should really care about so I'd just stick with abs_builddir = $(shell ...)
The $(abs_srcdir) could be inlined with ``, but you are right that the use of $(abs_builddir) has to be expanded up front for make to track the right file in its dependency. So ACK to this patch if the switch to ?= instead of = works.
It doesn't. Jirka
On Tue, Mar 18, 2014 at 11:36:51AM +0100, Jiri Denemark wrote:
On Mon, Mar 17, 2014 at 09:33:11 -0600, Eric Blake wrote:
On 03/14/2014 12:55 PM, Jiri Denemark wrote:
On Fri, Mar 14, 2014 at 07:54:58 -0600, Eric Blake wrote:
On 03/14/2014 04:43 AM, Jiri Denemark wrote:
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
+# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd)
Hmm, just noticed another thing - with NEWER automake, we are now less efficient (calling out to $(shell) overwrites the value that is already provided for free without a subprocess by newer automake). Does it work if you use:
abs_builddir ?= $(shell pwd)
Automake does a fair amount of magic here so abs_builddir = $(shel ...) does not actually overwrite the old value because there is no old value :-) Automake just does not put it's own abs_builddir definition in that case. If I switch to abs_builddir ?= $(shell ...), automake adds it's own definition so it seems it could work except that it doesn't. With "=", automake adds the definition above the other variable definition and namely above libvirt_cpu_la_DEPENDENCIES which uses it:
abs_builddir = $(shell ...) ... libvirt_cpu_la_DEPENDENCIES = $(abs_builddir)/... ... rules
When I switch to "?=", automake apparently does not recognize it as a variable definition and puts it between the block with variable definitions and rules. Thus the result is:
abs_builddir = /some/build/path ... libvirt_cpu_la_DEPENDENCIES = $(abs_builddir)/... ... abs_builddir ?= $(shell ...) ... rules
which of course does not work with old automake which does not add the first abs_builddir definition there and thus libvirt_cpu_la_DEPENDENCIES sees $(abs_builddir) empty.
Anyway, making two extra shell commands once per build does not seem like anything we should really care about so I'd just stick with
abs_builddir = $(shell ...)
Why don't we just avoid the whole issue by removing use of abs_srcdir and abs_builddir. Can this rule: $(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@ be just changed to cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(srcdir)/cpu/cpu_map.xml $@ 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 Tue, Mar 18, 2014 at 10:42:01 +0000, Daniel Berrange wrote:
On Tue, Mar 18, 2014 at 11:36:51AM +0100, Jiri Denemark wrote:
On Mon, Mar 17, 2014 at 09:33:11 -0600, Eric Blake wrote:
On 03/14/2014 12:55 PM, Jiri Denemark wrote:
On Fri, Mar 14, 2014 at 07:54:58 -0600, Eric Blake wrote:
On 03/14/2014 04:43 AM, Jiri Denemark wrote:
Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
+# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd)
Hmm, just noticed another thing - with NEWER automake, we are now less efficient (calling out to $(shell) overwrites the value that is already provided for free without a subprocess by newer automake). Does it work if you use:
abs_builddir ?= $(shell pwd)
Automake does a fair amount of magic here so abs_builddir = $(shel ...) does not actually overwrite the old value because there is no old value :-) Automake just does not put it's own abs_builddir definition in that case. If I switch to abs_builddir ?= $(shell ...), automake adds it's own definition so it seems it could work except that it doesn't. With "=", automake adds the definition above the other variable definition and namely above libvirt_cpu_la_DEPENDENCIES which uses it:
abs_builddir = $(shell ...) ... libvirt_cpu_la_DEPENDENCIES = $(abs_builddir)/... ... rules
When I switch to "?=", automake apparently does not recognize it as a variable definition and puts it between the block with variable definitions and rules. Thus the result is:
abs_builddir = /some/build/path ... libvirt_cpu_la_DEPENDENCIES = $(abs_builddir)/... ... abs_builddir ?= $(shell ...) ... rules
which of course does not work with old automake which does not add the first abs_builddir definition there and thus libvirt_cpu_la_DEPENDENCIES sees $(abs_builddir) empty.
Anyway, making two extra shell commands once per build does not seem like anything we should really care about so I'd just stick with
abs_builddir = $(shell ...)
Why don't we just avoid the whole issue by removing use of abs_srcdir and abs_builddir. Can this rule:
$(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@
be just changed to
cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(srcdir)/cpu/cpu_map.xml $@
That's what I tried first but it does not work at all. I don't understand why but make thinks cpu/cpu_map.xml target is uptodate even though the file does not exist in builddir. And $(srcdir) doesn't work for relative VPATH. For example, if VPATH is .., then abs_srcdir = /some/path/src abs_builddir = /some/path/build/src srcdir = ../../src and the /some/path/build/src/cpu/cpu_map.xml link will be ../../src/cpu/cpu_map.xml and thus will point to itself. And we can't just blindly do ln -s ../$(srcdir)/cpu/cpu_map.xml $@ because this would not work for absolute VPATH, when srcdir is /some/path/src. Yeah, it's a mess. Jirka
On 03/18/2014 05:00 AM, Jiri Denemark wrote:
Why don't we just avoid the whole issue by removing use of abs_srcdir and abs_builddir. Can this rule:
$(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@
be just changed to
cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(srcdir)/cpu/cpu_map.xml $@
That's what I tried first but it does not work at all. I don't understand why but make thinks cpu/cpu_map.xml target is uptodate even though the file does not exist in builddir.
That would be VPATH rewriting at play. Does $(builddir)/cpu/cpu_map.xml fare any better?
And $(srcdir) doesn't work for relative VPATH. For example, if VPATH is .., then
abs_srcdir = /some/path/src abs_builddir = /some/path/build/src srcdir = ../../src
and the /some/path/build/src/cpu/cpu_map.xml link will be ../../src/cpu/cpu_map.xml and thus will point to itself.
$(srcdir) is not the problem - that part of the rule can be written absolute as follows: $(AM_V_GEN)ln -s `cd $(srcdir) && pwd`/cpu/cpu_map.xml $@ It's just the $(abs_builddir) we're trying to figure out if we can avoid.
And we can't just blindly do
ln -s ../$(srcdir)/cpu/cpu_map.xml $@
because this would not work for absolute VPATH, when srcdir is /some/path/src.
Yeah, it's a mess.
I looked at gnulib for inspiration, because it also does a symlink of GNUmakefile when doing a VPATH build while using the original file when doing in-tree. But it was depressing: it doesn't work for VPATH builds on RHEL 5: configure.ac: # Autoconf 2.61a.99 and earlier don't support linking a file only # in VPATH builds. But since GNUmakefile is for maintainer use # only, it does not matter if we skip the link with older autoconf. # Automake 1.10.1 and earlier try to remove GNUmakefile in non-VPATH # builds, so use a shell variable to bypass this. GNUmakefile=GNUmakefile m4_if(m4_version_compare([2.61a.100], m4_defn([m4_PACKAGE_VERSION])), [1], [], [AC_CONFIG_LINKS([$GNUmakefile:$GNUmakefile], [], [GNUmakefile=$GNUmakefile])]) Makefile.am: distclean-local: clean-GNUmakefile clean-GNUmakefile: test '$(srcdir)' = . || rm -f $(top_builddir)/GNUmakefile But maybe that's some food for thought - instead of having a rule that uses a direct file name, perhaps you can instead have a witness rule on a stamp file name, where we write the link ourselves when needed, and then have all dependencies be on the stamp (which will ALWAYS exist only in builddir): cpu/cpu_map.xml.stamp: $(AM_V_GEN)if test -f cpu/cpu_map.xml; then \ :; else \ ln -s `cd $(srcdir) && pwd`/cpu/cpu_map.xml \ cpu/cpu_map.xml; \ fi && touch $@ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Mar 18, 2014 at 06:27:48 -0600, Eric Blake wrote:
On 03/18/2014 05:00 AM, Jiri Denemark wrote:
Why don't we just avoid the whole issue by removing use of abs_srcdir and abs_builddir. Can this rule:
$(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@
be just changed to
cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(srcdir)/cpu/cpu_map.xml $@
That's what I tried first but it does not work at all. I don't understand why but make thinks cpu/cpu_map.xml target is uptodate even though the file does not exist in builddir.
That would be VPATH rewriting at play. Does $(builddir)/cpu/cpu_map.xml fare any better?
No, that doesn't work either. ...
But maybe that's some food for thought - instead of having a rule that uses a direct file name, perhaps you can instead have a witness rule on a stamp file name, where we write the link ourselves when needed, and then have all dependencies be on the stamp (which will ALWAYS exist only in builddir):
cpu/cpu_map.xml.stamp: $(AM_V_GEN)if test -f cpu/cpu_map.xml; then \ :; else \ ln -s `cd $(srcdir) && pwd`/cpu/cpu_map.xml \ cpu/cpu_map.xml; \ fi && touch $@
OK, this seems to work. It's uglier and doesn't regenerate the link if someone removes it (the *.stamp file would need to be removed too) but if that's considered a better way compared to setting abs_*dir, I can make a proper patch out of it after testing it in all situations. Jirka
On 03/18/2014 09:32 AM, Jiri Denemark wrote:
On Tue, Mar 18, 2014 at 06:27:48 -0600, Eric Blake wrote:
On 03/18/2014 05:00 AM, Jiri Denemark wrote:
Why don't we just avoid the whole issue by removing use of abs_srcdir and abs_builddir. Can this rule:
$(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@
be just changed to
cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(srcdir)/cpu/cpu_map.xml $@
That's what I tried first but it does not work at all. I don't understand why but make thinks cpu/cpu_map.xml target is uptodate even though the file does not exist in builddir.
That would be VPATH rewriting at play. Does $(builddir)/cpu/cpu_map.xml fare any better?
No, that doesn't work either.
...
But maybe that's some food for thought - instead of having a rule that uses a direct file name, perhaps you can instead have a witness rule on a stamp file name, where we write the link ourselves when needed, and then have all dependencies be on the stamp (which will ALWAYS exist only in builddir):
cpu/cpu_map.xml.stamp: $(AM_V_GEN)if test -f cpu/cpu_map.xml; then \ :; else \ ln -s `cd $(srcdir) && pwd`/cpu/cpu_map.xml \ cpu/cpu_map.xml; \ fi && touch $@
OK, this seems to work. It's uglier and doesn't regenerate the link if someone removes it (the *.stamp file would need to be removed too) but if that's considered a better way compared to setting abs_*dir, I can make a proper patch out of it after testing it in all situations.
Everything we've tried is ugly. At this point, relying on the GNU make extension of $(shell), and using = instead of ?=, is probably the least bad solution. So feel free to keep testing the .stamp alternative if you want, but I'm okay if you check in what you've already tested instead of sinking more time into it. Oh, one other thing I thought of: Why not just name the git version cpu_map.xml.in, and use autoconf's normal mechanisms to always create the non-.in version in builddir. True, there's no @foo@ sequences being replaced, but that way, we don't have to worry about $(abs_srcdir) in the makefile at all; we also don't have to mess with a .stamp. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Mar 18, 2014 at 09:44:43 -0600, Eric Blake wrote:
On 03/18/2014 09:32 AM, Jiri Denemark wrote:
On Tue, Mar 18, 2014 at 06:27:48 -0600, Eric Blake wrote:
On 03/18/2014 05:00 AM, Jiri Denemark wrote:
Why don't we just avoid the whole issue by removing use of abs_srcdir and abs_builddir. Can this rule:
$(abs_builddir)/cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(abs_srcdir)/cpu/cpu_map.xml $@
be just changed to
cpu/cpu_map.xml: $(AM_V_GEN)ln -s $(srcdir)/cpu/cpu_map.xml $@
That's what I tried first but it does not work at all. I don't understand why but make thinks cpu/cpu_map.xml target is uptodate even though the file does not exist in builddir.
That would be VPATH rewriting at play. Does $(builddir)/cpu/cpu_map.xml fare any better?
No, that doesn't work either.
...
But maybe that's some food for thought - instead of having a rule that uses a direct file name, perhaps you can instead have a witness rule on a stamp file name, where we write the link ourselves when needed, and then have all dependencies be on the stamp (which will ALWAYS exist only in builddir):
cpu/cpu_map.xml.stamp: $(AM_V_GEN)if test -f cpu/cpu_map.xml; then \ :; else \ ln -s `cd $(srcdir) && pwd`/cpu/cpu_map.xml \ cpu/cpu_map.xml; \ fi && touch $@
OK, this seems to work. It's uglier and doesn't regenerate the link if someone removes it (the *.stamp file would need to be removed too) but if that's considered a better way compared to setting abs_*dir, I can make a proper patch out of it after testing it in all situations.
Everything we've tried is ugly. At this point, relying on the GNU make extension of $(shell), and using = instead of ?=, is probably the least bad solution. So feel free to keep testing the .stamp alternative if you want, but I'm okay if you check in what you've already tested instead of sinking more time into it.
Oh, one other thing I thought of: Why not just name the git version cpu_map.xml.in, and use autoconf's normal mechanisms to always create the non-.in version in builddir. True, there's no @foo@ sequences being replaced, but that way, we don't have to worry about $(abs_srcdir) in the makefile at all; we also don't have to mess with a .stamp.
OK, I went ahead and pushed the minimal solution, i.e., setting abs_{src,build}dir = $(shell ...) After all, having the variables set may save us from similar build issues should any of the abs variables be useful for other rules. Jirka
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark -
Pavel Hrdina