Re: PATCH: autoload with explicit path

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

(It's a bit late now, but that "PATH" in the subject line was a typo
because the Subject line in my email client at home is in very small
print.)

Vin is reporting failures where the symptom in both cases appears to be
not finding an autoload function when using "autoload -X" within the
function (with fpath=(.), but that aspect is probably irrelevant).

I can't reproduce this using the normal tests, but I have found a
possible cause and added a test that does fail in the same way.  This
patch fixes that failure.

The problem is the overloading of filename in struct shfunc.  If the
function was alrady loaded, which is the case if it contains an
explicit autoload -X (rather than one generated internally by looking
at the flags for an undefined function), then the filename indicates the
location of the source for the function.  If this came from a file with
an absolute path, and there was no explicit directory path in the
autoload -X statement, the file path was erroneously taken as a
directory for loading.

This adds an explicit flag to indicate filename is being used for that
purpose, unsetting it when the filename is set to the file's path.

Also be a bit more careful checking if a function wasn't let loaded when
using the new functions options.  If it's already loaded they're
irrelevant.

pws

P.S. test failure reports to the list, please, so everyone can see ---
thanks.

> From: Vin Shelton <acs@xxxxxxxxxxxxxxxxxxxx>
> To: Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
> Cc: Zsh hackers list <zsh-workers@xxxxxxx>
> Date: 11 January 2017 at 23:15
> Subject: Re: PATH: autoload with explicit path
> 
> Hi, Peter -
> 
> Thanks for this. I see two test failures:
> 
> opt/src/zsh-2017-01-11/Test/A04redirect.ztst: all tests successful.
> /opt/src/zsh-2017-01-11/Test/A05execution.ztst: starting.
> Test /opt/src/zsh-2017-01-11/Test/A05execution.ztst failed: bad status 1,
> expected 0 from:
>  unfunction functst
>  print "print Yet another version" >functst
>  functst() { autoload -X; }
>  functst
> Error output:
> (eval):1: functst: function definition file not found
> Was testing: autoloading via -X
> /opt/src/zsh-2017-01-11/Test/A05execution.ztst: test failed.
> /opt/src/zsh-2017-01-11/Test/A06assign.ztst: starting.
> 
> and
> 
> /opt/src/zsh-2017-01-11/Test/C03traps.ztst: all tests successful.
> /opt/src/zsh-2017-01-11/Test/C04funcdef.ztst: starting.
> --- /tmp/zsh.ztst.8259/ztst.out 2017-01-11 17:56:26.309239694 -0500
> +++ /tmp/zsh.ztst.8259/ztst.tout 2017-01-11 17:56:26.309239694 -0500
> @@ -1,4 +1,4 @@
> -oops was successfully autoloaded
>  oops () {
> 
> *   print oops was successfully autoloaded
> 
> *   # undefined
> *   builtin autoload -X /opt/src/zsh-2017-01-11/Test/ztst.zsh
> }
> Test /opt/src/zsh-2017-01-11/Test/C04funcdef.ztst failed: output differs
> from expected as shown above for:
> (
>  fpath=(.)
>  print "print oops was successfully autoloaded" >oops
>  oops() { eval autoload -X }
>  oops
>  which -x2 oops
> )
> Error output:
> (eval):1: oops: function definition file not found
> Was testing: autoload containing eval
> /opt/src/zsh-2017-01-11/Test/C04funcdef.ztst: test failed.
> /opt/src/zsh-2017-01-11/Test/C05debug.ztst: starting.
> /opt/src/zsh-2017-01-11/Test/C05debug.ztst: all tests successful.
> 
> I see this on a variety of linux systems. Please let me know if you need
> more details.

diff --git a/Src/builtin.c b/Src/builtin.c
index b986dd8..716ddd4 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2936,10 +2936,11 @@ check_autoload(Shfunc shf, char *name, Options ops, int func)
     {
 	return eval_autoload(shf, name, ops, func);
     }
