diff options
| -rw-r--r-- | mk/scripts/subst-identity.awk | 38 | ||||
| -rw-r--r-- | mk/subst.mk | 29 | ||||
| -rw-r--r-- | regress/infra-unittests/subst.sh | 200 | ||||
| -rw-r--r-- | regress/infra-unittests/test.subr | 4 |
4 files changed, 263 insertions, 8 deletions
diff --git a/mk/scripts/subst-identity.awk b/mk/scripts/subst-identity.awk new file mode 100644 index 00000000000..fb6bdaab9e2 --- /dev/null +++ b/mk/scripts/subst-identity.awk @@ -0,0 +1,38 @@ +#! /usr/bin/awk -f +# $NetBSD: subst-identity.awk,v 1.1 2020/04/29 18:33:57 rillig Exp $ +# +# Tests whether a sed(1) command line consists of only identity substitutions +# like s,id,id,. +# +# See SUBST_NOOP_OK and regress/infra-unittests/subst.sh. +# + +function is_safe_char(ch) { + return ch ~ /[\t -~]/ && ch !~ /[$&*.\[\\\]^]/; +} + +function is_identity_subst(s, len, i, sep, pat) { + len = length(s); + if (len < 6 || substr(s, 1, 1) != "s") + return 0; + + sep = substr(s, 2, 1); + i = 3; + while (i < len && substr(s, i, 1) != sep && is_safe_char(substr(s, i, 1))) + i++; + pat = substr(s, 3, i - 3); + + return (s == "s" sep pat sep pat sep || + s == "s" sep pat sep pat sep "g"); +} + +function main( i) { + for (i = 1; i + 1 < ARGC; i += 2) + if (ARGV[i] != "-e" || !is_identity_subst(ARGV[i + 1])) + return 0; + return i == ARGC && ARGC > 1; +} + +BEGIN { + exit(main() ? 0 : 1); +} diff --git a/mk/subst.mk b/mk/subst.mk index dc84bb227aa..eb15f28317d 100644 --- a/mk/subst.mk +++ b/mk/subst.mk @@ -1,4 +1,4 @@ -# $NetBSD: subst.mk,v 1.84 2020/04/23 19:32:53 rillig Exp $ +# $NetBSD: subst.mk,v 1.85 2020/04/29 18:33:56 rillig Exp $ # # The subst framework replaces text in one or more files in the WRKSRC # directory. Packages can define several ``classes'' of replacements. @@ -22,8 +22,20 @@ # SUBST classes. Defaults to "no". # # SUBST_NOOP_OK -# Whether it is ok to have filename patterns in SUBST_FILES that -# don't have any effect. +# Whether it is ok to list files in SUBST_FILES that don't contain +# any of the patterns from SUBST_SED or SUBST_VARS. Such a +# situation often arises when a package is updated to a newer +# version, and the build instructions of the package have been +# made more portable or flexible. +# +# This setting only affects the filename patterns in SUBST_FILES. +# It does not (yet) affect the regular expressions in SUBST_SED. +# +# From the viewpoint of sed(1), a pattern like s|man|man| may look +# redundant but it really isn't, because the second "man" probably +# comes from ${PKGMANDIR}, which may be configured to a different +# directory. Patterns like these are therefore allowed, even if +# they are no-ops in the current configuration. # # For backwards compatibility this defaults to "yes", but it # should rather be set to "no". @@ -94,7 +106,7 @@ # SUBST_SHOW_DIFF?= no -SUBST_NOOP_OK?= yes # only for backwards compatiblity +SUBST_NOOP_OK?= yes # only for backwards compatibility _VARGROUPS+= subst _USER_VARS.subst= SUBST_SHOW_DIFF SUBST_NOOP_OK @@ -183,6 +195,13 @@ ${_SUBST_COOKIE.${class}}: }; \ ${SUBST_FILTER_CMD.${class}} < "$$file" > "$$tmpfile"; \ ${CMP} -s "$$tmpfile" "$$file" && { \ + [ ${"${SUBST_FILTER_CMD.${class}}" == "LC_ALL=C ${SED} ${SUBST_SED.${class}}":?true:false} ] \ + && ${AWK} -f ${PKGSRCDIR}/mk/scripts/subst-identity.awk -- ${SUBST_SED.${class}} \ + && found=`LC_ALL=C ${SED} -n ${SUBST_SED.${class}:C,^['"]?s.*,&p,} "$$file"` \ + && [ "x$$found" != "x" ] && { \ + changed=yes; \ + continue; \ + }; \ ${_SUBST_WARN.${class}} "Nothing changed in \"$$file\"."; \ ${RM} -f "$$tmpfile"; \ continue; \ @@ -193,7 +212,7 @@ ${_SUBST_COOKIE.${class}}: ${MV} -f "$$tmpfile" "$$file"; \ ${ECHO} "$$file" >> ${.TARGET}.tmp; \ done; \ - \ + \ [ "$$changed,${SUBST_NOOP_OK.${class}:tl}" = no,no ] && { \ noop_count="$$noop_count+"; \ noop_patterns="$$noop_patterns$$noop_sep$$pattern"; \ diff --git a/regress/infra-unittests/subst.sh b/regress/infra-unittests/subst.sh index 3359b323df7..882d7726dc4 100644 --- a/regress/infra-unittests/subst.sh +++ b/regress/infra-unittests/subst.sh @@ -1,5 +1,5 @@ #! /bin/sh -# $NetBSD: subst.sh,v 1.25 2020/04/26 12:46:33 rillig Exp $ +# $NetBSD: subst.sh,v 1.26 2020/04/29 18:33:56 rillig Exp $ # # Tests for mk/subst.mk. # @@ -14,6 +14,7 @@ test_case_set_up() { create_file "prepare-subst.mk" <<EOF # The tools that are used by subst.mk +AWK= awk CHMOD= chmod CMP= cmp DIFF= diff @@ -1131,3 +1132,200 @@ if test_case_begin "unreadable file"; then test_case_end fi + + +if test_case_begin "identity substitution implementation"; then + + assert_identity() { + ai_expected="$1"; shift + awk -f "$pkgsrcdir/mk/scripts/subst-identity.awk" -- "$@" \ + && ai_actual="yes" || ai_actual="no" + + [ "$ai_actual" = "$ai_expected" ] \ + || assert_fail "expected '%s', got '%s' for %s\n" "$ai_expected" "$ai_actual" "$*" + } + + # If there is no SUBST_SED at all, this is not the situation + # that is targeted by this test for identity substitution. + assert_identity "no" # no substitutions at all + + # Even though this is an identity substitution, it is missing + # the -e option and thus does not follow the usual format. + # Therefore it is considered just a normal substitution. + assert_identity "no" 's,from,from,' + + # The following are typical identity substitutions. + # It does not matter whether the g modifier is there or not. + # Unknown modifiers are not allowed though. + assert_identity "yes" -e 's,from,from,' + assert_identity "yes" -e 's;from;from;' + assert_identity "yes" -e 's,from,from,g' + assert_identity "no" -e 's,from,from,gunknown' + + # The identity substitution may include characters other than + # A-Za-z0-9, but no characters that have a special meaning in + # basic regular expressions. + assert_identity "yes" -e 's,/dev/audio,/dev/audio,' + assert_identity "yes" -e 's!/dev/audio!/dev/audio!' + + # There may be several identity substitutions in the same + # SUBST_SED. As long as all these substitutions are identity + # substitutions, they may be skipped. As soon as there is one + # other substitution, the whole SUBST_SED is treated as usual. + assert_identity "yes" -e 's;from;from;' -e 's!second!second!' + assert_identity "no" -e 's,changing,x,' -e 's,id,id,' + assert_identity "no" -e 's,id,id,' -e 's,changing,x,' + + # A demonstration of all ASCII characters that may appear in an + # identity substitution. + # + # The # and $ are excluded since they are interpreted specially + # in Makefiles and would thus be confusing to the human reader. + # + # The characters *.?[\]^ have a special meaning in the pattern of the + # substitution. + # The & has a special meaning in the replacement of the + # substitution. + specials='!"%'\''()+,-/:;<=>@_`{|}~' + assert_identity "yes" -e "sX${specials}X${specials}X" + + test_case_end +fi + + +if test_case_begin "identity substitution, found in file"; then + + # There are many situations in which a fixed text is replaced + # with a dynamic value that may or may not be equal to the + # original text. + # + # Typical examples are s|man|${PKGMANDIR}|, s|/usr/pkg|${PREFIX}|, + # s|/dev/audio|${DEVOSSAUDIO}|. + # + # It is not an error if these substitutions result in a no-op, + # provided that the text is actually found in the file. + # + # Alternatives for this special exception would be: + # + # 1. Mark these blocks as SUBST_NOOP_OK. This would not detect + # outdated definitions. Since this detection is the main goal + # of SUBST_NOOP_OK, this is out of the question. + # + # 2. Surround these blocks with a condition like ".if ${VAR} != + # fixed-value ... .endif". This pattern only works if VAR is + # definitely assigned, which often requires a corresponding + # .include line, leading to code bloat. It would also mean that + # variables defined in bsd.pkg.mk could not be used in SUBST + # blocks like these. + + create_file_lines "testcase.mk" \ + 'SUBST_CLASSES+= id' \ + 'SUBST_FILES.id= file' \ + 'SUBST_SED.id= -e s,before,before,' \ + 'SUBST_NOOP_OK.id= no' \ + '' \ + '.include "prepare-subst.mk"' \ + '.include "mk/subst.mk"' + create_file_lines "file" \ + 'before' + + run_bmake "testcase.mk" "subst-id" 1> "$tmpdir/out" 2>&1 \ + && exitcode=0 || exitcode=$? + + assert_that "out" --file-is-lines \ + '=> Substituting "id" in file' + + test_case_end +fi + + +if test_case_begin "identity substitution, not found in file"; then + + create_file_lines "testcase.mk" \ + 'SUBST_CLASSES+= id' \ + 'SUBST_FILES.id= file' \ + 'SUBST_SED.id= s,before,before,' \ + 'SUBST_NOOP_OK.id= no' \ + '' \ + '.include "prepare-subst.mk"' \ + '.include "mk/subst.mk"' + create_file_lines "file" \ + 'other' + + run_bmake "testcase.mk" "subst-id" 1> "$tmpdir/out" 2>&1 \ + && exitcode=0 || exitcode=$? + + assert_that "out" --file-is-lines \ + '=> Substituting "id" in file' \ + 'warning: [subst.mk:id] Nothing changed in "file".' \ + 'fail: [subst.mk:id] The filename pattern "file" has no effect.' \ + '*** Error code 1' \ + '' \ + 'Stop.' \ + "$make: stopped in $PWD" + + test_case_end +fi + + +if test_case_begin "identity + effective substitution"; then + + create_file_lines "testcase.mk" \ + 'SUBST_CLASSES+= id' \ + 'SUBST_FILES.id= file' \ + 'SUBST_SED.id= -e s,no-op,no-op,g' \ + 'SUBST_SED.id+= -e s,from,to,' \ + 'SUBST_NOOP_OK.id= no' \ + '' \ + '.include "prepare-subst.mk"' \ + '.include "mk/subst.mk"' + create_file_lines "file" \ + 'from' + + run_bmake "testcase.mk" "subst-id" 1> "$tmpdir/out" 2>&1 \ + && exitcode=0 || exitcode=$? + + assert_that "out" --file-is-lines \ + '=> Substituting "id" in file' + assert_that "file" --file-is-lines \ + 'to' + + test_case_end +fi + + +if test_case_begin "identity + no-op substitution"; then + + # If there were only an identity substitution, it wouldn't be an + # error. But since there is a regular substitution as well, + # that substitution is an unexpected no-op and is therefore + # flagged as an error. + + create_file_lines "testcase.mk" \ + 'SUBST_CLASSES+= id' \ + 'SUBST_FILES.id= file' \ + 'SUBST_SED.id= -e s,no-op,no-op,g' \ + 'SUBST_SED.id+= -e s,from,to,' \ + 'SUBST_NOOP_OK.id= no' \ + '' \ + '.include "prepare-subst.mk"' \ + '.include "mk/subst.mk"' + create_file_lines "file" \ + 'other' + + run_bmake "testcase.mk" "subst-id" 1> "$tmpdir/out" 2>&1 \ + && exitcode=0 || exitcode=$? + + assert_that "out" --file-is-lines \ + '=> Substituting "id" in file' \ + 'warning: [subst.mk:id] Nothing changed in "file".' \ + 'fail: [subst.mk:id] The filename pattern "file" has no effect.' \ + '*** Error code 1' \ + '' \ + 'Stop.' \ + "$make: stopped in $PWD" + assert_that "file" --file-is-lines \ + 'other' + + test_case_end +fi diff --git a/regress/infra-unittests/test.subr b/regress/infra-unittests/test.subr index afad5b8a5cf..b6fcaddd666 100644 --- a/regress/infra-unittests/test.subr +++ b/regress/infra-unittests/test.subr @@ -1,5 +1,5 @@ #! /bin/sh -# $NetBSD: test.subr,v 1.10 2020/04/26 12:46:33 rillig Exp $ +# $NetBSD: test.subr,v 1.11 2020/04/29 18:33:56 rillig Exp $ # # This file defines utilities for testing Makefile fragments and shell # programs from the pkgsrc infrastructure. While testing one part of the @@ -246,7 +246,7 @@ create_pkgsrc_file() { run_bmake() { cat <<EOF > "$tmpdir/test.subr.main.mk" -PKGSRCDIR= $relative_pkgsrcdir +PKGSRCDIR= $pkgsrcdir .PATH: $mocked_pkgsrcdir .PATH: $pkgsrcdir .include "$1" |