CVS commit: pkgsrc

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

Module Name:    pkgsrc
Committed By:   rillig
Date:           Mon May  4 21:32:48 UTC 2020

Modified Files:
        pkgsrc/mk/check: check-portability.mk check-portability.sh
            check-subr.awk
Added Files:
        pkgsrc/regress/infra-unittests: check-portability.sh

Log Message:
mk/check/check-portability: skip X.in if X is patched

A commonly occuring scenario is that a package patches the configure
script, but that the corresponding configure.in contains shell code that
is not portable.  In cases like these, configure.in is typically not used
during the build, therefore there is no need to check it for portability.

This also applies to all other combinations where a file is patched and
the corresponding file.in contains unportable shell code.


To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.15 pkgsrc/mk/check/check-portability.mk
cvs rdiff -u -r1.19 -r1.20 pkgsrc/mk/check/check-portability.sh
cvs rdiff -u -r1.3 -r1.4 pkgsrc/mk/check/check-subr.awk
cvs rdiff -u -r0 -r1.1 pkgsrc/regress/infra-unittests/check-portability.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/check/check-portability.mk
diff -u pkgsrc/mk/check/check-portability.mk:1.14 pkgsrc/mk/check/check-portability.mk:1.15
--- pkgsrc/mk/check/check-portability.mk:1.14   Fri Mar 13 08:11:36 2020
+++ pkgsrc/mk/check/check-portability.mk        Mon May  4 21:32:48 2020
@@ -1,4 +1,4 @@
-# $NetBSD: check-portability.mk,v 1.14 2020/03/13 08:11:36 rillig Exp $
+# $NetBSD: check-portability.mk,v 1.15 2020/05/04 21:32:48 rillig Exp $
 #
 # This file contains some checks that are applied to the configure
 # scripts to check for certain constructs that are known to cause
@@ -43,4 +43,5 @@ _check-portability:
        cd ${WRKSRC};                                                   \
        env     SKIP_FILTER=${CHECK_PORTABILITY_SKIP:@p@${p}) skip=yes;;@:Q} \
                PREFIX=${PREFIX}                                        \
+               PATCHDIR=${PATCHDIR}                                    \
                sh ${PKGSRCDIR}/mk/check/check-portability.sh

Index: pkgsrc/mk/check/check-portability.sh
diff -u pkgsrc/mk/check/check-portability.sh:1.19 pkgsrc/mk/check/check-portability.sh:1.20
--- pkgsrc/mk/check/check-portability.sh:1.19   Fri Mar 13 08:11:36 2020
+++ pkgsrc/mk/check/check-portability.sh        Mon May  4 21:32:48 2020
@@ -1,4 +1,4 @@
-# $NetBSD: check-portability.sh,v 1.19 2020/03/13 08:11:36 rillig Exp $
+# $NetBSD: check-portability.sh,v 1.20 2020/05/04 21:32:48 rillig Exp $
 #
 # This program checks all files in the current directory and any
 # subdirectories for portability issues that are likely to result in
@@ -34,6 +34,8 @@ check_shell() {
        fi
 }
 
