Module Name:    src
Committed By:   rillig
Date:           Wed Nov  4 04:24:57 UTC 2020

Modified Files:
        src/distrib/sets/lists/tests: mi
        src/usr.bin/make: var.c
        src/usr.bin/make/unit-tests: Makefile
Added Files:
        src/usr.bin/make/unit-tests: cmdline-undefined.exp cmdline-undefined.mk

Log Message:
make(1): add test for undefined variables in command line arguments

The variable discardUndefined has an implicit negation in its name,
which makes it hard to understand.  Plus, most of the time it is true.
It's better to have a flag that is false most of the time and has a
positive name.

On the first attempt of inverting that variable, I stumbled upon
MainParseArgs, which initially leaves discardUndefined == FALSE, and
after handling the dashed options, sets it to TRUE.  This would make a
difference when more command line arguments would be added later via the
.MAKEFLAGS special target.

Upon further inspection, the only place where discardUndefined is used
is in VarAssign_EvalSubst in parse.c, and that place is not reachable
from any of the dashed options.  Therefore, discardUndefined could
already be set at the very beginning of MainParseArgs or even when
initializing the global variable itself, without any observable
difference.

Not even the ::= variable modifier could do anything about this since it
is not reachable from the dashed command line options as well, and in
addition, it expands its right-hand side in any case, always discarding
undefined variables.  Oh, these little inconsistencies everywhere.


To generate a diff of this commit:
cvs rdiff -u -r1.960 -r1.961 src/distrib/sets/lists/tests/mi
cvs rdiff -u -r1.655 -r1.656 src/usr.bin/make/var.c
cvs rdiff -u -r1.187 -r1.188 src/usr.bin/make/unit-tests/Makefile
cvs rdiff -u -r0 -r1.1 src/usr.bin/make/unit-tests/cmdline-undefined.exp \
    src/usr.bin/make/unit-tests/cmdline-undefined.mk

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

Modified files:

Index: src/distrib/sets/lists/tests/mi
diff -u src/distrib/sets/lists/tests/mi:1.960 src/distrib/sets/lists/tests/mi:1.961
--- src/distrib/sets/lists/tests/mi:1.960	Tue Nov  3 17:17:31 2020
+++ src/distrib/sets/lists/tests/mi	Wed Nov  4 04:24:57 2020
@@ -1,4 +1,4 @@
-# $NetBSD: mi,v 1.960 2020/11/03 17:17:31 rillig Exp $
+# $NetBSD: mi,v 1.961 2020/11/04 04:24:57 rillig Exp $
 #
 # Note: don't delete entries from here - mark them as "obsolete" instead.
 #
@@ -4819,6 +4819,8 @@
 ./usr/tests/usr.bin/make/unit-tests/cmd-errors.mk				tests-usr.bin-tests	compattestfile,atf
 ./usr/tests/usr.bin/make/unit-tests/cmd-interrupt.exp				tests-usr.bin-tests	compattestfile,atf
 ./usr/tests/usr.bin/make/unit-tests/cmd-interrupt.mk				tests-usr.bin-tests	compattestfile,atf
+./usr/tests/usr.bin/make/unit-tests/cmdline-undefined.exp			tests-usr.bin-tests	compattestfile,atf
+./usr/tests/usr.bin/make/unit-tests/cmdline-undefined.mk			tests-usr.bin-tests	compattestfile,atf
 ./usr/tests/usr.bin/make/unit-tests/cmdline.exp					tests-usr.bin-tests	compattestfile,atf
 ./usr/tests/usr.bin/make/unit-tests/cmdline.mk					tests-usr.bin-tests	compattestfile,atf
 ./usr/tests/usr.bin/make/unit-tests/comment.exp					tests-usr.bin-tests	compattestfile,atf

Index: src/usr.bin/make/var.c
diff -u src/usr.bin/make/var.c:1.655 src/usr.bin/make/var.c:1.656
--- src/usr.bin/make/var.c:1.655	Wed Nov  4 03:37:51 2020
+++ src/usr.bin/make/var.c	Wed Nov  4 04:24:57 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: var.c,v 1.655 2020/11/04 03:37:51 rillig Exp $	*/
+/*	$NetBSD: var.c,v 1.656 2020/11/04 04:24:57 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -130,7 +130,7 @@
 #include "metachar.h"
 
 /*	"@(#)var.c	8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.655 2020/11/04 03:37:51 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.656 2020/11/04 04:24:57 rillig Exp $");
 
 #define VAR_DEBUG1(fmt, arg1) DEBUG1(VAR, fmt, arg1)
 #define VAR_DEBUG2(fmt, arg1, arg2) DEBUG2(VAR, fmt, arg1, arg2)
@@ -2978,6 +2978,8 @@ ok:
     }
 
     delim = st->startc == '(' ? ')' : '}';
+    /* TODO: Add test for using the ::= modifier in a := assignment line.
+     * Probably st->eflags should be passed down without VARE_ASSIGN here. */
     res = ParseModifierPart(pp, delim, st->eflags, st, &val, NULL, NULL, NULL);
     if (res != VPR_OK)
 	return AMR_CLEANUP;

