Re: [PATCH]: New hook function "atexec"

Re: [PATCH]: New hook function "atexec"

[Michael Hwang]

[-- Attachment #1: Type: text/plain, Size: 2488 bytes --]

Peter Stephenson <p.w.stephenson@ntlworld.com>wrote:

> I'm not sure where this needs to be called execcmd(), there's so much
> going on there.  The issue I'm most worried about is redirection; where
> is the output of hook functions called in the middle of a pipeline going?

atexec is triggered before redirection, so it's not a problem.

% atexec () { print -- ${(q-)@} }
% print "Testing 1 2 3" | cat > temp

The commands are printed to the screen, indicating that atexec is not affected by redirection. Opening temp reveals nothing unexpected.

> However, it also worries me that this is calling execcmd() recursively
> in strange ways never before attempted---we partly set up a command for
> execution, then call an entire arbitrary execution tree, then finish
> executing the command.  This could easily have odd effects.  I wonder if
> using execsave() and execrestore() might be sensible.

I'm not sure if it's sensible either. I know at least that I have to set "errflag = 0", as execsave doesn't take care of it.

> I'm surprised you only need to set atexec to zero in two places, but
> there may be another way of testing the conditions you want anyway.

The difficulty is figuring out whether or not the current zsh process is an asynchronous child. As far as I know, zsh doesn't keep track of whether the current process is the result of an asynchronous fork. I don't think we can eliminate the "atexec = 0" here.

Also, I was thinking that sfcontext should be set to SFC_SUBST during command substitution. That would be a way to remove an "atexec = 0".

> I don't see why you need the test for sourcelevel, which stops it
> happening in all "." files.  That might be sensible for init files, but
> otherwise it seems a bit arbitrary when other limitations might be more
> useful.  For example, I don't see anything to stop this being run for
> every individual command in every function run by the user, which could
> be a huge overhead, probably more so that "." files which tend to be
> one-off.  (I think the test for "sfcontext" happens to catch a lot of
> completion functions, however.)  I'd be tempted to add a "locallevel"
> test to exclude all function bodies.  If necessary we can think of a way
> of reactivating it in a function.

Without the test for sourcelevel, a manual "source" by the user would trigger atexec for each command in the file. The sfcontext of the body of a function is SFC_DIRECT, and hence is excluded from atexec.

Michael Hwang



      

[-- Attachment #2: Type: text/html, Size: 3007 bytes --]

Re: [PATCH]: New hook function "atexec"

[Peter Stephenson]

On Thu, 16 Jul 2009 23:27:49 -0700 (PDT)
Michael Hwang <nomex45@yahoo.com> wrote:
> I've hacked in a new hook function similar to preexec called "atexec".

That's a reasonable idea: some thoughts:

I'm not sure where this needs to be called execcmd(), there's so much
going on there.  The issue I'm most worried about is redirection; where
is the output of hook functions called in the middle of a pipeline going?

However, it also worries me that this is calling execcmd() recursively
in strange ways never before attempted---we partly set up a command for
execution, then call an entire arbitrary execution tree, then finish
executing the command.  This could easily have odd effects.  I wonder if
using execsave() and execrestore() might be sensible.

I'm surprised you only need to set atexec to zero in two places, but
there may be another way of testing the conditions you want anyway.

I don't see why you need the test for sourcelevel, which stops it
happening in all "." files.  That might be sensible for init files, but
otherwise it seems a bit arbitrary when other limitations might be more
useful.  For example, I don't see anything to stop this being run for
every individual command in every function run by the user, which could
be a huge overhead, probably more so that "." files which tend to be
one-off.  (I think the test for "sfcontext" happens to catch a lot of
completion functions, however.)  I'd be tempted to add a "locallevel"
test to exclude all function bodies.  If necessary we can think of a way
of reactivating it in a function.

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

[PATCH]: New hook function "atexec"

[Michael Hwang]

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

I've hacked in a new hook function similar to preexec called "atexec". The differences are:

1) atexec run before each command in the list/sublist.
2) The first argument is the command being run. The remaining arguments are the ones fed to the command.
3) The arguments have all substitutions made.

atexec is only run:
1) In interactive mode
2) When a command is executed by the user
3) When the command is not in the background
4) When not executing command substitution

For example, let's say I'm running xterm, and have this atexec:
atexec () { print -n -- "\e]0;${(q-)@}\a" }

When I run "make && make install" (compiling zsh, for instance), the xterm title will be "make" when executing "make". If that command is successful and moves on to "make install", the xterm's title will change to "make install".

Because atexec shows exactly what is being executed, something like "print $HOME $(( 3 + 4 ))" will give atexec arguments of "print /home/foobar 7". Globs are also resolved, which might be annoying. Moving into a folder AUTO_CD-style will give atexec an actual "cd" command.

