CVS commit: pkgsrc

["Roland Illig" <rillig%netbsd.org@localhost>]

Module Name:    pkgsrc
Committed By:   rillig
Date:           Fri Mar 20 06:17:48 UTC 2020

Modified Files:
        pkgsrc/mk: subst.mk
        pkgsrc/regress/infra-unittests: subst.sh

Log Message:
mk/subst.mk: fix early evaluation of SUBST_FILES

In the case of pkglocaledir, the SUBST_FILES are generated by a shell
command. That command assumes that the WRKDIR already exists. Therefore
SUBST_FILES must be evaluated as late as possible.

See mk/configure/replace-localedir.mk; an example package that fails is
devel/gettext-tools.


To generate a diff of this commit:
cvs rdiff -u -r1.63 -r1.64 pkgsrc/mk/subst.mk
cvs rdiff -u -r1.2 -r1.3 pkgsrc/regress/infra-unittests/subst.sh

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: pkgsrc/mk/subst.mk
diff -u pkgsrc/mk/subst.mk:1.63 pkgsrc/mk/subst.mk:1.64
--- pkgsrc/mk/subst.mk:1.63     Thu Mar 19 16:57:35 2020
+++ pkgsrc/mk/subst.mk  Fri Mar 20 06:17:48 2020
@@ -1,4 +1,4 @@
-# $NetBSD: subst.mk,v 1.63 2020/03/19 16:57:35 rillig Exp $
+# $NetBSD: subst.mk,v 1.64 2020/03/20 06:17:48 rillig Exp $
 #
 # The subst framework replaces text in one or more files in the WRKSRC
 # directory. Packages can define several ``classes'' of replacements.
@@ -136,10 +136,15 @@ ${_SUBST_COOKIE.${_class_}}:
 .  if !empty(SUBST_MESSAGE.${_class_})
        ${RUN} ${ECHO_SUBST_MSG} ${SUBST_MESSAGE.${_class_}:Q}
 .  endif
-.for pattern in ${SUBST_FILES.${_class_}}
-       ${RUN} cd ${WRKSRC:Q};                                          \
+       ${RUN}                                                          \
+       basedir=${WRKSRC:Q};                                            \
+       emptydir="$$basedir/.subst-empty";                              \
+       patterns=${SUBST_FILES.${_class_}:Q};                           \
+       ${MKDIR} "$$emptydir"; cd "$$emptydir";                         \
+       for pattern in $$patterns; do                                   \
        changed=no;                                                     \