+patched_files=",$(awk 'BEGIN { ORS = "," } /^\+\+\+ / { print $2 }' "$PATCHDIR"/patch-*),"
+
 find ./* -type f -print 2>/dev/null \
 | sed 's,$,_,' \
 | {
@@ -44,6 +46,11 @@ find ./* -type f -print 2>/dev/null \
 
                skip=no
                eval "case \"\$fname\" in $SKIP_FILTER *.orig) skip=yes;; esac"
+               case "$fname" in *.in)
+                       case ",$patched_files," in *,"${fname%.in}",*)
+                               skip=yes
+                       esac
+               esac
                [ $skip = no ] || continue
 
                skip_shebang_test=no

Index: pkgsrc/mk/check/check-subr.awk
diff -u pkgsrc/mk/check/check-subr.awk:1.3 pkgsrc/mk/check/check-subr.awk:1.4
--- pkgsrc/mk/check/check-subr.awk:1.3  Sun Dec 31 13:35:10 2006
+++ pkgsrc/mk/check/check-subr.awk      Mon May  4 21:32:48 2020
@@ -1,4 +1,4 @@
-# $NetBSD: check-subr.awk,v 1.3 2006/12/31 13:35:10 rillig Exp $
+# $NetBSD: check-subr.awk,v 1.4 2020/05/04 21:32:48 rillig Exp $
 #
 # This file contains functions that are used by the various awk
 # programs that check things in pkgsrc. All these programs must be
@@ -7,7 +7,7 @@
 # CK_FNAME
 #      The name of the file that is checked. Since awk interprets
 #      command line arguments in a weird way, the input file must be
-#      passed via stdin.
+#      passed via the environment.
 #
 # CK_PROGNAME
 #      The program name to be used in diagnostic messages.

Added files:

Index: pkgsrc/regress/infra-unittests/check-portability.sh
diff -u /dev/null pkgsrc/regress/infra-unittests/check-portability.sh:1.1
--- /dev/null   Mon May  4 21:32:48 2020
+++ pkgsrc/regress/infra-unittests/check-portability.sh Mon May  4 21:32:48 2020
@@ -0,0 +1,177 @@
+#! /bin/sh
+# $NetBSD: check-portability.sh,v 1.1 2020/05/04 21:32:48 rillig Exp $
+#
+# Test cases for mk/check/check-portability.*.
+#
+
+set -eu
+
+. "./test.subr"
+
+# Runs the shell program for the given file.
+check_portability_sh() {
+       env     PATCHDIR='patches' \
+               PREFIX='/nonexistent' \
+               sh "$pkgsrcdir/mk/check/check-portability.sh" \
+               1>"$tmpdir/out" 2>&1 \
+       && exitcode=0 || exitcode=$?
+}
+
+# Runs the AWK program in standalone mode for the given file.
+check_portability_awk() {
+       env     CK_FNAME="$1" \
+               CK_PROGNAME='check-portability.awk' \
+               awk     -f "$pkgsrcdir/mk/check/check-subr.awk" \
+                       -f "$pkgsrcdir/mk/check/check-portability.awk" \
+                       "$1" \
+               1>"$tmpdir/out" 2>&1 \
+       && exitcode=0 || exitcode=$?
+}
+
+test_case_set_up() {
+       rm -rf "$tmpdir/work"
+       mkdir "$tmpdir/work"
+       cd "$tmpdir/work"
+}
+
+# TODO: remove the "work/" from the tests.
+# The $tmpdir must be a bit structured:
+# $tmpdir/pkgsrc       these files override the actual pkgsrc files
+# $tmpdir/work         current working directory
+
+
+if test_case_begin "test ... = ..."; then
+
+       create_file_lines 'work/file' \
+               'if [ "$var" = value ]; then' \
+               '  ...' \
+               'elif test "$var" = value ]; then' \
+               '  ...' \
+               'fi'
+
+       check_portability_awk 'file'
+
+       assert_that 'out' --file-is-empty
+       assert_that $exitcode --equals 0
+
+       test_case_end
+fi
+
+
+if test_case_begin 'test ... == ...'; then
+
+       create_file_lines 'work/file' \
+               'if [ "$var" == value ]; then' \
+               '  ...' \
+               'elif test "$var" == value ]; then' \
+               '  ...' \
+               'fi'
+
+       check_portability_awk 'file'
+
+       create_file 'expected' <<'EOF'
+ERROR: [check-portability.awk] => Found test ... == ...:
+ERROR: [check-portability.awk] file:1: if [ "$var" == value ]; then
+ERROR: [check-portability.awk] file:3: elif test "$var" == value ]; then
+
+Explanation:
+===========================================================================
+The "test" command, as well as the "[" command, are not required to know
+the "==" operator. Only a few implementations like bash and some
+versions of ksh support it.
+
+When you run "test foo == foo" on a platform that does not support the
+"==" operator, the result will be "false" instead of "true". This can
+lead to unexpected behavior.
+
+There are two ways to fix this error message. If the file that contains
+the "test ==" is needed for building the package, you should create a
+patch for it, replacing the "==" operator with "=". If the file is not
+needed, add its name to the CHECK_PORTABILITY_SKIP variable in the
+package Makefile.
+===========================================================================
+
+EOF
+       assert_that 'out' --file-equals 'expected'
+       assert_that $exitcode --equals 1
+
+       test_case_end
+fi
+
+
+if test_case_begin 'configure patched, configure.in bad'; then
+
+       create_file_lines 'work/patches/patch-aa' \
+               '+++ configure 2020-05-04'
+       create_file_lines 'work/configure' \
+               '#! /bin/sh' \
+               'good'
+       create_file_lines 'work/configure.in' \
+               'test a == b'
+
+       check_portability_sh
+
+       assert_that "out" --file-is-empty
+       assert_that $exitcode --equals 0
+
+       test_case_end
+fi
+
+
+if test_case_begin 'configure patched and still bad'; then
+
+       create_file_lines 'work/patches/patch-aa' \
+               '+++ configure 2020-05-04'
+       create_file_lines 'work/configure' \
+               '#! /bin/sh' \
+               'test a == b'
+
+       check_portability_sh
+
+       create_file 'expected' <<'EOF'
+ERROR: [check-portability.awk] => Found test ... == ...:
+ERROR: [check-portability.awk] configure:2: test a == b
+
+Explanation:
+===========================================================================
+The "test" command, as well as the "[" command, are not required to know
+the "==" operator. Only a few implementations like bash and some
+versions of ksh support it.
+
+When you run "test foo == foo" on a platform that does not support the
+"==" operator, the result will be "false" instead of "true". This can
+lead to unexpected behavior.
+
+There are two ways to fix this error message. If the file that contains
+the "test ==" is needed for building the package, you should create a
+patch for it, replacing the "==" operator with "=". If the file is not
+needed, add its name to the CHECK_PORTABILITY_SKIP variable in the
+package Makefile.
+===========================================================================
+
+EOF
+       assert_that 'out' --file-equals 'expected'
+       assert_that $exitcode --equals 1
+
+       test_case_end
+fi
+
+
+if test_case_begin 'special characters in filenames'; then
+
+       # Ensure that the filename matching does not treat special
+       # characters as shell commands.
+
+       create_file_lines 'work/patches/patch-aa' \
+               '+++ [[[[(`" 2020-05-04'
+       create_file_lines 'work/+++ [[[[(`"' \
+               '#! /bin/sh' \
+               'test a = b'
+
+       check_portability_sh
+
+       assert_that 'out' --file-is-empty
+       assert_that $exitcode --equals 0
+
+       test_case_end
+fi