atexec is not run for backgrounded commands in case it interferes with the output of foreground commands. atexec is not run for command substitution, because anything it prints will be output on the line editor.

I'm hoping the man documentation is clear enough.

Michael Hwang



      

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: atexec.patch --]
[-- Type: text/x-patch; name="atexec.patch", Size: 3856 bytes --]

diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo
index 4131d66..e07107d 100644
--- a/Doc/Zsh/contrib.yo
+++ b/Doc/Zsh/contrib.yo
@@ -301,8 +301,8 @@ called at the same point; these are so-called `hook functions'.
 The shell function tt(add-zsh-hook) provides a simple way of adding or
 removing functions from the array.
 
-var(hook) is one of tt(chpwd), tt(periodic), tt(precmd) or tt(preexec),
-the special functions in question.
+var(hook) is one of tt(atexec), tt(chpwd), tt(periodic), tt(precmd),
+or tt(preexec), the special functions in question.
 
 var(functions) is name of an ordinary shell function.  If no options
 are given this will be added to the array of functions to be executed.
diff --git a/Doc/Zsh/func.yo b/Doc/Zsh/func.yo
index 5f8df99..7689647 100644
--- a/Doc/Zsh/func.yo
+++ b/Doc/Zsh/func.yo
@@ -208,6 +208,15 @@ causes an immediately following tt(periodic) function not to run (though
 it may run at the next opportunity).
 
 startitem()
+findex(atexec)
+vindex(atexec_functions)
+item(tt(atexec))(
+Executed immediately before each individual command executed by the user.
+The first argument is the name of the command being executed. The remaining
+arguments are the arguments given to the command. Unlike tt(preexec), all
+substitutions have been made. The hook function will not be run for
+backgrounded commands or command substitution.
+)
 findex(chpwd)
 vindex(chpwd_functions)
 item(tt(chpwd))(
diff --git a/Src/exec.c b/Src/exec.c
index e682379..8d7c65f 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2233,6 +2233,15 @@ resolvebuiltin(const char *cmdarg, HashNode hn)
     return hn;
 }
 
+/* Controls whether or not we want to run the atexec hook
+ * function(s). For example, we don't want to run it for
+ * backgrounded commands. We also don't want it to run for
+ * command substitution, otherwise anything atexec prints
+ * will be output to the line editor.
+
+/**/
+int atexec = 1;
+
 /**/
 static void
 execcmd(Estate state, int input, int output, int how, int last1)
@@ -2742,6 +2751,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	}
 	/* pid == 0 */
 	close(synch[0]);
+	if (how & Z_ASYNC)
+	    atexec = 0;
 	flags = ((how & Z_ASYNC) ? ESUB_ASYNC : 0) | ESUB_PGRP;
 	if ((type != WC_SUBSH) && !(how & Z_ASYNC))
 	    flags |= ESUB_KEEPTRAP;
@@ -2779,6 +2790,35 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	goto err;
     }
 
+    /* Execute atexec and/or atexec_functions.
+     *
+     * Notice that we do this after prefork substitutions;
+     * this means we get to see exactly whats being executed.
+     *
+     * atexec is run when
+     * 1. The shell is interactive (though it might be useful as
+     *    a debugging tool in scripts?)
+     * 2. Command was called by user 
+     * 3. Not doing process substitution (otherwise if atexec prints
+     *    anything, it is pushed onto the command line)
+     * 4. Non-background command is executed. No point
+     *    in running atexec on something you wanted to hide. */
+    if (interact &&
+	atexec &&
+	!sourcelevel &&
+	!sfcontext &&
+	args &&
+	(getshfunc("atexec") ||
+	paramtab->getnode(paramtab, "atexec" HOOK_SUFFIX))) {
+	/* Temporarily add a node at the beginning of the args
+	 * link list to indicate we are calling atexec,
+	 * and remove it afterwards. */
+	pushnode(args, "atexec");
+	callhookfunc("atexec", args, 1, NULL);
+	uremnode(args, firstnode(args));
+	errflag = 0;
+    }
+
     /* Make a copy of stderr for xtrace output before redirecting */
     fflush(xtrerr);
     if (isset(XTRACE) && xtrerr == stderr &&
@@ -3524,6 +3564,7 @@ getoutput(char *cmd, int qt)
     child_unblock();
     zclose(pipes[0]);
     redup(pipes[1], 1);
+    atexec = 0; /* Don't run atexec. */
     entersubsh(ESUB_PGRP|ESUB_NOMONITOR);
     cmdpush(CS_CMDSUBST);
     execode(prog, 0, 1);