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

2016-09-09 Thread Jan Beulich
>>> On 09.09.16 at 18:05,  wrote:
> On 09/09/2016 11:20 AM, Jan Beulich wrote:
> On 09.09.16 at 15:56,  wrote:
>>> On 09/09/2016 09:29 AM, Jan Beulich wrote:
>>> On 09.09.16 at 15:07,  wrote:
> On 09/09/2016 04:03 AM, Jan Beulich wrote:
> On 08.09.16 at 20:51,  wrote:
>>> On 09/08/2016 10:15 AM, Jan Beulich wrote:
>>> On 07.09.16 at 20:59,  wrote:
>  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_X 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.
> No, not a leading one. Inside a name:
>
> Expected:
>
> [root@ovs104 libacpi]# mkdir -p /tmp/root/xen.git
> [root@ovs104 libacpi]# iasl -vs -p /tmp/root/xen.git/dsdt_anycpu -tc
 Then my suggestion of using relative paths would still help?
>>> Apparently it doesn't like any dots:
>>>
>>> [root@ovs104 /]# mkdir -p /tmp/root/foo
>>> [root@ovs104 /]# mkdir -p /tmp/root/bar
>>> [root@ovs104 /]# cd /tmp/root/foo/
>>> [root@ovs104 foo]# ls -aR /tmp/root/
>>> /tmp/root/:
>>> .  ..  bar  foo
>>>
>>> /tmp/root/bar:
>>> .  ..
>>>
>>> /tmp/root/foo:
>>> .  ..
>>> [root@ovs104 foo]# ~/iasl.f12 -vs -p "../bar/dsdt_anycpu" -tc
>> Why would you need to use ../ ?
> 
> 
> Didn't you ask to test with a relative path?

Yes, and I gave an example of what I meant: dir/name.ext; no
./ or ../ or alike.

Jan


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


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

2016-09-09 Thread Boris Ostrovsky
On 09/09/2016 11:20 AM, Jan Beulich wrote:
 On 09.09.16 at 15:56,  wrote:
>> On 09/09/2016 09:29 AM, Jan Beulich wrote:
>> On 09.09.16 at 15:07,  wrote:
 On 09/09/2016 04:03 AM, Jan Beulich wrote:
 On 08.09.16 at 20:51,  wrote:
>> On 09/08/2016 10:15 AM, Jan Beulich wrote:
>> On 07.09.16 at 20:59,  wrote:
  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_X 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.
 No, not a leading one. Inside a name:

 Expected:

 [root@ovs104 libacpi]# mkdir -p /tmp/root/xen.git
 [root@ovs104 libacpi]# iasl -vs -p /tmp/root/xen.git/dsdt_anycpu -tc
>>> Then my suggestion of using relative paths would still help?
>> Apparently it doesn't like any dots:
>>
>> [root@ovs104 /]# mkdir -p /tmp/root/foo
>> [root@ovs104 /]# mkdir -p /tmp/root/bar
>> [root@ovs104 /]# cd /tmp/root/foo/
>> [root@ovs104 foo]# ls -aR /tmp/root/
>> /tmp/root/:
>> .  ..  bar  foo
>>
>> /tmp/root/bar:
>> .  ..
>>
>> /tmp/root/foo:
>> .  ..
>> [root@ovs104 foo]# ~/iasl.f12 -vs -p "../bar/dsdt_anycpu" -tc
> Why would you need to use ../ ?


Didn't you ask to test with a relative path?

(Not this this is needed anymore, now that Ian suggested a solution).

-boris

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


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

2016-09-09 Thread Jan Beulich
>>> On 09.09.16 at 15:56,  wrote:
> On 09/09/2016 09:29 AM, Jan Beulich wrote:
> On 09.09.16 at 15:07,  wrote:
>>> On 09/09/2016 04:03 AM, Jan Beulich wrote:
>>> On 08.09.16 at 20:51,  wrote:
> On 09/08/2016 10:15 AM, Jan Beulich wrote:
> On 07.09.16 at 20:59,  wrote:
>>>  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_X 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.
>>> No, not a leading one. Inside a name:
>>>
>>> Expected:
>>>
>>> [root@ovs104 libacpi]# mkdir -p /tmp/root/xen.git
>>> [root@ovs104 libacpi]# iasl -vs -p /tmp/root/xen.git/dsdt_anycpu -tc
>> Then my suggestion of using relative paths would still help?
> 
> Apparently it doesn't like any dots:
> 
> [root@ovs104 /]# mkdir -p /tmp/root/foo
> [root@ovs104 /]# mkdir -p /tmp/root/bar
> [root@ovs104 /]# cd /tmp/root/foo/
> [root@ovs104 foo]# ls -aR /tmp/root/
> /tmp/root/:
> .  ..  bar  foo
> 
> /tmp/root/bar:
> .  ..
> 
> /tmp/root/foo:
> .  ..
> [root@ovs104 foo]# ~/iasl.f12 -vs -p "../bar/dsdt_anycpu" -tc

Why would you need to use ../ ?

Jan


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


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

