Re: [Xen-devel] [PATCH v3 13/19] acpi: Makefile should better tolerate interrupts

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.09.16 at 20:51, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 09/08/2016 10:15 AM, Jan Beulich wrote:
>>>>> On 07.09.16 at 20:59, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> --- a/tools/libacpi/Makefile
>>> +++ b/tools/libacpi/Makefile
>>> @@ -21,38 +21,45 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>>>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c  
> dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
>>>  H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h 
> ssdt_tpm.h)
>>>  
>>> +ifeq ($(subst all,,$(MAKECMDGOALS)),)
>>> +TDIR := $(shell mktemp -d --tmpdir=$(TMPDIR) tmp_XXXXXX)
>>> +endif
>> How is this (or really the rules using this directory) supposed to work
>> when other than "all" gets built?
> 
> 
> I realize it's a somewhat weak argument but only 'all' and 'clean'
> targets are supposed to be used. In fact I was thinking about explicitly
> making a check for targets.

Hmm, that would be quite limiting in case one wants to analyze a
problem with just a single build step. (On the hypervisor side I,
every once in a while, find it helpful to be able to compile individual
files into .o, or even produce the intermediate .i or .s.)

>>>  vpath iasl $(PATH)
>>>  all: $(C_SRC) $(H_SRC)
>>> +   rm -fr $(TDIR)
>> And how is the temporary directory going to get cleaned up when
>> interrupting make? I think you really should use a subdirectory
>> underneath the build directory, which then can stay there until
>> "make clean". And then you can also use mv instead of cp below,
>> or even move-if-changed.
> 
> The reason I am doing this in /tmp and use tmp_XXXXX as template is
> because I found that at least one old versions of iasl has a bug where
> it can't process path that has a '.' in it. It drops anything after the
> dot, presumably because it thinks it's file suffix.

That . is a leading one, as in ./path/file.ext? If so, why can't this be
made path/file.ext? The leading ./ shouldn't be necessary after all.

Also this part of your answer doesn't address the cleanup aspect at
all. (And btw., thinking about it again I don't see the need for a
subdirectory. We have ample room to name the intermediate files
suitably without causing any name collision.)

> This is true on fedora12 (2009), which is what we still use as build
> environment. I know that fedora 18 doesn't have this problem but I don't
> know at which point this got fixed. (Interestingly enough, I tried to
> build from F12's sources for iasl and could not reproduce this. Now, i
> built with new tools so perhaps the bug is not in iasl itself but in
> something like yacc, which is used for the build).
> 
> I don't think we have any requirement on supported iasl version so I am
> not sure we can ignore this error.

I agree.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel