[PATCH v1] libacpi: use temporary files for generated files

[PATCH v1] libacpi: use temporary files for generated files

[Olaf Hering]

Use a temporay file, and move it in place once done.
The same pattern exists already for other dependencies.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/libacpi/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
index c17f3924cc..2cc4cc585b 100644
--- a/tools/libacpi/Makefile
+++ b/tools/libacpi/Makefile
@@ -43,7 +43,8 @@ all: $(C_SRC) $(H_SRC)
 
 $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
 	iasl -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $<
-	sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@
+	sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@.$(TMP_SUFFIX)
+	mv -f $@.$(TMP_SUFFIX) $@
 	rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex)
  
 $(MK_DSDT): mk_dsdt.c

Re: [PATCH v1] libacpi: use temporary files for generated files

[Jan Beulich]

On 26.10.2020 21:41, Olaf Hering wrote:
> Use a temporay file, and move it in place once done.
> The same pattern exists already for other dependencies.

This pattern is used when a rule consists of multiple commands
having their output appended to one another's. This isn't the
case here, so I'm afraid I'd like it to be made more obvious
what the gain / improvement is. That is - I don't mind the
change, if indeed it is for a good reason.

Jan

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/libacpi/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
> index c17f3924cc..2cc4cc585b 100644
> --- a/tools/libacpi/Makefile
> +++ b/tools/libacpi/Makefile
> @@ -43,7 +43,8 @@ all: $(C_SRC) $(H_SRC)
>  
>  $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
>  	iasl -vs -p $(ACPI_BUILD_DIR)/$*.$(TMP_SUFFIX) -tc $<
> -	sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@
> +	sed -e 's/AmlCode/$*/g' -e 's/_aml_code//g' $(ACPI_BUILD_DIR)/$*.hex >$@.$(TMP_SUFFIX)
> +	mv -f $@.$(TMP_SUFFIX) $@
>  	rm -f $(addprefix $(ACPI_BUILD_DIR)/, $*.aml $*.hex)
>   
>  $(MK_DSDT): mk_dsdt.c
> 

Re: [PATCH v1] libacpi: use temporary files for generated files

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

Am Tue, 27 Oct 2020 11:16:04 +0100
schrieb Jan Beulich <jbeulich@suse.com>:

> This pattern is used when a rule consists of multiple commands
> having their output appended to one another's.

My understanding is: a rule is satisfied as soon as the file exists. In this case once the invoked shell opens the file for writing. If this is the case, the change should be applied.

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] libacpi: use temporary files for generated files

[Jan Beulich]

On 27.10.2020 11:27, Olaf Hering wrote:
> Am Tue, 27 Oct 2020 11:16:04 +0100
> schrieb Jan Beulich <jbeulich@suse.com>:
> 
>> This pattern is used when a rule consists of multiple commands
>> having their output appended to one another's.
> 
> My understanding is: a rule is satisfied as soon as the file exists.

No - once make has found that a rule's commands need running, it'll
run the full set and only check again afterwards. If there was a
racing parallel make, things would be different, but such a race
would need addressing elsewhere: No two possibly racing threads of
the make process ought to be creating the same files. The creation
of the files needs synchronizing in such a case.

Jan

Re: [PATCH v1] libacpi: use temporary files for generated files

[Andrew Cooper]

On 27/10/2020 10:37, Jan Beulich wrote:
> On 27.10.2020 11:27, Olaf Hering wrote:
>> Am Tue, 27 Oct 2020 11:16:04 +0100
>> schrieb Jan Beulich <jbeulich@suse.com>:
>>
>>> This pattern is used when a rule consists of multiple commands
>>> having their output appended to one another's.
>> My understanding is: a rule is satisfied as soon as the file exists.
> No - once make has found that a rule's commands need running, it'll
> run the full set and only check again afterwards.

It stops at the first command which fails.

Olaf is correct, but the problem here is an incremental build issue, not
a parallel build issue.

Intermediate files must not use the name of the target, or a failure and
re-build will use the (bogus) intermediate state rather than rebuilding it.

~Andrew

Re: [PATCH v1] libacpi: use temporary files for generated files

[Jan Beulich]

On 27.10.2020 11:57, Andrew Cooper wrote:
> On 27/10/2020 10:37, Jan Beulich wrote:
>> On 27.10.2020 11:27, Olaf Hering wrote:
>>> Am Tue, 27 Oct 2020 11:16:04 +0100
>>> schrieb Jan Beulich <jbeulich@suse.com>:
>>>
>>>> This pattern is used when a rule consists of multiple commands
>>>> having their output appended to one another's.
>>> My understanding is: a rule is satisfied as soon as the file exists.
>> No - once make has found that a rule's commands need running, it'll
>> run the full set and only check again afterwards.
> 
> It stops at the first command which fails.
> 
> Olaf is correct, but the problem here is an incremental build issue, not
> a parallel build issue.
> 
> Intermediate files must not use the name of the target, or a failure and
> re-build will use the (bogus) intermediate state rather than rebuilding it.

