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)

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.

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(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 ...) 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 $@

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?

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 $@

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.