GNU bug report logs - #19614
make dist exits succesfully even when tar exits with error

Previous Next

Full log

Message #19 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: bogdro_rep <at> gmx.us, jimis <at> gmx.net
Cc: automake-patches <at> gnu.org
Subject: Re: bug#19614: Split packaging invocation to catch errors
Date: Mon, 17 Jul 2023 19:22:39 -0600

Hi Dimitrios, Bogdan - back on this bug from 2015 (sorry):
    https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19614

Bogdan sent a patch that splits the tar and compress into separate
invocations.  It seems basically good to me, but the dist-formats test
fails because it builds multiple archive formats (.tar.gz, .tar.bz2,
etc.) in parallel, and so removing $(distdir).tar (and .err) files are
subject to a race condition.

So my question is, will it suffice in this limited case to just put $$
into the filenames? It seems like it should be ok to me, but I'm not
sure I have enough imagination to know why that would fail. I can't see
figuring out how to run mktemp here.

Advice please? --thanks, karl.

P.S. I also made small changes to the patch 1) to use a ".tarerr" file
instead of just ".tar" to save stderr, since I think ".err" might
already be in use by some packages, and 2) remove the .tar and .tarerr
in the event of success, else make distclean leaves them behind.
Here's the diff I've currently got for distdir.am.

diff --git a/lib/am/distdir.am b/lib/am/distdir.am
index 264713c33..62a781be6 100644
--- a/lib/am/distdir.am
+++ b/lib/am/distdir.am
@@ -33,7 +33,11 @@ am__remove_distdir = \
 ## See automake bug#10470.
       || { sleep 5 && rm -rf "$(distdir)"; }; \
   else :; fi
-am__post_remove_distdir = $(am__remove_distdir)
+## We now generate and compress the tar file in separate commands,
+## and also save stderr, all to detect errors from tar. Clean up.
+am__post_remove_distdir = \
+  rm -f $(distdir).tar $(distdir).tarerr \
+  && $(am__remove_distdir)
 endif %?TOPDIR_P%
 
 if %?SUBDIRS%
@@ -334,31 +338,56 @@ if %?TOPDIR_P%
 GZIP_ENV = --best
 .PHONY: dist-gzip
 dist-gzip: distdir
-	tardir=$(distdir) && $(am__tar) | eval GZIP= gzip $(GZIP_ENV) -c >$(distdir).tar.gz
+	tardir=$(distdir) && $(am__tar) > $(distdir).tar 2>$(distdir).tarerr
+	test ! -s $(distdir).tarerr || (cat $(distdir).tarerr && \
+		rm -rf $(DIST_ARCHIVES) $(distdir).tar $(distdir).tarerr && \
+		$(am__post_remove_distdir) && \
+		exit 1)
+	eval GZIP= gzip $(GZIP_ENV) $(distdir).tar -c >$(distdir).tar.gz
 	$(am__post_remove_distdir)
 
 ?BZIP2?DIST_ARCHIVES += $(distdir).tar.bz2
 .PHONY: dist-bzip2
 dist-bzip2: distdir
-	tardir=$(distdir) && $(am__tar) | BZIP2=$${BZIP2--9} bzip2 -c >$(distdir).tar.bz2
+	tardir=$(distdir) && $(am__tar) > $(distdir).tar 2>$(distdir).tarerr
+	test ! -s $(distdir).tarerr || (cat $(distdir).tarerr && \
+		rm -rf $(DIST_ARCHIVES) $(distdir).tar $(distdir).tarerr && \
+		$(am__post_remove_distdir) && \
+		exit 1)
+	BZIP2=$${BZIP2--9} bzip2 -c $(distdir).tar >$(distdir).tar.bz2
 	$(am__post_remove_distdir)
 
 ?LZIP?DIST_ARCHIVES += $(distdir).tar.lz
 .PHONY: dist-lzip
 dist-lzip: distdir
-	tardir=$(distdir) && $(am__tar) | lzip -c $${LZIP_OPT--9} >$(distdir).tar.lz
+	tardir=$(distdir) && $(am__tar) > $(distdir).tar 2>$(distdir).tarerr
+	test ! -s $(distdir).tarerr || (cat $(distdir).tarerr && \
+		rm -rf $(DIST_ARCHIVES) $(distdir).tar $(distdir).tarerr && \
+		$(am__post_remove_distdir) && \
+		exit 1)
+	lzip -c $${LZIP_OPT--9} $(distdir).tar >$(distdir).tar.lz
 	$(am__post_remove_distdir)
 
 ?XZ?DIST_ARCHIVES += $(distdir).tar.xz
 .PHONY: dist-xz
 dist-xz: distdir
-	tardir=$(distdir) && $(am__tar) | XZ_OPT=$${XZ_OPT--e} xz -c >$(distdir).tar.xz
+	tardir=$(distdir) && $(am__tar) > $(distdir).tar 2>$(distdir).tarerr
+	test ! -s $(distdir).tarerr || (cat $(distdir).tarerr && \
+		rm -rf $(DIST_ARCHIVES) $(distdir).tar $(distdir).tarerr && \
+		$(am__post_remove_distdir) && \
+		exit 1)
+	XZ_OPT=$${XZ_OPT--e} xz -c $(distdir).tar >$(distdir).tar.xz
 	$(am__post_remove_distdir)
 
 ?ZSTD?DIST_ARCHIVES += $(distdir).tar.zst
 .PHONY: dist-zstd
 dist-zstd: distdir
-	tardir=$(distdir) && $(am__tar) | zstd -c $${ZSTD_CLEVEL-$${ZSTD_OPT--19}} >$(distdir).tar.zst
+	tardir=$(distdir) && $(am__tar) > $(distdir).tar 2>$(distdir).tarerr
+	test ! -s $(distdir).tarerr || (cat $(distdir).tarerr && \
+		rm -rf $(DIST_ARCHIVES) $(distdir).tar $(distdir).tarerr && \
+		$(am__post_remove_distdir) && \
+		exit 1)
+	zstd -c $${ZSTD_CLEVEL-$${ZSTD_OPT--19}} $(distdir).tar >$(distdir).tar.zst
 	$(am__post_remove_distdir)
 
 ?COMPRESS?DIST_ARCHIVES += $(distdir).tar.Z
@@ -367,7 +396,12 @@ dist-tarZ: distdir
 	@echo WARNING: "Support for distribution archives compressed with" \
 		       "legacy program 'compress' is deprecated." >&2
 	@echo WARNING: "It will be removed altogether in Automake 2.0" >&2
-	tardir=$(distdir) && $(am__tar) | compress -c >$(distdir).tar.Z
+	tardir=$(distdir) && $(am__tar) > $(distdir).tar 2>$(distdir).tarerr
+	test ! -s $(distdir).tarerr || (cat $(distdir).tarerr && \
+		rm -rf $(DIST_ARCHIVES) $(distdir).tar $(distdir).tarerr && \
+		$(am__post_remove_distdir) && \
+		exit 1)
+	compress -c $(distdir).tar >$(distdir).tar.Z
 	$(am__post_remove_distdir)
 
 ?SHAR?DIST_ARCHIVES += $(distdir).shar.gz

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.