2016-09-09 Thread Boris Ostrovsky
On 09/09/2016 10:33 AM, Ian Jackson wrote:
> Boris Ostrovsky writes ("Re: [PATCH v3 13/19] acpi: Makefile should better 
> tolerate interrupts"):
>> In fact, I can just append a dot to the name:
> I think it would be less weird to append a dot and then an actual
> extension with some letters or something.  I'm not sure what the
> extension ought to be but just a dot is going to be weird and
> confusing.

Yes, I will append some sort of a suffix and add a comment explaining
why it is needed.

-boris

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


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

2016-09-09 Thread Ian Jackson
Boris Ostrovsky writes ("Re: [PATCH v3 13/19] acpi: Makefile should better 
tolerate interrupts"):
> In fact, I can just append a dot to the name:

I think it would be less weird to append a dot and then an actual
extension with some letters or something.  I'm not sure what the
extension ought to be but just a dot is going to be weird and
confusing.

Ian.

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


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

2016-09-09 Thread Boris Ostrovsky
On 09/09/2016 10:13 AM, Ian Jackson wrote:
> Boris Ostrovsky writes ("Re: [PATCH v3 13/19] acpi: Makefile should better 
> tolerate interrupts"):
>> On 09/09/2016 09:29 AM, Jan Beulich wrote:
>>> Then my suggestion of using relative paths would still help?
>> Apparently it doesn't like any dots:
> ...
>> [root@ovs104 foo]# ~/iasl.f12 -vs -p "../bar/dsdt_anycpu" -tc
>> /tmp/dsdt_anycpu.asl &>/dev/null
>> [root@ovs104 foo]# ls -aR /tmp/root/
>> /tmp/root/:
>> .  ..  bar  foo
>>
>> /tmp/root/bar:
>> .  ..
>>
>> /tmp/root/foo:
>> .  ..  ..aml  ..hex
>> [root@ovs104 foo]#
> This seems to suggest that it looks for the last dot, not the first
> one.
>
> If you provide a filename which has an "extension", as it seems to
> expect, does it DTRT ?

Indeed!

In fact, I can just append a dot to the name:

[root@ovs104 tmp]# mkdir -p /tmp/root/xen.git
[root@ovs104 tmp]# ~/iasl.f12 -vs -p /tmp/root/xen.git/dsdt_anycpu -tc
/tmp/dsdt_anycpu.asl &>/dev/null
[root@ovs104 tmp]# ls -aR /tmp/root/
/tmp/root/:
.  ..  xen.aml  xen.git  xen.hex

/tmp/root/xen.git:
.  ..
[root@ovs104 tmp]# ~/iasl.f12 -vs -p /tmp/root/xen.git/dsdt_anycpu. -tc
/tmp/dsdt_anycpu.asl &>/dev/null
[root@ovs104 tmp]# ls -aR /tmp/root/
/tmp/root/:
.  ..  xen.aml  xen.git  xen.hex

/tmp/root/xen.git:
.  ..  dsdt_anycpu.aml  dsdt_anycpu.hex
[root@ovs104 tmp]#


With that I can go back to my original version where I don't need to
have a temp directory at all.

Thanks Ian.

-boris

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


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

2016-09-09 Thread Ian Jackson
Boris Ostrovsky writes ("Re: [PATCH v3 13/19] acpi: Makefile should better 
tolerate interrupts"):
> On 09/09/2016 09:29 AM, Jan Beulich wrote:
> > Then my suggestion of using relative paths would still help?
> 
> Apparently it doesn't like any dots:
...
> [root@ovs104 foo]# ~/iasl.f12 -vs -p "../bar/dsdt_anycpu" -tc
> /tmp/dsdt_anycpu.asl &>/dev/null
> [root@ovs104 foo]# ls -aR /tmp/root/
> /tmp/root/:
> .  ..  bar  foo
> 
> /tmp/root/bar:
> .  ..
> 
> /tmp/root/foo:
> .  ..  ..aml  ..hex
> [root@ovs104 foo]#

This seems to suggest that it looks for the last dot, not the first
one.

If you provide a filename which has an "extension", as it seems to
expect, does it DTRT ?

Ian.

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


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

2016-09-09 Thread Boris Ostrovsky
On 09/09/2016 09:29 AM, Jan Beulich wrote:
 On 09.09.16 at 15:07,  wrote:
>> On 09/09/2016 04:03 AM, Jan Beulich wrote:
>> On 08.09.16 at 20:51,  wrote:
 On 09/08/2016 10:15 AM, Jan Beulich wrote:
 On 07.09.16 at 20:59,  wrote:
>>  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_X 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.
>> No, not a leading one. Inside a name:
>>
>> Expected:
>>
>> [root@ovs104 libacpi]# mkdir -p /tmp/root/xen.git
>> [root@ovs104 libacpi]# iasl -vs -p /tmp/root/xen.git/dsdt_anycpu -tc
> Then my suggestion of using relative paths would still help?

Apparently it doesn't like any dots:

[root@ovs104 /]# mkdir -p /tmp/root/foo
[root@ovs104 /]# mkdir -p /tmp/root/bar
[root@ovs104 /]# cd /tmp/root/foo/
[root@ovs104 foo]# ls -aR /tmp/root/
/tmp/root/:
.  ..  bar  foo

/tmp/root/bar:
.  ..

/tmp/root/foo:
.  ..
[root@ovs104 foo]# ~/iasl.f12 -vs -p "../bar/dsdt_anycpu" -tc
/tmp/dsdt_anycpu.asl &>/dev/null
[root@ovs104 foo]# ls -aR /tmp/root/
/tmp/root/:
.  ..  bar  foo

/tmp/root/bar:
.  ..

/tmp/root/foo:
.  ..  ..aml  ..hex
[root@ovs104 foo]#


-boris

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


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

2016-09-09 Thread Jan Beulich
>>> On 09.09.16 at 15:07,  wrote:
> On 09/09/2016 04:03 AM, Jan Beulich wrote:
> On 08.09.16 at 20:51,  wrote:
>>> On 09/08/2016 10:15 AM, Jan Beulich wrote:
>>> On 07.09.16 at 20:59,  wrote:
>  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_X 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.
> 
> No, not a leading one. Inside a name:
> 
> Expected:
> 
> [root@ovs104 libacpi]# mkdir -p /tmp/root/xen.git
> [root@ovs104 libacpi]# iasl -vs -p /tmp/root/xen.git/dsdt_anycpu -tc

Then my suggestion of using relative paths would still help?

>>  (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.)
> 
> That, in fact, is what I initially had --- I just added an extra temp
> suffix to intermediate files during a build. And right before sending
> the series I figured I'd give it a spin on our build server. Big mistake!

Big mistake? You leave me curious, but unable guess.

Jan


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


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

2016-09-09 Thread Boris Ostrovsky
On 09/09/2016 04:03 AM, Jan Beulich wrote:
 On 08.09.16 at 20:51,  wrote:
>> On 09/08/2016 10:15 AM, Jan Beulich wrote:
>> On 07.09.16 at 20:59,  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_XX)
 +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.)

Yes.

I tried to figure out a way for Makefile to always execute a rule when
before it exits but didn't find anything. You can do it as a *first*
rule (with "-include ") but not as a final one.

My only defense here (again, pretty weak) is that making libacpi 'all'
target is pretty fast.

>
  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_X 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.

No, not a leading one. Inside a name:

Expected:

[root@ovs104 libacpi]# mkdir -p /tmp/root/xen.git
[root@ovs104 libacpi]# iasl -vs -p /tmp/root/xen.git/dsdt_anycpu -tc
/tmp/dsdt_anycpu.asl &>/dev/null
[root@ovs104 libacpi]# ls -R /tmp/root/
/tmp/root/:
xen.git

/tmp/root/xen.git:
dsdt_anycpu.aml  dsdt_anycpu.hex
[root@ovs104 libacpi]#


Fedora 12:

[root@ovs104 libacpi]# mkdir -p /tmp/root/xen.git
[root@ovs104 libacpi]# ~/iasl.f12 -vs -p /tmp/root/xen.git/dsdt_anycpu
-tc /tmp/dsdt_anycpu.asl &>/dev/null
[root@ovs104 libacpi]# ls -R /tmp/root/
/tmp/root/:
xen.aml  xen.git  xen.hex

/tmp/root/xen.git:
[root@ovs104 libacpi]#

I tried escaping the dot, putting path in quotes, etc. but without any
success.

>
> Also this part of your answer doesn't address the cleanup aspect at
> all.

Again, not the greatest way to deal with this, but we will leave temp
directory in /tmp on interrupts. (And on error I could have added an
error handling target but decided not to do that so that it's easier to
debug the error).


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

That, in fact, is what I initially had --- I just added an extra temp
suffix to intermediate files during a build. And right before sending
the series I figured I'd give it a spin on our build server. Big mistake!

So I agree that this is not the best solution but I haven't come up with
anything better.

-boris


>
>> 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@lists.xen.org
https://lists.xen.org/xen-devel


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

2016-09-09 Thread Jan Beulich
>>> On 08.09.16 at 20:51,  wrote:
> On 09/08/2016 10:15 AM, Jan Beulich wrote:
> On 07.09.16 at 20:59,  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_XX)
>>> +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_X 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@lists.xen.org
https://lists.xen.org/xen-devel


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

2016-09-08 Thread Boris Ostrovsky
On 09/08/2016 10:15 AM, Jan Beulich wrote:
 On 07.09.16 at 20:59,  wrote:
>> Intermediate stages of building a target should be made with
>> temporary files that are copied to final target in the end.
>>
>> Signed-off-by: Boris Ostrovsky 
>> ---
>> New in v3
> Ah, here we go.
>
>> --- 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_XX)
>> +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.


>
>>  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_X 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.

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.

-boris




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


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

2016-09-08 Thread Jan Beulich
>>> On 07.09.16 at 20:59,  wrote:
> Intermediate stages of building a target should be made with
> temporary files that are copied to final target in the end.
> 
> Signed-off-by: Boris Ostrovsky 
> ---
> New in v3

Ah, here we go.

> --- 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_XX)
> +endif

How is this (or really the rules using this directory) supposed to work
when other than "all" gets built?

>  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.

Jan


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