But there's no intermediate file here - the file gets created in one
go. Furthermore doesn't make delete the target file(s) when a rule
fails? (One may not want to rely on this, and hence indeed keep multi-
part rules update intermediate files of different names.)

Jan

Re: [PATCH v1] libacpi: use temporary files for generated files

[Anthony PERARD]

On Tue, Oct 27, 2020 at 12:06:56PM +0100, Jan Beulich wrote:
> On 27.10.2020 11:57, Andrew Cooper wrote:
> > On 27/10/2020 10:37, Jan Beulich wrote:
> >> On 27.10.2020 11:27, Olaf Hering wrote:
> >>> Am Tue, 27 Oct 2020 11:16:04 +0100
> >>> schrieb Jan Beulich <jbeulich@suse.com>:
> >>>
> >>>> This pattern is used when a rule consists of multiple commands
> >>>> having their output appended to one another's.
> >>> My understanding is: a rule is satisfied as soon as the file exists.
> >> No - once make has found that a rule's commands need running, it'll
> >> run the full set and only check again afterwards.
> > 
> > It stops at the first command which fails.
> > 
> > Olaf is correct, but the problem here is an incremental build issue, not
> > a parallel build issue.
> > 
> > Intermediate files must not use the name of the target, or a failure and
> > re-build will use the (bogus) intermediate state rather than rebuilding it.
> 
> But there's no intermediate file here - the file gets created in one
> go. Furthermore doesn't make delete the target file(s) when a rule
> fails? (One may not want to rely on this, and hence indeed keep multi-
> part rules update intermediate files of different names.)

What if something went badly enough and sed didn't managed to write the
whole file and make never had a chance to remove the bogus file?

Surely, this patch is a strict improvement to the build system.

Cheers,

-- 
Anthony PERARD

Re: [PATCH v1] libacpi: use temporary files for generated files

[Jan Beulich]

On 28.10.2020 19:13, Anthony PERARD wrote:
> On Tue, Oct 27, 2020 at 12:06:56PM +0100, Jan Beulich wrote:
>> On 27.10.2020 11:57, Andrew Cooper wrote:
>>> On 27/10/2020 10:37, Jan Beulich wrote:
>>>> On 27.10.2020 11:27, Olaf Hering wrote:
>>>>> Am Tue, 27 Oct 2020 11:16:04 +0100
>>>>> schrieb Jan Beulich <jbeulich@suse.com>:
>>>>>
>>>>>> This pattern is used when a rule consists of multiple commands
>>>>>> having their output appended to one another's.
>>>>> My understanding is: a rule is satisfied as soon as the file exists.
>>>> No - once make has found that a rule's commands need running, it'll
>>>> run the full set and only check again afterwards.
>>>
>>> It stops at the first command which fails.
>>>
>>> Olaf is correct, but the problem here is an incremental build issue, not
>>> a parallel build issue.
>>>
>>> Intermediate files must not use the name of the target, or a failure and
>>> re-build will use the (bogus) intermediate state rather than rebuilding it.
>>
>> But there's no intermediate file here - the file gets created in one
>> go. Furthermore doesn't make delete the target file(s) when a rule
>> fails? (One may not want to rely on this, and hence indeed keep multi-
>> part rules update intermediate files of different names.)
> 
> What if something went badly enough and sed didn't managed to write the
> whole file and make never had a chance to remove the bogus file?

How's this different from an object file getting only partly written
and not deleted? We'd have to use the temporary file approach in
literally every rule if we wanted to cater for this.

Jan

Re: [PATCH v1] libacpi: use temporary files for generated files

[Anthony PERARD]

