Re: [libvirt] [PATCH] build: Fix build with old automake

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(a)redhat.com&gt; > >>> --- > > >>> +# 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 ...)