Re: [libvirt] [PATCH 1/4] build: avoid $(srcdir) in *_SOURCES
On 09/10/2013 03:58 AM, Michal Privoznik wrote:
> Some things to remember that affect VPATH builds, and where an
> in-tree build is blissfully unaware of the issues: if a VPATH
> build fails to find a file that was used as a prereq of any
> other target, then the rule for that file will expand $@ to
> prefer the current build dir (bad because a VPATH build on a
> fresh checkout will then stick $@ in the current directory
> instead of the desired srcdir); conversely, if a VPATH build
> finds the file in srcdir but decides it needs to be rebuilt,
> then the rule for that file will expand $@ to include the
> directory where it was found out-of-date (bad for an explicit
> listing of $(srcdir)/$@ because an incremental VPATH build will
> then expand srcdir twice). As we want these files to go into
> srcdir unconditionally, we have to massage or avoid $@ for any
> recipe that involves one of these files.
>
> -$(srcdir)/remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl
\
> +remote_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \
> $(REMOTE_PROTOCOL)
> $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \
> - --mode=server remote REMOTE $(REMOTE_PROTOCOL) > $@
> + --mode=server remote REMOTE $(REMOTE_PROTOCOL) \
> + > $(srcdir)/remote_dispatch.h
The other option is to have this line as:
> $(srcdir)/$@
No, that explicitly does not work in VPATH setups, as mentioned in my
commit message. On the first run (after a fresh checkout), $@ sees that
remote_dispatch.h does not exist, so $@ expands to 'remote_dispatch.h',
and the recipe creates $(srcdir)/remote_dispatch.h. But in an
incremental rebuild, after changing a prerequisite file (such as the .x
file embedded in $(REMOTE_PROTOCOL)), $@ discovers that the target file
already exists in $(srcdir) via the VPATH lookup, so it expands $@ to
'$(srcdir)/remote_dispatch.h', and the recipe attempts to build
'$(srcdir)/$(srcdir)/remote_dispatch.h'. If $(srcdir) is .., then you
have a mess on your hands (creating the wrong file, and the right file
didn't get updated). Hence, the patch HAS to use absolute naming,
rather than $@ magic, for anything that we _always_ want placed into srcdir.
(For an in-tree build, $(srcdir) is '.', and since you can't tell
between './remote_dispatch.h' vs. '././remote_dispatch.h', the problem
only affects VPATH builds.)
But this version is okay as is. ACK.
Thanks for reviewing the series; I'll push shortly.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)