On Thu, Oct 29, 2020 at 09:47:09AM +0100, Jan Beulich wrote:
> On 28.10.2020 19:13, Anthony PERARD wrote:
> > On Tue, Oct 27, 2020 at 12:06:56PM +0100, Jan Beulich wrote:
> >> On 27.10.2020 11:57, Andrew Cooper wrote:
> >>> On 27/10/2020 10:37, Jan Beulich wrote:
> >>>> On 27.10.2020 11:27, Olaf Hering wrote:
> >>>>> Am Tue, 27 Oct 2020 11:16:04 +0100
> >>>>> schrieb Jan Beulich <jbeulich@suse.com>:
> >>>>>
> >>>>>> This pattern is used when a rule consists of multiple commands
> >>>>>> having their output appended to one another's.
> >>>>> My understanding is: a rule is satisfied as soon as the file exists.
> >>>> No - once make has found that a rule's commands need running, it'll
> >>>> run the full set and only check again afterwards.
> >>>
> >>> It stops at the first command which fails.
> >>>
> >>> Olaf is correct, but the problem here is an incremental build issue, not
> >>> a parallel build issue.
> >>>
> >>> Intermediate files must not use the name of the target, or a failure and
> >>> re-build will use the (bogus) intermediate state rather than rebuilding it.
> >>
> >> But there's no intermediate file here - the file gets created in one
> >> go. Furthermore doesn't make delete the target file(s) when a rule
> >> fails? (One may not want to rely on this, and hence indeed keep multi-
> >> part rules update intermediate files of different names.)
> > 
> > What if something went badly enough and sed didn't managed to write the
> > whole file and make never had a chance to remove the bogus file?
> 
> How's this different from an object file getting only partly written
> and not deleted? We'd have to use the temporary file approach in
> literally every rule if we wanted to cater for this.

I though that things like `gcc' would write the final object to a
temporary place then rename it to the final destination, but that
doesn't seems to be the case.

I tried to see what happens if the `sed' command fails, and the target is
created, empty, and doesn't gets deleted by make. So an incremental
build uses a broken file without trying to rebuild it.

If we want `make' to delete target when a rule fails, I think we need to
add '.DELETE_ON_ERROR:' somewhere. Or avoid creating files before the
command is likely to succeed.

Cheers,

-- 
Anthony PERARD

Re: [PATCH v1] libacpi: use temporary files for generated files

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 278 bytes --]

Am Thu, 29 Oct 2020 10:57:15 +0000
schrieb Anthony PERARD <anthony.perard@citrix.com>:

> we need to add '.DELETE_ON_ERROR:'

The last paragraph at https://www.gnu.org/software/make/manual/html_node/Errors.html#Errors suggests that this is a good thing to have.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] libacpi: use temporary files for generated files

[Jan Beulich]

On 29.10.2020 11:57, Anthony PERARD wrote:
> On Thu, Oct 29, 2020 at 09:47:09AM +0100, Jan Beulich wrote:
>> On 28.10.2020 19:13, Anthony PERARD wrote:
>>> On Tue, Oct 27, 2020 at 12:06:56PM +0100, Jan Beulich wrote:
>>>> On 27.10.2020 11:57, Andrew Cooper wrote:
>>>>> On 27/10/2020 10:37, Jan Beulich wrote:
>>>>>> On 27.10.2020 11:27, Olaf Hering wrote:
>>>>>>> Am Tue, 27 Oct 2020 11:16:04 +0100
>>>>>>> schrieb Jan Beulich <jbeulich@suse.com>:
>>>>>>>
>>>>>>>> This pattern is used when a rule consists of multiple commands
>>>>>>>> having their output appended to one another's.
>>>>>>> My understanding is: a rule is satisfied as soon as the file exists.
>>>>>> No - once make has found that a rule's commands need running, it'll
>>>>>> run the full set and only check again afterwards.
>>>>>
>>>>> It stops at the first command which fails.
>>>>>
>>>>> Olaf is correct, but the problem here is an incremental build issue, not
>>>>> a parallel build issue.
>>>>>
>>>>> Intermediate files must not use the name of the target, or a failure and
>>>>> re-build will use the (bogus) intermediate state rather than rebuilding it.
>>>>
>>>> But there's no intermediate file here - the file gets created in one
>>>> go. Furthermore doesn't make delete the target file(s) when a rule
>>>> fails? (One may not want to rely on this, and hence indeed keep multi-
>>>> part rules update intermediate files of different names.)
>>>
>>> What if something went badly enough and sed didn't managed to write the
>>> whole file and make never had a chance to remove the bogus file?
>>
>> How's this different from an object file getting only partly written
>> and not deleted? We'd have to use the temporary file approach in
>> literally every rule if we wanted to cater for this.
> 
> I though that things like `gcc' would write the final object to a
> temporary place then rename it to the final destination, but that
> doesn't seems to be the case.
> 
> I tried to see what happens if the `sed' command fails, and the target is
> created, empty, and doesn't gets deleted by make. So an incremental
> build uses a broken file without trying to rebuild it.

IOW it's rather a courtesy of the compiler / assembler / linker
to delete their output files on error.

> If we want `make' to delete target when a rule fails, I think we need to
> add '.DELETE_ON_ERROR:' somewhere.

Ah, indeed. I thought this was the default nowadays, but the doc
says it isn't. I think this would be preferable over touching
individual rules to go through temporary files.

Jan