Subject: Re: make: implementing + command prefix
To: Simon J. Gerraty <sjg@crufty.net>
From: James Chacon <jmc@NetBSD.org>
List: tech-toolchain
Date: 05/05/2004 21:33:01
On Wed, May 05, 2004 at 06:05:20PM -0700, Simon J. Gerraty wrote:
> The posix spec says a command line prefixed with '+' should be
> executed even when -n -q or -t are given.
> 
> The patch below implements this for -n at least.  
> I took a sleazy out for -jN, ie. use CompatRunCommand when jobs
> discovers a + prefixed command and we are not otherwise going to
> execute it.  The unit test shows this works ok.

That sounds fine. There are places in the tree we explictly put make into
compat mode to avoid -j behavior. So make needing to do it internally for
some cases sounds fine.

> 
> BTW, I'd incorporate the compat test I posted earlier into
> unit-tests/posix - assuming both changes go in.

Yep

James

> 
> ? unit-tests/posix
> Index: compat.c
> ===================================================================
> RCS file: /cvsroot/src/usr.bin/make/compat.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 compat.c
> --- compat.c	10 Sep 2003 18:04:22 -0000	1.52
> +++ compat.c	6 May 2004 01:04:00 -0000
> @@ -122,9 +122,24 @@ static char 	    meta[256];
>  static GNode	    *curTarg = NILGNODE;
>  static GNode	    *ENDNode;
>  static void CompatInterrupt(int);
> -static int CompatRunCommand(ClientData, ClientData);
>  static int CompatMake(ClientData, ClientData);
>  
> +static void
> +Compat_Init(void)
> +{
> +    const char *cp;
> +
> +    Shell_Init();		/* setup default shell */
> +    
> +    for (cp = "#=|^(){};&<>*?[]:$`\\\n"; *cp != '\0'; cp++) {
> +	meta[(unsigned char) *cp] = 1;
> +    }
> +    /*
> +     * The null character serves as a sentinel in the string.
> +     */
> +    meta[0] = 1;
> +}
> +
>  /*-
>   *-----------------------------------------------------------------------
>   * CompatInterrupt --
> @@ -187,12 +202,13 @@ CompatInterrupt(int signo)
>   *
>   *-----------------------------------------------------------------------
>   */
> -static int
> +int
>  CompatRunCommand(ClientData cmdp, ClientData gnp)
>  {
>      char    	  *cmdStart;	/* Start of expanded command */
>      char 	  *cp, *bp;
>      Boolean 	  silent,   	/* Don't print command */
> +	    	  doIt,		/* Execute even if -n */
>  		  errCheck; 	/* Check errors */
>      int 	  reason;   	/* Reason for child's death */
>      int	    	  status;   	/* Description of child's death */
> @@ -217,7 +233,8 @@ CompatRunCommand(ClientData cmdp, Client
>  #endif
>      silent = gn->type & OP_SILENT;
>      errCheck = !(gn->type & OP_IGNORE);
> -
> +    doIt = FALSE;
> +    
>      cmdNode = Lst_Member (gn->commands, (ClientData)cmd);
>      cmdStart = Var_Subst (NULL, cmd, gn, FALSE);
>  
> @@ -245,11 +262,19 @@ CompatRunCommand(ClientData cmdp, Client
>  	return(0);
>      }
>  
> -    while ((*cmd == '@') || (*cmd == '-')) {
> -	if (*cmd == '@') {
> +    while ((*cmd == '@') || (*cmd == '-') || (*cmd == '+')) {
> +	switch (*cmd) {
> +	case '@':
>  	    silent = TRUE;
> -	} else {
> +	    break;
> +	case '-':
>  	    errCheck = FALSE;
> +	    break;
> +	case '+':
> +	    doIt = TRUE;
> +	    if (!meta[0])		/* we came here from jobs */
> +		Compat_Init();
> +	    break;
>  	}
>  	cmd++;
>      }
> @@ -279,7 +304,7 @@ CompatRunCommand(ClientData cmdp, Client
>       * If we're not supposed to execute any commands, this is as far as
>       * we go...
>       */
> -    if (NoExecute(gn)) {
> +    if (!doIt && NoExecute(gn)) {
>  	return (0);
>      }
>  
> @@ -596,11 +621,10 @@ cohorts:
>  void
>  Compat_Run(Lst targs)
>  {
> -    const char    *cp;	    /* Pointer to string of shell meta-characters */
>      GNode   	  *gn = NULL;/* Current root target */
>      int	    	  errors;   /* Number of targets not remade due to errors */
>  
> -    Shell_Init();		/* setup default shell */
> +    Compat_Init();
>  
>      if (signal(SIGINT, SIG_IGN) != SIG_IGN) {
>  	signal(SIGINT, CompatInterrupt);
> @@ -615,14 +639,6 @@ Compat_Run(Lst targs)
>  	signal(SIGQUIT, CompatInterrupt);
>      }
>  
> -    for (cp = "#=|^(){};&<>*?[]:$`\\\n"; *cp != '\0'; cp++) {
> -	meta[(unsigned char) *cp] = 1;
> -    }
> -    /*
> -     * The null character serves as a sentinel in the string.
> -     */
> -    meta[0] = 1;
> -
>      ENDNode = Targ_FindNode(".END", TARG_CREATE);
>      /*
>       * If the user has defined a .BEGIN target, execute the commands attached
> Index: job.c
> ===================================================================
> RCS file: /cvsroot/src/usr.bin/make/job.c,v
> retrieving revision 1.83
> diff -u -p -r1.83 job.c
> --- job.c	20 Dec 2003 00:18:22 -0000	1.83
> +++ job.c	6 May 2004 01:04:01 -0000
> @@ -704,11 +704,24 @@ JobPrintCommand(ClientData cmdp, ClientD
>      /*
>       * Check for leading @' and -'s to control echoing and error checking.
>       */
> -    while (*cmd == '@' || *cmd == '-') {
> -	if (*cmd == '@') {
> +    while (*cmd == '@' || *cmd == '-' || (*cmd == '+')) {
> +	switch (*cmd) {
> +	case '@':
>  	    shutUp = TRUE;
> -	} else {
> +	    break;
> +	case '-':
>  	    errOff = TRUE;
> +	    break;
> +	case '+':
> +	    if (noSpecials) {
> +		/*
> +		 * We're not actually executing anything...
> +		 * but this one needs to be - use compat mode just for it.
> +		 */
> +		CompatRunCommand(cmdp, (ClientData)job->node);
> +		return 0;
> +	    }
> +	    break;
>  	}
>  	cmd++;
>      }
> Index: nonints.h
> ===================================================================
> RCS file: /cvsroot/src/usr.bin/make/nonints.h,v
> retrieving revision 1.30
> diff -u -p -r1.30 nonints.h
> --- nonints.h	6 Mar 2004 03:57:07 -0000	1.30
> +++ nonints.h	6 May 2004 01:04:01 -0000
> @@ -90,6 +90,7 @@ void Arch_End(void);
>  int Arch_IsLib(GNode *);
>  
>  /* compat.c */
> +int CompatRunCommand(ClientData cmdp, ClientData gnp);
>  void Compat_Run(Lst);
>  
>  /* cond.c */
> Index: unit-tests/Makefile
> ===================================================================
> RCS file: /cvsroot/src/usr.bin/make/unit-tests/Makefile,v
> retrieving revision 1.12
> diff -u -p -r1.12 Makefile
> --- unit-tests/Makefile	8 Apr 2004 00:59:01 -0000	1.12
> +++ unit-tests/Makefile	6 May 2004 01:04:01 -0000
> @@ -23,6 +23,7 @@ SUBFILES= \
>  	modmatch \
>  	modts \
>  	modword \
> +	posix \
>  	ternary \
>  	varcmd
>  
> Index: unit-tests/test.exp
> ===================================================================
> RCS file: /cvsroot/src/usr.bin/make/unit-tests/test.exp,v
> retrieving revision 1.11
> diff -u -p -r1.11 test.exp
> --- unit-tests/test.exp	13 Apr 2004 16:06:23 -0000	1.11
> +++ unit-tests/test.exp	6 May 2004 01:04:01 -0000
> @@ -184,6 +184,21 @@ LIST:tw:C/ /,/g="one two three four five
>  LIST:tw:C/ /,/1g="one two three four five six"
>  LIST:tw:tW:C/ /,/="one,two three four five six"
>  LIST:tW:tw:C/ /,/="one two three four five six"
> +Hi this is a normal command
> +This one executes even with -n
> +another command
> +make -n
> +echo Hi this is a normal command
> +echo This one executes even with -n
> +This one executes even with -n
> +echo another command
> +make -n -j1
> +{ echo Hi this is a normal command 
> +} || exit $?
> +echo This one executes even with -n
> +This one executes even with -n
> +{ echo another command 
> +} || exit $?
>  The answer is unknown
>  The answer is unknown
>  The answer is empty
> --- /dev/null	Wed May  5 18:03:52 2004
> +++ unit-tests/posix	Wed May  5 17:35:20 2004
> @@ -0,0 +1,13 @@
> +# $Id$
> +
> +all:	hello
> +	@echo make -n
> +	@${.MAKE} -f ${MAKEFILE} -n hello
> +	@echo make -n -j1
> +	@${.MAKE} -f ${MAKEFILE} -n -j1 hello
> +
> +hello:
> +	@echo Hi this is a normal command
> +	+@echo This one executes even with -n
> +	@echo another command
> +
> 
>