-    if (OPT_ISSET(ops,'r') || OPT_ISSET(ops,'R'))
+    if ((OPT_ISSET(ops,'r') || OPT_ISSET(ops,'R')) &&
+	(shf->node.flags & PM_UNDEFINED))
     {
 	char *dir_path;
-	if (shf->filename) {
+	if (shf->filename && (shf->node.flags & PM_LOADDIR)) {
 	    char *spec_path[2];
 	    spec_path[0] = shf->filename;
 	    spec_path[1] = NULL;
@@ -2964,6 +2965,7 @@ check_autoload(Shfunc shf, char *name, Options ops, int func)
 		dir_path = xsymlink(dir_path, 1);
 	    }
 	    shf->filename = ztrdup(dir_path);
+	    shf->node.flags |= PM_LOADDIR;
 	    return 0;
 	}
 	if (OPT_ISSET(ops,'R')) {
@@ -3017,7 +3019,8 @@ add_autoload_function(Shfunc shf, char *funcname)
 {
     char *nam;
     if (*funcname == '/' && funcname[1] &&
-	(nam = strrchr(funcname, '/')) && nam[1]) {
+	(nam = strrchr(funcname, '/')) && nam[1] &&
+	(shf->node.flags & PM_UNDEFINED)) {
 	char *dir;
 	nam = strrchr(funcname, '/');
 	if (nam == funcname) {
@@ -3028,6 +3031,7 @@ add_autoload_function(Shfunc shf, char *funcname)
 	}
 	zsfree(shf->filename);
 	shf->filename = ztrdup(dir);
+	shf->node.flags |= PM_LOADDIR;
 	shfunctab->addnode(shfunctab, ztrdup(nam), shf);
     } else {
 	shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
@@ -3278,6 +3282,7 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	    if (*argv) {
 		zsfree(shf->filename);
 		shf->filename = ztrdup(*argv);
+		on |= PM_LOADDIR;
 	    }
 	    shf->node.flags = on;
 	    ret = eval_autoload(shf, funcname, ops, func);
diff --git a/Src/exec.c b/Src/exec.c
index 7bec7ce..68c455b 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5160,7 +5160,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
     pushheap();
 
     noaliases = (shf->node.flags & PM_UNALIASED);
-    if (shf->filename && shf->filename[0] == '/')
+    if (shf->filename && shf->filename[0] == '/' &&
+	(shf->node.flags & PM_LOADDIR))
     {
 	char *spec_path[2];
 	spec_path[0] = dupstring(shf->filename);
@@ -5203,7 +5204,7 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 		shf->funcdef = prog;
 	    else
 		shf->funcdef = dupeprog(prog, 0);
-	    shf->node.flags &= ~PM_UNDEFINED;
+	    shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR);
 	    zsfree(shf->filename);
 	    shf->filename = fname;
 	} else {
@@ -5227,7 +5228,7 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	    shf->funcdef = stripkshdef(prog, shf->node.nam);
 	else
 	    shf->funcdef = dupeprog(stripkshdef(prog, shf->node.nam), 0);
-	shf->node.flags &= ~PM_UNDEFINED;
+	shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR);
 	zsfree(shf->filename);
 	shf->filename = fname;
     }
diff --git a/Src/zsh.h b/Src/zsh.h
index 67c5a35..7d18333 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1823,6 +1823,7 @@ struct tieddata {
 
 /* Remaining flags do not correspond directly to command line arguments */
 #define PM_DONTIMPORT_SUID (1<<19) /* do not import if running setuid */
+#define PM_LOADDIR      (1<<19) /* (function) filename gives load directory */
 #define PM_SINGLE       (1<<20) /* special can only have a single instance  */
 #define PM_LOCAL	(1<<21) /* this parameter will be made local        */
 #define PM_SPECIAL	(1<<22) /* special builtin parameter                */
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index 1821b78..7100280 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -419,6 +419,16 @@
 0:autoload -dX with path
 >I have been loaded by default path.
 
+  (
+    fpath=(.)
+    print 'loadthisfunc() { autoload -X }' >loadthisfunc_sourceme
+    print 'print Function was loaded correctly.' >loadthisfunc
+    source $PWD/loadthisfunc_sourceme
+    loadthisfunc
+  )
+0: autoload -X interaction with absolute filename used for source location
+>Function was loaded correctly.
+
 %clean
 
  rm -f file.in file.out