Making mk/subst.mk more robust

[Roland Illig <roland.illig%gmx.de@localhost>]

Hi,

currently, the SUBST framework in mk/subst.mk may lead to file
corruption during active package development. This happens when a sed(1)
command has a syntax error, so that the whole command is terminated. In
such a case, the original file becomes empty.

The appended patch fixes that by applying the sed(1) commands to the
original file and saving the result in a temporary file. Only after that
succeeded is the original file overwritten.

The one tricky part is the SUBST_POSTCMD, which relies on the exact
procedure from before the patch. But since it is only used in exactly
one place (mk/wrapper), not by any packages in either main pkgsrc or
pkgsrc-wip, I would like to remove that variable.

Thoughts?

Roland

Index: mk/subst.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/subst.mk,v
retrieving revision 1.54
diff -u -p -r1.54 subst.mk
--- mk/subst.mk	13 Oct 2013 21:38:36 -0000	1.54
+++ mk/subst.mk	30 Jan 2016 12:33:53 -0000
@@ -40,10 +40,6 @@
 #	Filter used to perform the actual substitution on the specified
 #	files.  Defaults to ${SED} ${SUBST_SED.<class>}.
 #
-# SUBST_POSTCMD.<class>
-#	Command to clean up after sed(1). Defaults to ${RM} -f
-#	$$file${_SUBST_BACKUP_SUFFIX}. For debugging, set it to ${DO_NADA}.
-#
 # SUBST_SKIP_TEXT_CHECK.<class>
 #	By default, each file is checked whether it really is a text file
 #	before any substitutions are done to it. Since that test is not
@@ -59,7 +55,7 @@ _VARGROUPS+=		subst
 _PKG_VARS.subst=	SUBST_CLASSES
 .for c in ${SUBST_CLASSES}
 .  for pv in SUBST_STAGE SUBST_MESSAGE SUBST_FILES SUBST_SED SUBST_VARS	\
-	SUBST_FILTER_CMD SUBST_POSTCMD SUBST_SKIP_TEXT_CHECK
+	SUBST_FILTER_CMD SUBST_SKIP_TEXT_CHECK
 _PKG_VARS.subst+=	${pv}.${c}
 .  endfor
 .endfor
@@ -84,7 +80,7 @@ SUBST_MESSAGE.${_class_}?=	Substituting 
 .  for v in ${SUBST_VARS.${_class_}}
 SUBST_FILTER_CMD.${_class_} +=	-e s,@${v}@,${${v}:S|\\|\\\\|gW:S|,|\\,|gW:S|&|\\\&|gW:Q},g
 .  endfor
-SUBST_POSTCMD.${_class_}?=	${RM} -f "$$tmpfile"
+_SUBST_KEEP.${_class_}?=	${DO_NADA}
 SUBST_SKIP_TEXT_CHECK.${_class_}?=	no
 
 .if !empty(SUBST_SKIP_TEXT_CHECK.${_class_}:M[Yy][Ee][Ss])
@@ -117,18 +113,18 @@ ${_SUBST_COOKIE.${_class_}}:
 		if [ ! -f "$$file" ]; then				\
 			${WARNING_MSG} "[subst.mk:${_class_}] Ignoring non-existent file \"$$file\"."; \
 		elif ${_SUBST_IS_TEXT_FILE.${_class_}}; then		\
-			${MV} -f "$$file" "$$tmpfile" || exit 1;	\
 			${SUBST_FILTER_CMD.${_class_}}			\
-			< "$$tmpfile"					\
-			> "$$file";					\
-			if ${TEST} -x "$$tmpfile"; then			\
-				${CHMOD} +x "$$file";			\
+			< "$$file"					\
+			> "$$tmpfile";					\
+			if ${TEST} -x "$$file"; then			\
+				${CHMOD} +x "$$tmpfile";		\
 			fi;						\
 			if ${CMP} -s "$$tmpfile" "$$file"; then 	\
 				${INFO_MSG} "[subst.mk:${_class_}] Nothing changed in $$file."; \
-				${MV} -f "$$tmpfile" "$$file";		\
+				${RM} -f "$$tmpfile";			\
 			else						\
-				${SUBST_POSTCMD.${_class_}};		\
+				${_SUBST_KEEP.${_class_}};		\
+				${MV} -f "$$tmpfile" "$$file"; 		\
 				${ECHO} "$$file" >> ${.TARGET};		\
 			fi;						\
 		else							\
Index: mk/wrapper/bsd.wrapper.mk
===================================================================
RCS file: /cvsroot/pkgsrc/mk/wrapper/bsd.wrapper.mk,v
retrieving revision 1.93
diff -u -p -r1.93 bsd.wrapper.mk
--- mk/wrapper/bsd.wrapper.mk	27 Apr 2015 19:59:07 -0000	1.93
+++ mk/wrapper/bsd.wrapper.mk	30 Jan 2016 12:33:53 -0000
@@ -688,7 +688,7 @@ SUBST_MESSAGE.unwrap=	Unwrapping files-t
 SUBST_FILES.unwrap=	${_UNWRAP_FILES}
 SUBST_SED.unwrap=	${_UNWRAP_SED}
 .if defined(_WRAPPER_DEBUG) && !empty(_WRAPPER_DEBUG:M[yY][eE][sS])
-SUBST_POSTCMD.unwrap=	${DO_NADA}
+_SUBST_KEEP.unwrap=	${CP} -f "$$file" "$$file.before-unwrap"
 .endif
 
 .endif