diff options
| author | christos <christos@NetBSD.org> | 2014-08-23 14:50:24 +0000 |
|---|---|---|
| committer | christos <christos@NetBSD.org> | 2014-08-23 14:50:24 +0000 |
| commit | 429f047bae8aafd587708a7ae349656e30e0e018 (patch) | |
| tree | 4242510985d80a15bfece318980bd75fc7b4ac50 /usr.bin/make | |
| parent | 3acd9378e9313f3674e9e0106323e59ed24a5d27 (diff) | |
PR/49085: Jarmo Jaakkola: fix several parsing errors
Don't exit from var.c:Var_Parse() before possible modifiers are handled
on D and F modified versions of local variables. Properly expand $(?D)
and $(?F) too.
Make line continuations in rule's commands POSIX compliant.
Fix the syntax error caused by lib(member) as the last target before
a dependency operator.
Document the line continuation change in the manual page. Also talk
more about the POSIX style local variables and their modifiers.
Add tests covering the fixed problems into d_posix.mk. The test is
a known failure at the moment because of PR 49086 and PR 49092.
[XXX: unconverted tests]
Diffstat (limited to 'usr.bin/make')
| -rw-r--r-- | usr.bin/make/make.1 | 157 | ||||
| -rw-r--r-- | usr.bin/make/parse.c | 62 | ||||
| -rw-r--r-- | usr.bin/make/var.c | 66 |
3 files changed, 199 insertions, 86 deletions
diff --git a/usr.bin/make/make.1 b/usr.bin/make/make.1 index ccc48325113..c6641706286 100644 --- a/usr.bin/make/make.1 +++ b/usr.bin/make/make.1 @@ -1,4 +1,4 @@ -.\" $NetBSD: make.1,v 1.230 2014/02/15 18:55:30 sjg Exp $ +.\" $NetBSD: make.1,v 1.231 2014/08/23 14:50:24 christos Exp $ .\" .\" Copyright (c) 1990, 1993 .\" The Regents of the University of California. All rights reserved. @@ -382,8 +382,11 @@ conditional directives, for loops, and comments. In general, lines may be continued from one line to the next by ending them with a backslash .Pq Ql \e . -The trailing newline character and initial whitespace on the following -line are compressed into a single space. +For any line that is not a shell command line (i.e. it does not begin +with a tab), the backslash, the following newline character, and initial +whitespace on the following line are compressed into a single space. +On command lines the backslash and the newline are left intact and +if the following line begins with tab(s), the first one is removed. .Sh FILE DEPENDENCY SPECIFICATIONS Dependency lines consist of one or more targets, an operator, and zero or more sources. @@ -500,20 +503,52 @@ operation does not change their behavior. For example, any command which needs to use .Dq cd or -.Dq chdir , -without side-effect should be put in parenthesis: +.Dq chdir +without side-effects should be put in parenthesis so they are executed +in a subshell: .Bd -literal -offset indent avoid-chdir-side-effects: @echo Building $@ in `pwd` - @(cd ${.CURDIR} && ${.MAKE} $@) + @(cd ${.CURDIR} && ${MAKE} $@) @echo Back in `pwd` ensure-one-shell-regardless-of-mode: @echo Building $@ in `pwd`; \\ - (cd ${.CURDIR} && ${.MAKE} $@); \\ + (cd ${.CURDIR} && ${MAKE} $@); \\ echo Back in `pwd` .Ed +.Pp +The backslash and the following newline are retained in the input to +the shell, but if the next line starts with tab(s), the first one of +those is removed. +This allows you to align the commands in the rule without introducing +unwanted whitespace into the command line itself. +What happens to the backslash-newline pair is up to the shell. +The standard shell, +.Xr sh 1 , +removes them both elsewhere than in single quotes, effectively catenating +the two lines. +The following examples demonstrate escaped newlines in command lines. +.Bl -column -offset indent " echo \*qfoo" "| cd dir \\" +.It "echo-foobar:" Ta "| syntax-error:" +.It " echo \*qfoo\\" Ta "| for i in a b\\" +.It " bar\*q" Ta "| do\\" +.It Ta "| echo $i\\" +.It Ta "| done" +.El +.Pp +The first one is an unnecessarily contrived way of doing +.Bd -compact -offset indent +.Ic "echo \*qfoobar\*q" +.Ed +The second one demonstrates why the semicolon is required in many places +where in a similar looking regular shell script it wouldn't be. +After the shell has removed the backslash newline pairs, the result +would be the syntactically incorrect command +.Bd -compact -offset indent +.Ic "for i in a bdo echo $idone" +.Ed .Sh VARIABLE ASSIGNMENTS Variables in make are much like variables in the shell, and, by tradition, consist of all upper-case letters. @@ -623,14 +658,18 @@ Variables defined in the makefile or in included makefiles. .It Command line variables Variables defined as part of the command line. .It Local variables -Variables that are defined specific to a certain target. -The seven local variables are as follows: +There are seven variables that are defined specific to a certain target. +Five of these are defined in POSIX but only in their short form. +The variables are as follows: .Bl -tag -width ".ARCHIVE" .It Va .ALLSRC The list of all sources for this target; also known as .Ql Va \&\*[Gt] . +This variable is a non-POSIX extension. .It Va .ARCHIVE -The name of the archive file. +The name of the archive file; also known as +.Ql Va \&! . +This variable is a non-POSIX extension. .It Va .IMPSRC In suffix-transformation rules, the name/path of the source from which the target is to be transformed (the @@ -639,7 +678,8 @@ source); also known as .Ql Va \&\*[Lt] . It is not defined in explicit rules. .It Va .MEMBER -The name of the archive member. +The name of the archive member; also known as +.Ql Va % . .It Va .OODATE The list of sources for this target that were deemed out-of-date; also known as @@ -648,31 +688,47 @@ known as The file prefix of the target, containing only the file portion, no suffix or preceding directory components; also known as .Ql Va * . +.Pp +POSIX only requires this variable to be availabe for suffix transformation +rules, but this implementation makes it available for all targets. +If the target does not have a known suffix (see +.Ic .SUFFIXES ) , +it is equivalent to +.Ql Va .TARGET . .It Va .TARGET The name of the target; also known as .Ql Va @ . .El .Pp -The shorter forms -.Ql Va @ , +To increase readability, the shorter forms +.Ql ( Va @ , +.Ql Va \&! , +.Ql Va % , .Ql Va \&? , -.Ql Va \&\*[Lt] , -.Ql Va \&\*[Gt] , +.Ql Va \*[Lt] , +.Ql Va \*[Gt] , and -.Ql Va * -are permitted for backward -compatibility with historical makefiles and are not recommended. -The six variables -.Ql Va "@F" , -.Ql Va "@D" , -.Ql Va "\*[Lt]F" , -.Ql Va "\*[Lt]D" , -.Ql Va "*F" , +.Ql Va * ) +should only be used when compatibility with POSIX or other implementations +is desired. +For compatibility with POSIX, all of the short forms may also be written +with an appended uppercase +.Ql D +or +.Ql F , +e.g.\& +.Ql @D +or +.Ql *F . +These modified versions replace each word in the expansion with their +.Em directory part ( Ql D ) +or +.Em filename part ( Ql F ) , +and are exactly equivalent to the +.Cm \&:H and -.Ql Va "*D" -are permitted for compatibility with -.At V -makefiles and are not recommended. +.Cm \&:T +variable modifiers, respectively. .Pp Four of the local variables may be used in sources on dependency lines because they expand to the proper value for each target on the line. @@ -1052,12 +1108,26 @@ If any of the modifiers in the modifier_variable contain a dollar sign .Pq Ql $ , these must be doubled to avoid early expansion. .Pp +Variable modifiers are not POSIX compatible except for +.Ql Cm :old_string=new_string , +.Ql Cm \&:H +and +.Ql Cm \&:T . +Even these have notable caveats, see their individual descriptions. +.Pp The supported modifiers are: .Bl -tag -width EEE .It Cm \&:E Replaces each word in the variable with its suffix. .It Cm \&:H -Replaces each word in the variable with everything but the last component. +Replaces each word in the variable with everything but the last component +.Ql ( "head only" ) . +In POSIX compatible makefiles this modification is only available for local +variables and with different syntax. +It is achieved by writing the variable with an appended +.Ql D , +e.g.\& +.Ql Va @D . .It Cm \&:M Ns Ar pattern Select only those words that match .Ar pattern . @@ -1260,7 +1330,14 @@ and are subjected to variable expansion before being parsed as regular expressions. .It Cm \&:T -Replaces each word in the variable with its last component. +Replaces each word in the variable with its last component +.Ql ( "tail only" ) . +In POSIX compatible makefiles this modification is only available for local +variables and with different syntax. +It is achieved by writing the variable with an appended +.Ql F , +e.g.\& +.Ql Va @F . .It Cm \&:u Remove adjacent duplicate words (like .Xr uniq 1 ) . @@ -1283,7 +1360,7 @@ to determine is any words match "42" you need to use something like: .It Ar :old_string=new_string This is the .At V -style variable substitution. +style variable substitution, later standardized in POSIX. It must be the last modifier specified. If .Ar old_string @@ -1300,6 +1377,9 @@ is the substring of .Ar old_string to be replaced in .Ar new_string . +The special meaning of +.Ar % +is not POSIX compatible. .Pp Variable expansion occurs in the normal fashion inside both .Ar old_string @@ -1749,7 +1829,20 @@ of three. Comments begin with a hash .Pq Ql \&# character, anywhere but in a shell -command line, and continue to the end of an unescaped new line. +command line, and continue to the end of an +.Em unescaped +new line. +This can be used to a great effect to comment out continued sections of +a makefile but it can also lead to very subtle, and possibly difficult +to spot, errors for that same reason. +Please observe some actually encountered problems: +.Bl -column -offset indent "| target: source" "| target: source" \ + "| target: source" +.It "#OLD_NAME =\\" Ta "| VAR =\\" Ta "| #VAR2 =\\" +.It "NEW_NAME =\\" Ta "| foo\\" Ta "| # foo\\" +.It "foo\\" Ta "| # bar\\" Ta "| # bar\\" +.It "bar" Ta "| baz" Ta "| target: source" +.El .Sh SPECIAL SOURCES (ATTRIBUTES) .Bl -tag -width .IGNOREx .It Ic .EXEC diff --git a/usr.bin/make/parse.c b/usr.bin/make/parse.c index 35f0a3b7172..52173fd5048 100644 --- a/usr.bin/make/parse.c +++ b/usr.bin/make/parse.c @@ -1,4 +1,4 @@ -/* $NetBSD: parse.c,v 1.198 2014/07/16 19:31:11 justin Exp $ */ +/* $NetBSD: parse.c,v 1.199 2014/08/23 14:50:24 christos Exp $ */ /* * Copyright (c) 1988, 1989, 1990, 1993 @@ -69,14 +69,14 @@ */ #ifndef MAKE_NATIVE -static char rcsid[] = "$NetBSD: parse.c,v 1.198 2014/07/16 19:31:11 justin Exp $"; +static char rcsid[] = "$NetBSD: parse.c,v 1.199 2014/08/23 14:50:24 christos Exp $"; #else #include <sys/cdefs.h> #ifndef lint #if 0 static char sccsid[] = "@(#)parse.c 8.3 (Berkeley) 3/19/94"; #else -__RCSID("$NetBSD: parse.c,v 1.198 2014/07/16 19:31:11 justin Exp $"); +__RCSID("$NetBSD: parse.c,v 1.199 2014/08/23 14:50:24 christos Exp $"); #endif #endif /* not lint */ #endif @@ -1193,8 +1193,14 @@ ParseDoDependency(char *line) curTargs = Lst_Init(FALSE); + /* + * Get targets. After each iteration 'line' is reset to point + * past the word that was just read so that it points at the start of + * the next one (i.e. first non-space) or any of the preceding blanks. + */ + cp = line; do { - for (cp = line; *cp && (ParseIsEscaped(lstart, cp) || + for ( ; *cp && (ParseIsEscaped(lstart, cp) || !(isspace((unsigned char)*cp) || *cp == '!' || *cp == ':' || *cp == LPAREN)); cp++) { @@ -1205,6 +1211,8 @@ ParseDoDependency(char *line) * so we can safely advance beyond it...There should be * no errors in this, as they would have been discovered * in the initial Var_Subst and we wouldn't be here. + * [XXX] Shouldn't this be an error? I thought dynamic + * source stuff is only allowed on the source side. */ int length; void *freeIt; @@ -1218,22 +1226,18 @@ ParseDoDependency(char *line) if (!ParseIsEscaped(lstart, cp) && *cp == LPAREN) { /* - * Archives must be handled specially to make sure the OP_ARCHV - * flag is set in their 'type' field, for one thing, and because - * things like "archive(file1.o file2.o file3.o)" are permissible. - * Arch_ParseArchive will set 'line' to be the first non-blank - * after the archive-spec. It creates/finds nodes for the members - * and places them on the given list, returning SUCCESS if all - * went well and FAILURE if there was an error in the - * specification. On error, line should remain untouched. + * Archive member spec. On SUCCESS relevant archive member + * nodes are added to targets and cp has been advanced to + * the first non-blank after the spec. On FAILURE cp is not + * changed, targets might not be so lucky. */ - if (Arch_ParseArchive(&line, targets, VAR_CMD) != SUCCESS) { + cp = line; + if (Arch_ParseArchive(&cp, targets, VAR_CMD) != SUCCESS) { Parse_Error(PARSE_FATAL, - "Error in archive specification: \"%s\"", line); + "Error in archive specification: \"%s\"", cp); goto out; - } else { + } else continue; - } } savec = *cp; @@ -1436,9 +1440,9 @@ ParseDoDependency(char *line) cp++; } } - line = cp; - } while (*line && (ParseIsEscaped(lstart, line) || - ((*line != '!') && (*line != ':')))); + /* Inline assignment in case of continue statements. */ + } while (*(line = cp) != '\0' && (ParseIsEscaped(lstart, cp) || + ((*cp != '!') && (*cp != ':')))); /* * Don't need the list of target names anymore... @@ -2772,10 +2776,22 @@ ParseGetLine(int flags, int *length) continue; } - /* Escaped '\n' replace following whitespace with a single ' ' */ - while (ptr[0] == ' ' || ptr[0] == '\t') - ptr++; - ch = ' '; + /* + * Escaped '\n'. On regular lines it is replaced with a space + * along with all the leading whitespace on the next line. + * On command-lines, backslash and newline must preserved for + * the shell but if there is a leading tab on the next line + * it is removed. Weird, but this is all from POSIX. + */ + if (line[0] != '\t') { + while (ptr[0] == ' ' || ptr[0] == '\t') + ptr++; + ch = ' '; + } else { + *tp++ = '\\'; + if (ptr[0] == '\t') + ptr++; + } } /* Delete any trailing spaces - eg from empty continuations */ diff --git a/usr.bin/make/var.c b/usr.bin/make/var.c index 26e3f4bd10d..4326c402d41 100644 --- a/usr.bin/make/var.c +++ b/usr.bin/make/var.c @@ -1,4 +1,4 @@ -/* $NetBSD: var.c,v 1.186 2014/06/20 06:13:45 sjg Exp $ */ +/* $NetBSD: var.c,v 1.187 2014/08/23 14:50:24 christos Exp $ */ /* * Copyright (c) 1988, 1989, 1990, 1993 @@ -69,14 +69,14 @@ */ #ifndef MAKE_NATIVE -static char rcsid[] = "$NetBSD: var.c,v 1.186 2014/06/20 06:13:45 sjg Exp $"; +static char rcsid[] = "$NetBSD: var.c,v 1.187 2014/08/23 14:50:24 christos Exp $"; #else #include <sys/cdefs.h> #ifndef lint #if 0 static char sccsid[] = "@(#)var.c 8.3 (Berkeley) 3/19/94"; #else -__RCSID("$NetBSD: var.c,v 1.186 2014/06/20 06:13:45 sjg Exp $"); +__RCSID("$NetBSD: var.c,v 1.187 2014/08/23 14:50:24 christos Exp $"); #endif #endif /* not lint */ #endif @@ -3603,6 +3603,7 @@ Var_Parse(const char *str, GNode *ctxt, Boolean errnum, int *lengthPtr, char name[2]; *freePtr = NULL; + nstr = NULL; dynamic = FALSE; start = str; parsestate.oneBigWord = FALSE; @@ -3734,7 +3735,7 @@ Var_Parse(const char *str, GNode *ctxt, Boolean errnum, int *lengthPtr, */ if ((v == NULL) && (ctxt != VAR_CMD) && (ctxt != VAR_GLOBAL) && (vlen == 2) && (str[1] == 'F' || str[1] == 'D') && - strchr("@%*!<>", str[0]) != NULL) { + strchr("@%?*!<>", str[0]) != NULL) { /* * Well, it's local -- go look for it. */ @@ -3746,7 +3747,7 @@ Var_Parse(const char *str, GNode *ctxt, Boolean errnum, int *lengthPtr, /* * No need for nested expansion or anything, as we're * the only one who sets these things and we sure don't - * but nested invocations in them... + * put nested invocations in them... */ nstr = Buf_GetAll(&v->val, NULL); @@ -3759,13 +3760,10 @@ Var_Parse(const char *str, GNode *ctxt, Boolean errnum, int *lengthPtr, } /* * Resulting string is dynamically allocated, so - * tell caller to free it. + * tell caller to free it. Can't return yet, there + * might be modifiers. */ *freePtr = nstr; - *lengthPtr = tstr-start+1; - Buf_Destroy(&buf, TRUE); - VarFreeEnv(v, TRUE); - return nstr; } } @@ -3837,28 +3835,34 @@ Var_Parse(const char *str, GNode *ctxt, Boolean errnum, int *lengthPtr, Buf_Destroy(&buf, TRUE); } - if (v->flags & VAR_IN_USE) { - Fatal("Variable %s is recursive.", v->name); - /*NOTREACHED*/ - } else { - v->flags |= VAR_IN_USE; - } - /* - * Before doing any modification, we have to make sure the value - * has been fully expanded. If it looks like recursion might be - * necessary (there's a dollar sign somewhere in the variable's value) - * we just call Var_Subst to do any other substitutions that are - * necessary. Note that the value returned by Var_Subst will have - * been dynamically-allocated, so it will need freeing when we - * return. - */ - nstr = Buf_GetAll(&v->val, NULL); - if (strchr(nstr, '$') != NULL) { - nstr = Var_Subst(NULL, nstr, ctxt, errnum); - *freePtr = nstr; - } + if (nstr == NULL) { + /* + * D and F modified versions of the short forms of local variables + * are already expanded. Expand others. + */ + if (v->flags & VAR_IN_USE) { + Fatal("Variable %s is recursive.", v->name); + /*NOTREACHED*/ + } else { + v->flags |= VAR_IN_USE; + } + /* + * Before doing any modification, we have to make sure the value + * has been fully expanded. If it looks like recursion might be + * necessary (there's a dollar sign somewhere in the variable's value) + * we just call Var_Subst to do any other substitutions that are + * necessary. Note that the value returned by Var_Subst will have + * been dynamically-allocated, so it will need freeing when we + * return. + */ + nstr = Buf_GetAll(&v->val, NULL); + if (strchr(nstr, '$') != NULL) { + nstr = Var_Subst(NULL, nstr, ctxt, errnum); + *freePtr = nstr; + } - v->flags &= ~VAR_IN_USE; + v->flags &= ~VAR_IN_USE; + } if ((nstr != NULL) && haveModifier) { int used; |
