[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]