Index: src/usr.bin/make/unit-tests/Makefile
diff -u src/usr.bin/make/unit-tests/Makefile:1.187 src/usr.bin/make/unit-tests/Makefile:1.188
--- src/usr.bin/make/unit-tests/Makefile:1.187	Tue Nov  3 17:17:31 2020
+++ src/usr.bin/make/unit-tests/Makefile	Wed Nov  4 04:24:57 2020
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.187 2020/11/03 17:17:31 rillig Exp $
+# $NetBSD: Makefile,v 1.188 2020/11/04 04:24:57 rillig Exp $
 #
 # Unit tests for make(1)
 #
@@ -41,6 +41,7 @@ TESTS+=		cmd-errors
 TESTS+=		cmd-errors-lint
 TESTS+=		cmd-interrupt
 TESTS+=		cmdline
+TESTS+=		cmdline-undefined
 TESTS+=		comment
 TESTS+=		cond-cmp-numeric
 TESTS+=		cond-cmp-numeric-eq

Added files:

Index: src/usr.bin/make/unit-tests/cmdline-undefined.exp
diff -u /dev/null src/usr.bin/make/unit-tests/cmdline-undefined.exp:1.1
--- /dev/null	Wed Nov  4 04:24:57 2020
+++ src/usr.bin/make/unit-tests/cmdline-undefined.exp	Wed Nov  4 04:24:57 2020
@@ -0,0 +1,17 @@
+The = assignment operator
+make: "cmdline-undefined.mk" line 27: From the command line: Undefined is .
+make: "cmdline-undefined.mk" line 28: From .MAKEFLAGS '=': Undefined is .
+make: "cmdline-undefined.mk" line 29: From .MAKEFLAGS ':=': Undefined is .
+make: "cmdline-undefined.mk" line 33: From the command line: Undefined is now defined.
+make: "cmdline-undefined.mk" line 34: From .MAKEFLAGS '=': Undefined is now defined.
+make: "cmdline-undefined.mk" line 35: From .MAKEFLAGS ':=': Undefined is now defined.
+
+The := assignment operator
+make: "cmdline-undefined.mk" line 27: From the command line: Undefined is .
+make: "cmdline-undefined.mk" line 28: From .MAKEFLAGS '=': Undefined is .
+make: "cmdline-undefined.mk" line 29: From .MAKEFLAGS ':=': Undefined is .
+make: "cmdline-undefined.mk" line 33: From the command line: Undefined is now defined.
+make: "cmdline-undefined.mk" line 34: From .MAKEFLAGS '=': Undefined is now defined.
+make: "cmdline-undefined.mk" line 35: From .MAKEFLAGS ':=': Undefined is now defined.
+
+exit status 0
Index: src/usr.bin/make/unit-tests/cmdline-undefined.mk
diff -u /dev/null src/usr.bin/make/unit-tests/cmdline-undefined.mk:1.1
--- /dev/null	Wed Nov  4 04:24:57 2020
+++ src/usr.bin/make/unit-tests/cmdline-undefined.mk	Wed Nov  4 04:24:57 2020
@@ -0,0 +1,40 @@
+# $NetBSD: cmdline-undefined.mk,v 1.1 2020/11/04 04:24:57 rillig Exp $
+#
+# Tests for undefined variable expressions in the command line.
+
+all:
+	# When the command line is parsed during the initial call of
+	# MainParseArgs, discardUndefined is still FALSE, therefore preserving
+	# undefined subexpressions.  This makes sense because at that early
+	# stage, almost no variables are defined.  On the other hand, the '='
+	# assignment operator does not expand its right-hand side anyway.
+	@echo 'The = assignment operator'
+	@${.MAKE} -f ${MAKEFILE} print-undefined \
+		CMDLINE='Undefined is $${UNDEFINED}.'
+	@echo
+
+	# The interesting case is using the ':=' assignment operator, which
+	# expands its right-hand side.  But only those variables that are
+	# defined.
+	@echo 'The := assignment operator'
+	@${.MAKE} -f ${MAKEFILE} print-undefined \
+		CMDLINE:='Undefined is $${UNDEFINED}.'
+	@echo
+
+.if make(print-undefined)
+
+.MAKEFLAGS: MAKEFLAGS_ASSIGN='Undefined is $${UNDEFINED}.'
+.MAKEFLAGS: MAKEFLAGS_SUBST:='Undefined is $${UNDEFINED}.'
+
+.info From the command line: ${CMDLINE}
+.info From .MAKEFLAGS '=': ${MAKEFLAGS_ASSIGN}
+.info From .MAKEFLAGS ':=': ${MAKEFLAGS_SUBST}
+
+UNDEFINED?=	now defined
+
+.info From the command line: ${CMDLINE}
+.info From .MAKEFLAGS '=': ${MAKEFLAGS_ASSIGN}
+.info From .MAKEFLAGS ':=': ${MAKEFLAGS_SUBST}
+
+print-undefined:
+.endif

Reply via email to