-       for file in ${pattern}; do                                      \
+       cd "$$basedir";                                                 \
+       for file in $$pattern; do                                       \
                case $$file in /*) ;; *) file="./$$file";; esac;        \
                tmpfile="$$file.subst.sav";                             \
                if [ ! -f "$$file" ]; then                              \
@@ -166,8 +171,9 @@ ${_SUBST_COOKIE.${_class_}}:
        done;                                                           \
        \
        if test "$$changed,${SUBST_NOOP_OK.${_class_}:tl}" = no,no; then \
-               ${FAIL_MSG} "[subst.mk:${_class_}] The pattern "${pattern:Q}" has no effect."; \
-       fi
-.endfor
+               ${FAIL_MSG} "[subst.mk:${_class_}] The pattern $$pattern has no effect."; \
+       fi; \
+       done; \
+       ${RMDIR} "$$emptydir"
        ${RUN} ${TOUCH} ${TOUCH_FLAGS} ${.TARGET:Q}
 .endfor

Index: pkgsrc/regress/infra-unittests/subst.sh
diff -u pkgsrc/regress/infra-unittests/subst.sh:1.2 pkgsrc/regress/infra-unittests/subst.sh:1.3
--- pkgsrc/regress/infra-unittests/subst.sh:1.2 Thu Mar 19 22:46:34 2020
+++ pkgsrc/regress/infra-unittests/subst.sh     Fri Mar 20 06:17:48 2020
@@ -9,7 +9,7 @@ set -eu
 testcase() {
        test_name="$1"
 
-       rm -rf "$tmpdir/".subst_*_done
+       rm -rf "$tmpdir"/.subst_*_done "$tmpdir"/.subst-empty
        rm -rf "$tmpdir"/*
        ls -A "$tmpdir"
 
@@ -19,8 +19,10 @@ testcase() {
 CHMOD=         chmod-is-not-used
 CMP=           cmp
 ECHO=          echo
+MKDIR=         mkdir -p
 MV=            mv
 RM=            rm
+RMDIR=         rmdir
 SED=           sed
 TEST=          test
 TOUCH=         touch
@@ -29,7 +31,7 @@ TR=           tr
 TRUE=          true
 
 # Commands that are specific to pkgsrc
-RUN=           @: set -x;
+RUN=           @set -e;
 STEP_MSG=      echo "=>"
 DO_NADA=       : do-nada
 INFO_MSG=      echo "info:"
@@ -164,7 +166,7 @@ EOF
 fi
 
 
-if testcase "single file noop"; then
+if testcase "single file noop, noop_ok=yes"; then
 
        create_file "testcase.mk" <<EOF
 SUBST_CLASSES+=                class
@@ -182,17 +184,17 @@ EOF
        create_file_lines "single"      "already an example"
 
        test_file "testcase.mk" > "$tmpdir/actual-output" && exitcode=0 || exitcode=$?
-       assert_that "$exitcode" --equals "0"
+
        create_file_lines "expected-output" \
                '=> Substituting "class" in single' \
                'info: [subst.mk:class] Nothing changed in ./single.'
-
        assert_that "actual-output" --file-equals "expected-output"
        assert_that "single" --file-contains-exactly "already an example"
+       assert_that "$exitcode" --equals "0"
 fi
 
 
-if testcase "single file noop ok"; then
+if testcase "single file noop, noop_ok=no"; then
 
        create_file "testcase.mk" <<EOF
 SUBST_CLASSES+=                class
@@ -210,7 +212,7 @@ EOF
        create_file_lines "single"      "already an example"
 
        test_file "testcase.mk" > "$tmpdir/actual-output" && exitcode=0 || exitcode=$?
-       assert_that "$exitcode" --equals "1"
+
        create_file_lines "expected-output" \
                '=> Substituting "class" in single' \
                'info: [subst.mk:class] Nothing changed in ./single.' \
@@ -219,9 +221,9 @@ EOF
                '' \
                'Stop.' \
                "$make: stopped in $PWD"
-
        assert_that "actual-output" --file-equals "expected-output"
        assert_that "single" --file-contains-exactly "already an example"
+       assert_that "$exitcode" --equals "1"
 fi
 
 
@@ -241,7 +243,7 @@ all: subst-class
 EOF
 
        test_file "testcase.mk" > "$tmpdir/actual-output" && exitcode=0 || exitcode=$?
-       assert_that "$exitcode" --equals "1"
+
        create_file_lines "expected-output" \
                '=> Substituting "class" in nonexistent' \
                'warning: [subst.mk:class] Ignoring non-existent file "./nonexistent".' \
@@ -250,8 +252,8 @@ EOF
                '' \
                'Stop.' \
                "$make: stopped in $PWD"
-
        assert_that "actual-output" --file-equals "expected-output"
+       assert_that "$exitcode" --equals "1"
 fi
 
 
@@ -271,12 +273,12 @@ all: subst-class
 EOF
 
        test_file "testcase.mk" > "$tmpdir/actual-output" && exitcode=0 || exitcode=$?
-       assert_that "$exitcode" --equals "0"
+
        create_file_lines "expected-output" \
                '=> Substituting "class" in nonexistent' \
                'warning: [subst.mk:class] Ignoring non-existent file "./nonexistent".'
-
        assert_that "actual-output" --file-equals "expected-output"
+       assert_that "$exitcode" --equals "0"
 fi
 
 
@@ -326,7 +328,6 @@ EOF
                'warning: [subst.mk:class] Ignoring non-existent file "./does".' \
                'warning: [subst.mk:class] Ignoring non-existent file "./not".' \
                'warning: [subst.mk:class] Ignoring non-existent file "./exist".'
-
        assert_that "actual-output" --file-equals "expected-output"
        assert_that "$exitcode" --equals "0"
 fi
@@ -354,7 +355,13 @@ EOF
                'info: [subst.mk:class] Nothing changed in ./first.' \
                'info: [subst.mk:class] Nothing changed in ./second.' \
                'info: [subst.mk:class] Nothing changed in ./third.'
-
        assert_that "actual-output" --file-equals "expected-output"
        assert_that "$exitcode" --equals "0"
 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.
+
+# TODO: Add tests for files having special characters, like space, dollar,
+# backticks, basically every ASCII character.