[libvirt] [PATCH] docs: ensure outputfile is deleted if rst2html/rst2man fail
This avoids leaving a zero length or partially generated output file on errors. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/Makefile.am | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index eb8de80b9c..9a1f7a6117 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8) grep -v '^\.\. contents::' < $< | \ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \ -e 's|RUNSTATEDIR|$(runstatedir)|g' | \ - $(RST2MAN) > $@ + $(RST2MAN) > $@ || { rm $@ && exit 1; } %.7: %.rst $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \ grep -v '^\.\. contents::' < $< | \ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \ -e 's|RUNSTATEDIR|$(runstatedir)|g' | \ - $(RST2MAN) > $@ + $(RST2MAN) > $@ || { rm $@ && exit 1; } %.8: %.rst $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \ grep -v '^\.\. contents::' < $< | \ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \ -e 's|RUNSTATEDIR|$(runstatedir)|g' | \ - $(RST2MAN) > $@ + $(RST2MAN) > $@ || { rm $@ && exit 1; } manpages/virkeycode-%.rst: $(top_srcdir)/src/keycodemapdb/data/keymaps.csv \ $(top_srcdir)/src/keycodemapdb/tools/keymap-gen Makefile.am @@ -414,11 +414,11 @@ manpages/%.html.in: manpages/%.rst grep -v '^:Manual ' < $< | \ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \ -e 's|RUNSTATEDIR|$(runstatedir)|g' | \ - $(RST2HTML) > $@ + $(RST2HTML) > $@ || { rm $@ && exit 1; } %.html.in: %.rst $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \ - $(RST2HTML) $< > $@ + $(RST2HTML) $< > $@ || { rm $@ && exit 1; } %.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \ $(acl_generated) -- 2.23.0
On 12/11/19 12:45 PM, Daniel P. Berrangé wrote:
This avoids leaving a zero length or partially generated output file on errors.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/Makefile.am | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am index eb8de80b9c..9a1f7a6117 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8) grep -v '^\.\. contents::' < $< | \ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \ -e 's|RUNSTATEDIR|$(runstatedir)|g' | \ - $(RST2MAN) > $@ + $(RST2MAN) > $@ || { rm $@ && exit 1; }
But still allows a truncated view of the file if another process accesses the file while RST2MAN is still running. Even better is to generate output to a temp file then atomically mv it into place, so that no concurrent process can ever see an incomplete file. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Wed, Dec 11, 2019 at 12:52:20PM -0600, Eric Blake wrote:
On 12/11/19 12:45 PM, Daniel P. Berrangé wrote:
This avoids leaving a zero length or partially generated output file on errors.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/Makefile.am | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am index eb8de80b9c..9a1f7a6117 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8) grep -v '^\.\. contents::' < $< | \ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \ -e 's|RUNSTATEDIR|$(runstatedir)|g' | \ - $(RST2MAN) > $@ + $(RST2MAN) > $@ || { rm $@ && exit 1; }
But still allows a truncated view of the file if another process accesses the file while RST2MAN is still running. Even better is to generate output to a temp file then atomically mv it into place, so that no concurrent process can ever see an incomplete file.
Is that a problem that actually impacts negatively in real world for builds? In various places in the make rules, we've either done the "|| rm" approach, or the temp file and rename approach. Personally I find the recipes using the "|| rm" approach are more maintainable/readable. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 12/11/19 12:59 PM, Daniel P. Berrangé wrote:
On Wed, Dec 11, 2019 at 12:52:20PM -0600, Eric Blake wrote:
On 12/11/19 12:45 PM, Daniel P. Berrangé wrote:
This avoids leaving a zero length or partially generated output file on errors.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/Makefile.am | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am index eb8de80b9c..9a1f7a6117 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8) grep -v '^\.\. contents::' < $< | \ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \ -e 's|RUNSTATEDIR|$(runstatedir)|g' | \ - $(RST2MAN) > $@ + $(RST2MAN) > $@ || { rm $@ && exit 1; }
But still allows a truncated view of the file if another process accesses the file while RST2MAN is still running. Even better is to generate output to a temp file then atomically mv it into place, so that no concurrent process can ever see an incomplete file.
Is that a problem that actually impacts negatively in real world for builds? In various places in the make rules, we've either done the "|| rm" approach, or the temp file and rename approach. Personally I find the recipes using the "|| rm" approach are more maintainable/readable.
If you're trying to browse the docs and build at the same time, such as checking if your tweak to a doc renders well, you could hit it; but you're also right that it's not a show-stopper problem. Go with whatever is easier to maintain if you don't care about the issue of atomicity in the files (the rm to avoid a long-term corruption is more important than the atomic mv to avoid a short-term corruption). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Daniel P. Berrangé -
Eric Blake