[pkgsrc/trunk]: pkgsrc mk/subst.mk: evaluate SUBST_MESSAGE only once, and late

[rillig <rillig%pkgsrc.org@localhost>]

details:   https://anonhg.NetBSD.org/pkgsrc/rev/e86fb6581174
branches:  trunk
changeset: 425167:e86fb6581174
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Fri Mar 20 09:00:44 2020 +0000

description:
mk/subst.mk: evaluate SUBST_MESSAGE only once, and late

The default value of SUBST_MESSAGE is based on SUBST_FILES, and that
variable may use the :sh modifier to list files from WRKSRC, which may
not exist at load time.

diffstat:

 mk/subst.mk                      |  11 +++++---
 regress/infra-unittests/subst.sh |  52 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 7 deletions(-)

diffs (94 lines):

diff -r 4aced05d0c88 -r e86fb6581174 mk/subst.mk
--- a/mk/subst.mk       Fri Mar 20 08:56:42 2020 +0000
+++ b/mk/subst.mk       Fri Mar 20 09:00:44 2020 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: subst.mk,v 1.64 2020/03/20 06:17:48 rillig Exp $
+# $NetBSD: subst.mk,v 1.65 2020/03/20 09:00:44 rillig Exp $
 #
 # The subst framework replaces text in one or more files in the WRKSRC
 # directory. Packages can define several ``classes'' of replacements.
@@ -36,6 +36,10 @@
 #      has no effect at all, to catch typos and outdated definitions.
 #      To prevent this, see SUBST_NOOP_OK.<class> below.
 #
+#      In most cases the filename patterns are given directly.
+#      If that is not flexible enough, use the :sh variable modifier.
+#      See mk/configure/replace-localedir.mk for an example.
+#
 # SUBST_SED.<class>
 #      List of sed(1) arguments to run on the specified files.
 #      Multiple commands can be specified using the -e option of sed.
@@ -133,9 +137,8 @@
 subst-${_class_}: ${_SUBST_COOKIE.${_class_}}
 
 ${_SUBST_COOKIE.${_class_}}:
-.  if !empty(SUBST_MESSAGE.${_class_})
-       ${RUN} ${ECHO_SUBST_MSG} ${SUBST_MESSAGE.${_class_}:Q}
-.  endif
+       ${RUN} message=${SUBST_MESSAGE.${_class_}:Q};                   \
+       if [ "$$message" ]; then ${ECHO_SUBST_MSG} "$$message"; fi
        ${RUN}                                                          \
        basedir=${WRKSRC:Q};                                            \
        emptydir="$$basedir/.subst-empty";                              \
diff -r 4aced05d0c88 -r e86fb6581174 regress/infra-unittests/subst.sh
--- a/regress/infra-unittests/subst.sh  Fri Mar 20 08:56:42 2020 +0000
+++ b/regress/infra-unittests/subst.sh  Fri Mar 20 09:00:44 2020 +0000
@@ -379,9 +379,55 @@
        test_case_end
 fi
 
-# TODO: Add test that ensures SUBST_FILES is evaluated as late as possible.
-# Evaluating it early breaks cases like _pkglocaledir where the list of files
-# is generated by a shell command. See mk/configure/replace-localedir.mk.
+
+if test_case_begin "late evaluation of SUBST_FILES"; then
+
+       # Ensure that SUBST_FILES is evaluated as late as possible.
+       # Evaluating it early breaks cases like pkglocaledir where the
+       # list of files is generated by a shell command.
+       # See mk/configure/replace-localedir.mk.
+
+       create_file "testcase.mk" <<EOF
+REPLACE_FILES_CMD.class= \
+       cd \${WRKSRC} && echo *r*
+
+SUBST_CLASSES+=                class
+SUBST_STAGE.class=     pre-configure
+SUBST_FILES.class=     first third # \${REPLACE_FILES_CMD.class:sh}
+SUBST_SED.class=       -e 's,from,to,'
+
+.include "prepare-subst.mk"
+RUN=           @set -e;
+WRKSRC=                \${WRKDIR}/package-1.0
+.include "mk/subst.mk"
+
+# It is tricky that the dependency must use an internal implementation
+# detail, but that's the only way to guarantee the correct ordering.
+\${_SUBST_COOKIE.class}: prepare-subst-class
+prepare-subst-class:
+       \${RUN} \${MKDIR} \${WRKSRC}
+       \${RUN} \${ECHO} "from" > "\${WRKSRC}/first"
+       \${RUN} \${ECHO} "from" > "\${WRKSRC}/second"
+       \${RUN} \${ECHO} "from" > "\${WRKSRC}/third"
+EOF
+
+       test_file "testcase.mk" "subst-class" \
+               1> "$tmpdir/actual-output" \
+               2> "$tmpdir/actual-stderr" \
+       && exitcode=0 || exitcode=$?
+
+       create_file_lines "expected-output" \
+               '=> Substituting "class" in first third'
+       assert_that "actual-output" --file-equals "expected-output"
+       assert_that "actual-stderr" --file-is-empty
+       assert_that "package-1.0/first" --file-contains-exactly "to"
+       assert_that "package-1.0/second" --file-contains-exactly "from"
+       assert_that "package-1.0/third" --file-contains-exactly "to"
+       assert_that "$exitcode" --equals "0"
+
+       test_case_end
+fi
+
 
 # TODO: Add tests for files having special characters, like space, dollar,
 # backticks, basically every ASCII character.