| Message ID | 20201026204151.23459-1-olaf@aepfle.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] libacpi: use temporary files for generated files | expand |
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 >
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
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
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
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
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,
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
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,
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
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
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
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(-)