zsh-workers
 help / color / mirror / code / Atom feed
* RE: PATH: autoload with explicit path
@ 2017-01-27 15:12 ` Sebastian Gniazdowski
  2017-01-27 16:24   ` Peter Stephenson
  0 siblings, 1 reply; 35+ messages in thread
From: Sebastian Gniazdowski @ 2017-01-27 15:12 UTC (permalink / raw)
  To: zsh-workers

On 11 December 2016 at 23:18, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> Then I found it was pretty much as easy (easier, actually, since fewer
> special cases) to implement this generally:
>
> autoload -Uz /path/to/myfunc
>
> defines myfunc to be found in the directory /path/to by reusing the
> filename element of the shfunc structure that's currently unused at this
> stage.

I do the non-fpath autoloading by local FPATH parameter. There's
drawback: if autoloaded function does further autoloading of other
function, then the earlier supplied local FPATH parameter isn't active
anymore.

So it's not possible to have /home/user/functions/{fun1,fun2}, autoload
fun1, and have fun2 available for fun1. Now, with the new feature,
directory path can be attached to function and if it calls autoload
itself, then the attached directory can be examined before FPATH. Could
this be added?

There are variations possible in this, whether it should be
attached-path that overloads FPATH, or FPATH to overload attached-path.
I think that a straightforward way would be to promote attached-path
(examine it first), to allow basic /home/user/functions/{fun1,fun2}
organization of functions, which doesn't need much head scratching, it's
selecting functions "in-package", easy to understand. Having FPATH
examined first would allow loading of patched fun2 from regular fpath
dir, but this is inconsistent – if one would want to have patched fun1
in FPATH, then he would need to drop the path argument from autoload,
and copy fun2.

 -- 
  Sebastian Gniazdowski
  psprint2@fastmail.com


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-27 15:12 ` PATH: autoload with explicit path Sebastian Gniazdowski
@ 2017-01-27 16:24   ` Peter Stephenson
  2017-01-27 18:40     ` Sebastian Gniazdowski
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Stephenson @ 2017-01-27 16:24 UTC (permalink / raw)
  To: zsh-workers

On Fri, 27 Jan 2017 07:12:59 -0800
Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> I do the non-fpath autoloading by local FPATH parameter. There's
> drawback: if autoloaded function does further autoloading of other
> function, then the earlier supplied local FPATH parameter isn't active
> anymore.
> 
> So it's not possible to have /home/user/functions/{fun1,fun2}, autoload
> fun1, and have fun2 available for fun1. Now, with the new feature,
> directory path can be attached to function and if it calls autoload
> itself, then the attached directory can be examined before FPATH.

You're worried about something like this?


# set up function
local path_that_only_appears_here=/foo/bar
autoload -Uz $path_that_only_appears_here/fun1


# definition of fun1
autoload -Uz <what-goes-here?>/fun2
fun2


You can already do this using funcsourcetrace from zsh/parameter:
${funcsourcetrace[1]%%:<->} is the source file for the immediately
enclosing scope, so take the directory part of that and you've
got what you want (being careful if you happen to have evals
or anonymous functions which generate their own scopes).

It's a bit limiting you can't get that information for any function,
since it's alrady available internally in the shfunc structure.  I'd be
happy to add an associative array in zsh/parameter parallel to
$functions, say $function_sources, that gives the information for all
functions.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-27 16:24   ` Peter Stephenson
@ 2017-01-27 18:40     ` Sebastian Gniazdowski
  2017-01-27 18:44       ` Peter Stephenson
  0 siblings, 1 reply; 35+ messages in thread
From: Sebastian Gniazdowski @ 2017-01-27 18:40 UTC (permalink / raw)
  To: zsh-workers

On Fri, Jan 27, 2017, at 08:24 AM, Peter Stephenson wrote:
> On Fri, 27 Jan 2017 07:12:59 -0800
> Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> > I do the non-fpath autoloading by local FPATH parameter. There's
> > drawback: if autoloaded function does further autoloading of other
> > function, then the earlier supplied local FPATH parameter isn't active
> > anymore.
> > 
> > So it's not possible to have /home/user/functions/{fun1,fun2}, autoload
> > fun1, and have fun2 available for fun1. Now, with the new feature,
> > directory path can be attached to function and if it calls autoload
> > itself, then the attached directory can be examined before FPATH.
> 
> You're worried about something like this?
> 
> 
> # set up function
> local path_that_only_appears_here=/foo/bar
> autoload -Uz $path_that_only_appears_here/fun1
> 
> 
> # definition of fun1
> autoload -Uz <what-goes-here?>/fun2
> fun2
> 
> 
> You can already do this using funcsourcetrace from zsh/parameter:
> ${funcsourcetrace[1]%%:<->} is the source file for the immediately
> enclosing scope, so take the directory part of that and you've
> got what you want (being careful if you happen to have evals
> or anonymous functions which generate their own scopes).

Internally via autoload it will be easy to create custom versions of
multiple functions. For example calendar does:

autoload -Uz calendar_{add,parse,read,scandate,show,lockfiles}

With the requested feature, I can copy any subset of above functions to
/home/user/functions, and "autoload -Uz /home/user/functions/calendar"
will pull them. This is also possible by prepending FPATH with
/home/user/functions, true. But deciding whether I want to pull in
custom calendar or not via FPATH management or by removing calendar*
files is hard. The new feature will be very convenient if user will have
multiple directories, for example a second one /home/user/new_functions.
To choose from where to pull in the calendar will be to simply change
path to it in autoload call.

So in .zshrc there will be:

# My custom calendar
autoload -Uz /home/user/functions/calendar

and that's it, no FPATH management or removing files from
/home/user/functions (to revert to official calendar, if the directory
is in FPATH).

> It's a bit limiting you can't get that information for any function,
> since it's alrady available internally in the shfunc structure.  I'd be
> happy to add an associative array in zsh/parameter parallel to
> $functions, say $function_sources, that gives the information for all
> functions.

Sounds nice, though it's nice that current implementation of the
absolute-path feature uses pointers to paths to limit memory usage, a
new parameter would probably spend that.

-- 
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-27 18:40     ` Sebastian Gniazdowski
@ 2017-01-27 18:44       ` Peter Stephenson
  2017-01-27 19:00         ` Sebastian Gniazdowski
  2017-01-28 18:05         ` Bart Schaefer
  0 siblings, 2 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-27 18:44 UTC (permalink / raw)
  To: zsh-workers

On Fri, 27 Jan 2017 10:40:15 -0800
Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> Internally via autoload it will be easy to create custom versions of
> multiple functions. For example calendar does:
> 
> autoload -Uz calendar_{add,parse,read,scandate,show,lockfiles}
> 
> With the requested feature

I'm still not clear if your "requested feature" is what I'm talking
about or not, i.e. what you can do already with $funcsourcetrace.

> > It's a bit limiting you can't get that information for any function,
> > since it's alrady available internally in the shfunc structure.  I'd be
> > happy to add an associative array in zsh/parameter parallel to
> > $functions, say $function_sources, that gives the information for all
> > functions.
> 
> Sounds nice, though it's nice that current implementation of the
> absolute-path feature uses pointers to paths to limit memory usage, a
> new parameter would probably spend that.

The new parameter would only look up the internal information when you
refer to it, there's nothing permanent additional to be stored.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-27 18:44       ` Peter Stephenson
@ 2017-01-27 19:00         ` Sebastian Gniazdowski
  2017-01-28 18:05         ` Bart Schaefer
  1 sibling, 0 replies; 35+ messages in thread
From: Sebastian Gniazdowski @ 2017-01-27 19:00 UTC (permalink / raw)
  To: zsh-workers

On Fri, Jan 27, 2017, at 10:44 AM, Peter Stephenson wrote:
> On Fri, 27 Jan 2017 10:40:15 -0800
> Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> > Internally via autoload it will be easy to create custom versions of
> > multiple functions. For example calendar does:
> > 
> > autoload -Uz calendar_{add,parse,read,scandate,show,lockfiles}
> > 
> > With the requested feature
> 
> I'm still not clear if your "requested feature" is what I'm talking
> about or not, i.e. what you can do already with $funcsourcetrace.

We talk about the same thing, though I want to remove requirement of
using absolute paths in any function files. You wrote:

> # definition of fun1
> autoload -Uz <what-goes-here?>/fun2
> fun2

The point is to make <what-goes-here> not needed at all. The path is to
be taken from "dircache" (that's the name used in previous messages).
Autoload should use dircache of fun1 to find fun2, then follow to
process fpath. There can be an option to enable/disable this.

I've described how flexible and clean would be management of custom
version of functions this way. It's like having packages of software
instead of functions. Requiring the <what-goes-here> part will make this
difficult.

> > Sounds nice, though it's nice that current implementation of the
> > absolute-path feature uses pointers to paths to limit memory usage, a
> > new parameter would probably spend that.
> 
> The new parameter would only look up the internal information when you
> refer to it, there's nothing permanent additional to be stored.

Thought about it, it's a cool approach.

-- 
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-27 18:44       ` Peter Stephenson
  2017-01-27 19:00         ` Sebastian Gniazdowski
@ 2017-01-28 18:05         ` Bart Schaefer
  2017-01-28 19:12           ` Peter Stephenson
  1 sibling, 1 reply; 35+ messages in thread
From: Bart Schaefer @ 2017-01-28 18:05 UTC (permalink / raw)
  To: zsh-workers

On Fri, 27 Jan 2017, Peter Stephenson wrote:

> I'm still not clear if your "requested feature" is what I'm talking
> about or not, i.e. what you can do already with $funcsourcetrace.

It's not so much about what Sebastian can do as it is about what he has
to rely upon other function writers to do.

That is, if I'm in the habit of writing "autoload thing" in the bootstrap
function for a library of things, but Sebastian wants to put my entire
thing-library in a place outside of $fpath, he can't do so without first
editing all my "autoload"s to add a full path.

If every autoload were relative to the absolute-path information of the
calling function context, Sebastian's plan would work without needing
to edit anything, provided that he autoloads the bootstrap function with
an absolute path.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-28 18:05         ` Bart Schaefer
@ 2017-01-28 19:12           ` Peter Stephenson
  2017-01-28 19:45             ` Bart Schaefer
  2017-01-29 12:27             ` Sebastian Gniazdowski
  0 siblings, 2 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-28 19:12 UTC (permalink / raw)
  To: zsh-workers

On Sat, 28 Jan 2017 10:05:46 -0800 (PST)
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Fri, 27 Jan 2017, Peter Stephenson wrote:
> 
> > I'm still not clear if your "requested feature" is what I'm talking
> > about or not, i.e. what you can do already with $funcsourcetrace.
> 
> It's not so much about what Sebastian can do as it is about what he has
> to rely upon other function writers to do.
> 
> That is, if I'm in the habit of writing "autoload thing" in the bootstrap
> function for a library of things, but Sebastian wants to put my entire
> thing-library in a place outside of $fpath, he can't do so without first
> editing all my "autoload"s to add a full path.
> 
> If every autoload were relative to the absolute-path information of the
> calling function context, Sebastian's plan would work without needing
> to edit anything, provided that he autoloads the bootstrap function with
> an absolute path.

I'm afraid this is just confusing matters.  Sebastian was asking for a
new option.  There's no question of changing the default behaviour to
prevent any files needing editing, that just screws up the current fpath
way of looking things up.  The question is how to get the function to
use the right directory, not by default, but using information that's
available from the current context.

(It's not entirely clear to me why you can't just "autoload
/wherever/it/is/*" to pick up all the files in the first place, avoiding
any additional autoload, but there are enough possible ways of arranging
a function suite I don't necessarily need to understand that bit.)

Instead of the option, I'm pointing out that using the absolute path
mechanism and the ability to find the path of the function does exactly
the same thing, so yet more potentially confusing (certainly to me)
options aren't actually getting you anything further.

The following patch means that

zmodload zsh/parameter
autoload -Uz ${functions_source[func]:h}/otherfunc

works, further giving you the power to decide where relative to the
source of "func" the code you want is to arbitrary complexity.
($functions_source is effectively a trick to access existing information;
there is no additional storage demand beyond the addition to the code
itself.  It's just a generalisation of the option Sebastian wanted that
presents path information in parameters rather than hiding it.)

I've also made it turn the directory into an absolute one when it's
loaded, so that if the function was found by e.g. fpath=(.) its
directory is recorded based on $PWD.  That's an optional extra, but I
presume it's more useful that being told the directory of a function's
source was ".". The resolution already happens for autoload -r, and you might
imagine that loading the file should resolve the path in the same way,
so I think it's natural anyway.

As this does everything necessary, I will not be adding any new options,
and I've done quite enough on this feature, but if you can argue among
yourselves for something else that doesn't yet work, feel free to go
ahead without me.

diff --git a/Doc/Zsh/mod_parameter.yo b/Doc/Zsh/mod_parameter.yo
index 3d260f8..942e4c5 100644
--- a/Doc/Zsh/mod_parameter.yo
+++ b/Doc/Zsh/mod_parameter.yo
@@ -37,6 +37,30 @@ vindex(dis_functions)
 item(tt(dis_functions))(
 Like tt(functions) but for disabled functions.
 )
+vindex(functions_source)
+item(tt(functions_source))(
+This readonly associative array maps names of enabled functions to the
+name of the file containing the source of the function.
+
+For an autoloaded function that has already been loaded, or marked for
+autoload with an absolute path, or that has had its path resolved with
+`tt(functions -r)', this is the file found for autoloading, resolved
+to an absolute path.
+
+For a function defined within the body of a script or sourced file,
+this is the name of that file.  In this case, this is the exact path
+originally used to that file, which may be a relative path.
+
+For any other function, including any defined at an interactive prompt or
+an autoload function whose path has not yet been resolved, this is
+the empty string.  However, the hash element is reported as defined
+just so long as the function is present:  the keys to this hash are
+the same as those to tt($funcions).
+)
+vindex(dis_functions_source)
+item(tt(dis_functions_source))(
+Like tt(functions_source) but for disabled functions.
+)
 vindex(builtins)
 item(tt(builtins))(
 This associative array gives information about the builtin commands
@@ -202,10 +226,13 @@ defined.  The line number is the line where the `tt(function) var(name)'
 or `var(name) tt(LPAR()RPAR())' started.  In the case of an autoloaded
 function  the line number is reported as zero.
 The format of each element is var(filename)tt(:)var(lineno).
+
 For functions autoloaded from a file in native zsh format, where only the
 body of the function occurs in the file, or for files that have been
 executed by the tt(source) or `tt(.)' builtins, the trace information is
-shown as var(filename)tt(:)var(0), since the entire file is the definition.
+shown as var(filename)tt(:)var(0), since the entire file is the
+definition.  The source file name is resolved to an absolute path when
+the function is loaded or the path to it otherwise resolved.
 
 Most users will be interested in the information in the
 tt(funcfiletrace) array instead.
diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c
index 98bcaba..6e62287 100644
--- a/Src/Modules/parameter.c
+++ b/Src/Modules/parameter.c
@@ -515,6 +515,98 @@ scanpmdisfunctions(HashTable ht, ScanFunc func, int flags)
     scanfunctions(ht, func, flags, DISABLED);
 }
 
+/* Functions for the functions_source special parameter. */
+
+/* Retrieve the source file for a function by explicit name */
+
+/**/
+static HashNode
+getfunction_source(UNUSED(HashTable ht), const char *name, int dis)
+{
+    Shfunc shf;
+    Param pm = NULL;
+
+    pm = (Param) hcalloc(sizeof(struct param));
+    pm->node.nam = dupstring(name);
+    pm->node.flags = PM_SCALAR|PM_READONLY;
+    pm->gsu.s = dis ? &pmdisfunction_gsu :  &pmfunction_gsu;
+
+    if ((shf = (Shfunc) shfunctab->getnode2(shfunctab, name)) &&
+	(dis ? (shf->node.flags & DISABLED) : !(shf->node.flags & DISABLED))) {
+	pm->u.str = getshfuncfile(shf);
+	if (!pm->u.str)
+	    pm->u.str = dupstring("");
+    }
+    return &pm->node;
+}
+
+/* Retrieve the source file for functions by scanning the table */
+
+/**/
+static void
+scanfunctions_source(UNUSED(HashTable ht), ScanFunc func, int flags, int dis)
+{
+    struct param pm;
+    int i;
+    HashNode hn;
+
+    memset((void *)&pm, 0, sizeof(struct param));
+    pm.node.flags = PM_SCALAR|PM_READONLY;
+    pm.gsu.s = dis ? &pmdisfunction_gsu : &pmfunction_gsu;
+
+    for (i = 0; i < shfunctab->hsize; i++) {
+	for (hn = shfunctab->nodes[i]; hn; hn = hn->next) {
+	    if (dis ? (hn->flags & DISABLED) : !(hn->flags & DISABLED)) {
+		pm.node.nam = hn->nam;
+		if (func != scancountparams &&
+		    ((flags & (SCANPM_WANTVALS|SCANPM_MATCHVAL)) ||
+		     !(flags & SCANPM_WANTKEYS))) {
+		    pm.u.str = getshfuncfile((Shfunc)hn);
+		    if (!pm.u.str)
+			pm.u.str = dupstring("");
+		}
+		func(&pm.node, flags);
+	    }
+	}
+    }
+}
+
+/* Param table entry for retrieving functions_source element */
+
+/**/
+static HashNode
+getpmfunction_source(HashTable ht, const char *name)
+{
+    return getfunction_source(ht, name, 0);
+}
+
+/* Param table entry for retrieving ds_functions_source element */
+
+/**/
+static HashNode
+getpmdisfunction_source(HashTable ht, const char *name)
+{
+    return getfunction_source(ht, name, 1);
+}
+
+/* Param table entry for scanning functions_source table */
+
+/**/
+static void
+scanpmfunction_source(HashTable ht, ScanFunc func, int flags)
+{
+    scanfunctions_source(ht, func, flags, 0);
+}
+
+/* Param table entry for scanning dis_functions_source table */
+
+/**/
+static void
+scanpmdisfunction_source(HashTable ht, ScanFunc func, int flags)
+{
+    scanfunctions_source(ht, func, flags, 1);
+}
+
 /* Functions for the funcstack special parameter. */
 
 /**/
@@ -2095,6 +2187,8 @@ static struct paramdef partab[] = {
 	    NULL, getpmdisbuiltin, scanpmdisbuiltins),
     SPECIALPMDEF("dis_functions", 0, 
 	    &pmdisfunctions_gsu, getpmdisfunction, scanpmdisfunctions),
+    SPECIALPMDEF("dis_functions_source", PM_READONLY, NULL,
+		 getpmdisfunction_source, scanpmdisfunction_source),
     SPECIALPMDEF("dis_galiases", 0,
 	    &pmdisgaliases_gsu, getpmdisgalias, scanpmdisgaliases),
     SPECIALPMDEF("dis_patchars", PM_ARRAY|PM_READONLY,
@@ -2111,6 +2205,8 @@ static struct paramdef partab[] = {
 	    &funcstack_gsu, NULL, NULL),
     SPECIALPMDEF("functions", 0, &pmfunctions_gsu, getpmfunction,
 		 scanpmfunctions),
+    SPECIALPMDEF("functions_source", PM_READONLY, NULL,
+		 getpmfunction_source, scanpmfunction_source),
     SPECIALPMDEF("functrace", PM_ARRAY|PM_READONLY,
 	    &functrace_gsu, NULL, NULL),
     SPECIALPMDEF("galiases", 0,
diff --git a/Src/hashtable.c b/Src/hashtable.c
index 1f2789d..8987c85 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -1566,6 +1566,15 @@ dircache_set(char **name, char *value)
 	*name = NULL;
     } else {
 	/*
+	 * As the function path has been resolved to a particular
+	 * location, we'll store it as an absolute path.
+	 */
+	if (*value != '/') {
+	    value = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP),
+			     "/", value);
+	    value = xsymlink(value, 1);
+	}
+	/*
 	 * We'll maintain the cache at exactly the right size rather
 	 * than overallocating.  The rationale here is that typically
 	 * we'll get a lot of functions in a small number of directories
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index 5edbe26..176841d 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -325,10 +325,10 @@
     printf '%s\n' 'oops(){}' 'ninjas-earring(){}' 'oops "$@"' >oops
     autoload oops
     oops
-    whence -v oops
+    whence -v oops | sed -e "s%$PWD%CURDIR%"
   )
 0:whence -v of zsh-style autoload
->oops is a shell function from ./oops
+>oops is a shell function from CURDIR/oops
 
   (
     fpath=(.)
diff --git a/Test/V06parameter.ztst b/Test/V06parameter.ztst
index c7df35d..c2a2a4d 100644
--- a/Test/V06parameter.ztst
+++ b/Test/V06parameter.ztst
@@ -1,15 +1,15 @@
 %test
 
   print 'print In sourced file
-  print $LINENO + $functrace + $funcsourcetrace
+  print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
   ' >sourcedfile
   print -r -- 'print Started functrace.zsh
   module_path=(./Modules)
-  print $LINENO + $functrace + $funcsourcetrace
+  print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
   :
   fn() {
     print Inside function $0
-    print $LINENO + $functrace + $funcsourcetrace
+    print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
   }
   :
   fn
@@ -17,7 +17,7 @@
   fpath=(. $fpath)
   :
   echo '\''print Inside $0
-    print $LINENO + $functrace + $funcsourcetrace
+    print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
   '\'' >autofn
   :
   autoload autofn
@@ -32,9 +32,9 @@
 >Inside function fn
 >2 + ./functrace.zsh:10 + ./functrace.zsh:5
 >Inside autofn
->2 + ./functrace.zsh:20 + ./autofn:0
+>2 + ./functrace.zsh:20 + CURDIR/autofn:0
 >Inside autofn
->2 + ./functrace.zsh:21 + ./autofn:0
+>2 + ./functrace.zsh:21 + CURDIR/autofn:0
 >In sourced file
 >2 + ./functrace.zsh:22 + ./sourcedfile:0
 
@@ -66,6 +66,19 @@
 >./rocky3.zsh:13 (eval):2
 >./rocky3.zsh:14 ./rocky3.zsh:14
 
+  (
+    fpath=($PWD)
+    print "print I have been autoloaded" >myfunc
+    autoload $PWD/myfunc
+    print ${functions_source[myfunc]/#$PWD/CURDIR}
+    myfunc
+    print ${functions_source[myfunc]/#$PWD/CURDIR}
+  )
+0: $functions_source
+>CURDIR/myfunc
+>I have been autoloaded
+>CURDIR/myfunc
+
 %clean
 
- rm -f autofn functrace.zsh rocky3.zsh sourcedfile
+ rm -f autofn functrace.zsh rocky3.zsh sourcedfile myfunc


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-28 19:12           ` Peter Stephenson
@ 2017-01-28 19:45             ` Bart Schaefer
  2017-01-28 19:56               ` Peter Stephenson
  2017-01-29 12:27             ` Sebastian Gniazdowski
  1 sibling, 1 reply; 35+ messages in thread
From: Bart Schaefer @ 2017-01-28 19:45 UTC (permalink / raw)
  To: zsh-workers

On Sat, 28 Jan 2017, Peter Stephenson wrote:

> On Sat, 28 Jan 2017 10:05:46 -0800 (PST)
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > If every autoload were relative to the absolute-path information of the
> > calling function context, Sebastian's plan would work without needing
> > to edit anything, provided that he autoloads the bootstrap function with
> > an absolute path.
>
> I'm afraid this is just confusing matters.  Sebastian was asking for a
> new option.

Well yes, the option would be on the autoload of the bootstrap function,
i.e.,

	autoload -P /some/hard/path/foo

where "foo" in turn calls "autoload bar" would cause the autoload of bar
to inherit /some/hard/path from foo.

> There's no question of changing the default behaviour

Indeed not.


("-P" chosen for example only and entirely without reference to whether it
conflicts with some other use of -P in typeset/functions/etc.)


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-28 19:45             ` Bart Schaefer
@ 2017-01-28 19:56               ` Peter Stephenson
  2017-01-28 20:37                 ` Sebastian Gniazdowski
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Stephenson @ 2017-01-28 19:56 UTC (permalink / raw)
  To: zsh-workers

On Sat, 28 Jan 2017 11:45:36 -0800 (PST)
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Well yes, the option would be on the autoload of the bootstrap function,
> i.e.,
> 
> 	autoload -P /some/hard/path/foo
> 
> where "foo" in turn calls "autoload bar" would cause the autoload of bar
> to inherit /some/hard/path from foo.

Oh.  That just doubles my suspicion that autoloading bar at the point
where you autoload foo is the right thing to do and anything else is
just getting you into the most horrible tangle.

Well, if you think you can implement that without getting into a tangle,
and people will actually use it, go ahead.  I'm 99% certain I'd just get
confused / I will implement it and people will just immediately demand
something completely different without even pausing to think / it will
instantly change into something even more bizarre and difficult to
understand.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-28 19:56               ` Peter Stephenson
@ 2017-01-28 20:37                 ` Sebastian Gniazdowski
  0 siblings, 0 replies; 35+ messages in thread
From: Sebastian Gniazdowski @ 2017-01-28 20:37 UTC (permalink / raw)
  To: zsh-workers

On Sat, Jan 28, 2017, at 11:56 AM, Peter Stephenson wrote:
> On Sat, 28 Jan 2017 11:45:36 -0800 (PST)
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > Well yes, the option would be on the autoload of the bootstrap function,
> > i.e.,
> > 
> > 	autoload -P /some/hard/path/foo
> > 
> > where "foo" in turn calls "autoload bar" would cause the autoload of bar
> > to inherit /some/hard/path from foo.
> 
> Oh.  That just doubles my suspicion that autoloading bar at the point
> where you autoload foo is the right thing to do and anything else is
> just getting you into the most horrible tangle.

I could copy calendar to /some/hard/path, modify and load it without
daunting task of altering FPATH. It's like a software package. This
simple approach shows that this isn't something really confusing.

For example, I could have in fpath:

/home/user/functions
/usr/local/share/zsh/site-functions
/usr/local/share/zsh/5.2-dev-2/functions
/home/user/functions2

Now, if I invoke "autoload calendar", from where it will be loaded?
First containing directory.

With the new functionality, I could load from anywhere I would want –
this is simpler than maintaining proper FPATH:

autoload -P /home/user/functions2/calendar
autoload -P /usr/local/share/zsh/site-functions/calendar
etc.

Such operation of using absolute paths is zshrc-like, a final stage
where instead of generalizing, we are defining. The rest is still
generalized – all paths in function files are still general. Only the
top (zshrc) is specific.

I think that this will make Zsh so much flexible that setups involving
not typical 2-3 function directories
(FPATH=my-own-functions:Zsh-default-functions), but say 5 paths will get
common – /home/.zsh/calendar1:/home/.zsh/calendar2 – doing this with
FPATH is rather impossible. With FPATH it's natural to add just 1 or 2
additional layers on top of /usr/local/share/zsh/5.2/functions, adding
more is just too demanding.

-- 
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-28 19:12           ` Peter Stephenson
  2017-01-28 19:45             ` Bart Schaefer
@ 2017-01-29 12:27             ` Sebastian Gniazdowski
  2017-01-29 16:11               ` Bart Schaefer
  1 sibling, 1 reply; 35+ messages in thread
From: Sebastian Gniazdowski @ 2017-01-29 12:27 UTC (permalink / raw)
  To: zsh-workers

On Sat, Jan 28, 2017, at 11:12 AM, Peter Stephenson wrote:
> As this does everything necessary, I will not be adding any new options,
> and I've done quite enough on this feature, but if you can argue among
> yourselves for something else that doesn't yet work, feel free to go
> ahead without me.

The new feature (absolute paths) is a great discovery. It's a way to
avoid FPATH. Fpath is abstract mechanism. System of layers. User is to
imagine the layers and establish what will be loaded. It's a full-blown
abstract system – actually nothing straightforward.

To make the new feature complete, absolute paths must descent to
autoloaded functions. Keeping the absolute path only to directly
autoloaded function is ersatz. It's a half-step. I autoload single
function, and then FPATH again takes control and does the "imagine the
layers" thing. This is a tragedy.

-- 
Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-29 12:27             ` Sebastian Gniazdowski
@ 2017-01-29 16:11               ` Bart Schaefer
  2017-01-29 17:32                 ` Sebastian Gniazdowski
  2017-01-29 17:58                 ` Peter Stephenson
  0 siblings, 2 replies; 35+ messages in thread
From: Bart Schaefer @ 2017-01-29 16:11 UTC (permalink / raw)
  To: zsh-workers

On Sun, 29 Jan 2017, Sebastian Gniazdowski wrote:

> To make the new feature complete, absolute paths must descent to
> autoloaded functions. Keeping the absolute path only to directly
> autoloaded function is ersatz. It's a half-step. I autoload single
> function, and then FPATH again takes control and does the "imagine the
> layers" thing. This is a tragedy.

Respectfully, Sebastian, I disagree.

Consider for example that several of the autoloadable functions in the
zsh distribution rely on "autoload colors".

If autoload behaved the way you suggest, this would break, because the
colors function would not be found at the absolute path from which the
calling function was loaded -- unless the writer of the calling function
has used "autoload -d", of course.  But if we're now relying on the
author to use the correct options for autoload, we're right back where
we started -- might as well rely on the author to make his function
suite "relocatable" by way of $function_source as well, and that is
now possible.

There's no magic solution that automatically covers all the variations
without cooperation from both the writer and the installer of the suite.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-29 16:11               ` Bart Schaefer
@ 2017-01-29 17:32                 ` Sebastian Gniazdowski
  2017-01-29 18:37                   ` Bart Schaefer
  2017-01-29 17:58                 ` Peter Stephenson
  1 sibling, 1 reply; 35+ messages in thread
From: Sebastian Gniazdowski @ 2017-01-29 17:32 UTC (permalink / raw)
  To: zsh-workers

On Sun, Jan 29, 2017, at 08:11 AM, Bart Schaefer wrote:
> On Sun, 29 Jan 2017, Sebastian Gniazdowski wrote:
> 
> > To make the new feature complete, absolute paths must descent to
> > autoloaded functions. Keeping the absolute path only to directly
> > autoloaded function is ersatz. It's a half-step. I autoload single
> > function, and then FPATH again takes control and does the "imagine the
> > layers" thing. This is a tragedy.
> 
> Respectfully, Sebastian, I disagree.
> 
> Consider for example that several of the autoloadable functions in the
> zsh distribution rely on "autoload colors".
> 
> If autoload behaved the way you suggest, this would break, because the
> colors function would not be found at the absolute path from which the
> calling function was loaded -- unless the writer of the calling function
> has used "autoload -d", of course. 

I wrote earlier that -P (let's use your proposed option name) should
prepend dirchache path to FPATH. In other words, -P causes any descent
autoload to try dircache part first, then follow to process fpath.

> But if we're now relying on the
> author to use the correct options for autoload, we're right back where
> we started -- might as well rely on the author to make his function
> suite "relocatable" by way of $function_source as well, and that is
> now possible.
> 
> There's no magic solution that automatically covers all the variations
> without cooperation from both the writer and the installer of the suite.

I think -P is a complete idea, as you know it comes from problems with
Zplugin, -P would solve those problems. So the idea has some real world
experience to back it up.

-- 
  Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-29 16:11               ` Bart Schaefer
  2017-01-29 17:32                 ` Sebastian Gniazdowski
@ 2017-01-29 17:58                 ` Peter Stephenson
  2017-01-30 11:37                   ` Sebastian Gniazdowski
  1 sibling, 1 reply; 35+ messages in thread
From: Peter Stephenson @ 2017-01-29 17:58 UTC (permalink / raw)
  To: zsh-workers

On Sun, 29 Jan 2017 08:11:04 -0800 (PST)
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Consider for example that several of the autoloadable functions in the
> zsh distribution rely on "autoload colors".
> 
> If autoload behaved the way you suggest, this would break, because the
> colors function would not be found at the absolute path from which the
> calling function was loaded -- unless the writer of the calling function
> has used "autoload -d", of course.  But if we're now relying on the
> author to use the correct options for autoload, we're right back where
> we started -- might as well rely on the author to make his function
> suite "relocatable" by way of $function_source as well, and that is
> now possible.
> 
> There's no magic solution that automatically covers all the variations
> without cooperation from both the writer and the installer of the suite.

That's roughly my thinking --- any attempt to magic it gets very messy
very quickly --- and you don't need to.  Here's how it can work.  I
think it puts the onus to do the clever stuff on the function code
writer, which is the right place.

1. User gets a heap of functions and is instructed to put them all in the
same directory (or under the same directory, we have the flexiblity to
make it work with a set of subdirectories).  That's not controversial
anyway, I don't think.

2. User puts:

autoload -Uz /wherever/they/put/it/mainfunc

in their start up.  Note it'll still work if they rely on $fpath ---
mainfunc is resolved when loaded, so the code below still works.  So you
are protected against the user doing anything as long as it's some valid
autoload that works in the version of the shell in us.  No special
instructions are needed.

3. mainfunc contains:

zmodload zsh/parameter
if (( ${+functions_source} )); then
  # If that's available, load by absolute path is available.
  # The -R barfs immediately if the function can't be found
  # (optional extra to simplify error handling).
  # You can add -d if you want to search $fpath, too.
  autoload -RUz ${functions_source[mainfunc]:h}/{other1,other2,other3}
  # ... or whatever.
else
  # We're stuck with old-fashioned fpath.  Add warning, check, whatever.
  autoload other1 other2 other3
fi

This can be tailored to suit.  Any half-baked magic in the shell C
implementation can't.  To repeat, I'm not in any case interested in
doing half-baked magic in the shell C implementation. so let's not
go down that route again.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-29 17:32                 ` Sebastian Gniazdowski
@ 2017-01-29 18:37                   ` Bart Schaefer
  2017-01-29 21:53                     ` Vin Shelton
  0 siblings, 1 reply; 35+ messages in thread
From: Bart Schaefer @ 2017-01-29 18:37 UTC (permalink / raw)
  To: zsh-workers

On Sun, 29 Jan 2017, Sebastian Gniazdowski wrote:

> I wrote earlier that -P (let's use your proposed option name) should
> prepend dirchache path to FPATH. In other words, -P causes any descent
> autoload to try dircache part first, then follow to process fpath.

That's still not sufficient for the general case, I think.  What happens
if two function suites have an autoload-dependency on one another, and
neither is in the default $fpath?


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-29 18:37                   ` Bart Schaefer
@ 2017-01-29 21:53                     ` Vin Shelton
  2017-01-30 10:06                       ` Peter Stephenson
  0 siblings, 1 reply; 35+ messages in thread
From: Vin Shelton @ 2017-01-29 21:53 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh Hackers' List

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

(Peter I copied you directly because email from my gmail account has been
bouncing.)

I am seeing two failures with the latest updates, see below.  Please let me
know if you need more info.

Regards,
  Vin


cd Test ; make check
make[1]: Entering directory '/SSD-2TB/opt/build/zsh-2017-01-29/Test'
if test -n "gcc"; then \
  cd .. && DESTDIR= \
  make MODDIR=`pwd`/Test/Modules install.modules > /dev/null; \
fi
if ZTST_testlist="`for f in /opt/src/zsh-2017-01-29/Test/*.ztst; \
           do echo $f; done`" \
 ZTST_srcdir="/opt/src/zsh-2017-01-29/Test" \
 ZTST_exe=../Src/zsh \
 ../Src/zsh +Z -f /opt/src/zsh-2017-01-29/Test/runtests.zsh; then \
 stat=0; \
else \
 stat=1; \
fi; \
sleep 1; \
rm -rf Modules .zcompdump; \
exit $stat
/opt/src/zsh-2017-01-29/Test/A01grammar.ztst: starting.
This test hangs the shell when it fails...
/opt/src/zsh-2017-01-29/Test/A01grammar.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/A02alias.ztst: starting.
This test hangs the shell when it fails...
/opt/src/zsh-2017-01-29/Test/A02alias.ztst: all tests successful.

/opt/src/zsh-2017-01-29/Test/A03quoting.ztst: starting.
/opt/src/zsh-2017-01-29/Test/A03quoting.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/A04redirect.ztst: starting.
/opt/src/zsh-2017-01-29/Test/A04redirect.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/A05execution.ztst: starting.
This test takes 5 seconds to fail...
This test takes 3 seconds and hangs the shell when it fails...
/opt/src/zsh-2017-01-29/Test/A05execution.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/A06assign.ztst: starting.
/opt/src/zsh-2017-01-29/Test/A06assign.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/A07control.ztst: starting.
/opt/src/zsh-2017-01-29/Test/A07control.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B01cd.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B01cd.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B02typeset.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B02typeset.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B03print.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B03print.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B04read.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B04read.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B05eval.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B05eval.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B06fc.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B06fc.ztst: all tests successful.

/opt/src/zsh-2017-01-29/Test/B07emulate.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B07emulate.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B08shift.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B08shift.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/B09hash.ztst: starting.
/opt/src/zsh-2017-01-29/Test/B09hash.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/C01arith.ztst: starting.
/opt/src/zsh-2017-01-29/Test/C01arith.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/C02cond.ztst: starting.
This test takes two seconds...
/opt/src/zsh-2017-01-29/Test/C02cond.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/C03traps.ztst: starting.
This test takes at least three seconds...
This test, too, takes at least three seconds...
Another test that takes three seconds
/opt/src/zsh-2017-01-29/Test/C03traps.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/C04funcdef.ztst: starting.
--- /tmp/zsh.ztst.11675/ztst.out 2017-01-29 16:48:50.438882533 -0500
+++ /tmp/zsh.ztst.11675/ztst.tout 2017-01-29 16:48:50.442882549 -0500
@@ -1 +1 @@
-oops is a shell function from CURDIR/oops
+oops is a shell function from /SSD-2TBCURDIR/oops
Test /opt/src/zsh-2017-01-29/Test/C04funcdef.ztst failed: output differs
from expected as shown above for:
  (
    fpath=(.)
    printf '%s\n' 'oops(){}' 'ninjas-earring(){}' 'oops "$@"' >oops
    autoload oops
    oops
    whence -v oops | sed -e "s%$PWD%CURDIR%"
  )
Was testing: whence -v of zsh-style autoload
/opt/src/zsh-2017-01-29/Test/C04funcdef.ztst: test failed.
/opt/src/zsh-2017-01-29/Test/C05debug.ztst: starting.
/opt/src/zsh-2017-01-29/Test/C05debug.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D01prompt.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D01prompt.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D02glob.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D02glob.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D03procsubst.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D03procsubst.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D04parameter.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D04parameter.ztst: all tests successful.

/opt/src/zsh-2017-01-29/Test/D05array.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D05array.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D06subscript.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D06subscript.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D07multibyte.ztst: starting.
Testing multibyte with locale en_US.UTF-8
Test case skipped: No Polish UTF-8 locale found, skipping sort test
/opt/src/zsh-2017-01-29/Test/D07multibyte.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D08cmdsubst.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D08cmdsubst.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/D09brace.ztst: starting.
/opt/src/zsh-2017-01-29/Test/D09brace.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/E01options.ztst: starting.
This test hangs the shell when it fails...
/opt/src/zsh-2017-01-29/Test/E01options.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/E02xtrace.ztst: starting.
/opt/src/zsh-2017-01-29/Test/E02xtrace.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V01zmodload.ztst: starting.
/opt/src/zsh-2017-01-29/Test/V01zmodload.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V02zregexparse.ztst: starting.
/opt/src/zsh-2017-01-29/Test/V02zregexparse.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V03mathfunc.ztst: starting.
/opt/src/zsh-2017-01-29/Test/V03mathfunc.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V04features.ztst: starting.
/opt/src/zsh-2017-01-29/Test/V04features.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V05styles.ztst: starting.
/opt/src/zsh-2017-01-29/Test/V05styles.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V06parameter.ztst: starting.
--- /tmp/zsh.ztst.14908/ztst.out 2017-01-29 16:48:55.798904574 -0500
+++ /tmp/zsh.ztst.14908/ztst.tout 2017-01-29 16:48:55.802904591 -0500
@@ -3,8 +3,8 @@
 Inside function fn
 2 + ./functrace.zsh:10 + ./functrace.zsh:5
 Inside autofn
-2 + ./functrace.zsh:20 + CURDIR/autofn:0
+2 + ./functrace.zsh:20 + /SSD-2TB/opt/build/zsh-2017-01-29/Test/autofn:0
 Inside autofn
-2 + ./functrace.zsh:21 + CURDIR/autofn:0
+2 + ./functrace.zsh:21 + /SSD-2TB/opt/build/zsh-2017-01-29/Test/autofn:0
 In sourced file
 2 + ./functrace.zsh:22 + ./sourcedfile:0
Test /opt/src/zsh-2017-01-29/Test/V06parameter.ztst failed: output differs
from expected as shown above for:
  print 'print In sourced file
  print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
  ' >sourcedfile
  print -r -- 'print Started functrace.zsh
  module_path=(./Modules)
  print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
  :
  fn() {
    print Inside function $0
    print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
  }
  :
  fn
  :
  fpath=(. $fpath)
  :
  echo '\''print Inside $0
    print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
  '\'' >autofn
  :
  autoload autofn
  :
  autofn
  autofn
  . ./sourcedfile' >functrace.zsh
  $ZTST_testdir/../Src/zsh +Z -f ./functrace.zsh
Was testing: Function tracing
/opt/src/zsh-2017-01-29/Test/V06parameter.ztst: test failed.
/opt/src/zsh-2017-01-29/Test/V07pcre.ztst: starting.
Testing PCRE multibyte with locale en_US.UTF-8
/opt/src/zsh-2017-01-29/Test/V07pcre.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V08zpty.ztst: starting.
/opt/src/zsh-2017-01-29/Test/V08zpty.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V09datetime.ztst: starting.
Test case skipped: Japanese UTF-8 locale not supported
/opt/src/zsh-2017-01-29/Test/V09datetime.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/V10private.ztst: starting.
/opt/src/zsh-2017-01-29/Test/V10private.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/W01history.ztst: starting.
History tests write to /dev/tty
  print ten ten nine one print

  print   print one two three four five six seven eight nine ten one two

  print mystery sequence

  print one

  print two

  print mystery sequence

  print metaphor\? shmetaphor!

  print metaphor!

  print -l metophor, Molochi,

  echo $(  echo foo bar) again

  echo more $(   echo $(  echo foo bar) again )

/opt/src/zsh-2017-01-29/Test/W01history.ztst: all tests successful.

/opt/src/zsh-2017-01-29/Test/X02zlevi.ztst: starting.
This test may hang the shell when it fails...
/opt/src/zsh-2017-01-29/Test/X02zlevi.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/X03zlebindkey.ztst: starting.
/opt/src/zsh-2017-01-29/Test/X03zlebindkey.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/Y01completion.ztst: starting.
/opt/src/zsh-2017-01-29/Test/Y01completion.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/Y02compmatch.ztst: starting.
/opt/src/zsh-2017-01-29/Test/Y02compmatch.ztst: all tests successful.
/opt/src/zsh-2017-01-29/Test/Y03arguments.ztst: starting.
/opt/src/zsh-2017-01-29/Test/Y03arguments.ztst: all tests successful.
**************************************
46 successful test scripts, 2 failures, 0 skipped
**************************************
Makefile:187: recipe for target 'check' failed
make[1]: *** [check] Error 1
make[1]: Leaving directory '/SSD-2TB/opt/build/zsh-2017-01-29/Test'
Makefile:263: recipe for target 'check' failed
make: *** [check] Error 2
check_zsh: make check failed with error code 2.  See /tmp/zsh_check2065.out.


-- 
And my clothes don't fit me no more
I walked a thousand miles
Just to slip this skin

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-29 21:53                     ` Vin Shelton
@ 2017-01-30 10:06                       ` Peter Stephenson
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-30 10:06 UTC (permalink / raw)
  To: Zsh Hackers' List

On Sun, 29 Jan 2017 16:53:01 -0500
Vin Shelton <acs@alumni.princeton.edu> wrote:
> (Peter I copied you directly because email from my gmail account has been
> bouncing.)
> 
> I am seeing two failures with the latest updates, see below.  Please let me
> know if you need more info.
> /opt/src/zsh-2017-01-29/Test/C04funcdef.ztst: starting.
> --- /tmp/zsh.ztst.11675/ztst.out 2017-01-29 16:48:50.438882533 -0500
> +++ /tmp/zsh.ztst.11675/ztst.tout 2017-01-29 16:48:50.442882549 -0500
> @@ -1 +1 @@
> -oops is a shell function from CURDIR/oops
> +oops is a shell function from /SSD-2TBCURDIR/oops
> /opt/src/zsh-2017-01-29/Test/V06parameter.ztst: starting.
> --- /tmp/zsh.ztst.14908/ztst.out 2017-01-29 16:48:55.798904574 -0500
> +++ /tmp/zsh.ztst.14908/ztst.tout 2017-01-29 16:48:55.802904591 -0500
> @@ -3,8 +3,8 @@
>  Inside function fn
>  2 + ./functrace.zsh:10 + ./functrace.zsh:5
>  Inside autofn
> -2 + ./functrace.zsh:20 + CURDIR/autofn:0
> +2 + ./functrace.zsh:20 + /SSD-2TB/opt/build/zsh-2017-01-29/Test/autofn:0
>  Inside autofn
> -2 + ./functrace.zsh:21 + CURDIR/autofn:0
> +2 + ./functrace.zsh:21 + /SSD-2TB/opt/build/zsh-2017-01-29/Test/autofn:0
>  In sourced file
>  2 + ./functrace.zsh:22 + ./sourcedfile:0

Let's try the following trick, which has been in use in B01cd.ztst for
some time so presumably works.

pws

diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index 176841d..0cf2b58 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -2,6 +2,10 @@
 
   mkdir funcdef.tmp
   cd funcdef.tmp
+  setopt chaselinks
+  cd .
+  unsetopt chaselinks
+  mydir=$PWD
 
 %test
 
@@ -325,10 +329,10 @@
     printf '%s\n' 'oops(){}' 'ninjas-earring(){}' 'oops "$@"' >oops
     autoload oops
     oops
-    whence -v oops | sed -e "s%$PWD%CURDIR%"
+    whence -v oops
   )
-0:whence -v of zsh-style autoload
->oops is a shell function from CURDIR/oops
+0q:whence -v of zsh-style autoload
+>oops is a shell function from $mydir/oops
 
   (
     fpath=(.)
diff --git a/Test/V06parameter.ztst b/Test/V06parameter.ztst
index c2a2a4d..10e0a27 100644
--- a/Test/V06parameter.ztst
+++ b/Test/V06parameter.ztst
@@ -1,15 +1,22 @@
+%prep
+
+  setopt chaselinks
+  cd .
+  unsetopt chaselinks
+  mydir=$PWD
+
 %test
 
   print 'print In sourced file
-  print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
+  print $LINENO + $functrace + ${funcsourcetrace}
   ' >sourcedfile
   print -r -- 'print Started functrace.zsh
   module_path=(./Modules)
-  print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
+  print $LINENO + $functrace + ${funcsourcetrace}
   :
   fn() {
     print Inside function $0
-    print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
+    print $LINENO + $functrace + ${funcsourcetrace}
   }
   :
   fn
@@ -17,7 +24,7 @@
   fpath=(. $fpath)
   :
   echo '\''print Inside $0
-    print $LINENO + $functrace + ${funcsourcetrace/#$PWD/CURDIR}
+    print $LINENO + $functrace + ${funcsourcetrace}
   '\'' >autofn
   :
   autoload autofn
@@ -26,15 +33,15 @@
   autofn
   . ./sourcedfile' >functrace.zsh
   $ZTST_testdir/../Src/zsh +Z -f ./functrace.zsh
-0:Function tracing
+0q:Function tracing
 >Started functrace.zsh
 >3 + +
 >Inside function fn
 >2 + ./functrace.zsh:10 + ./functrace.zsh:5
 >Inside autofn
->2 + ./functrace.zsh:20 + CURDIR/autofn:0
+>2 + ./functrace.zsh:20 + $mydir/autofn:0
 >Inside autofn
->2 + ./functrace.zsh:21 + CURDIR/autofn:0
+>2 + ./functrace.zsh:21 + $mydir/autofn:0
 >In sourced file
 >2 + ./functrace.zsh:22 + ./sourcedfile:0
 
@@ -70,14 +77,14 @@
     fpath=($PWD)
     print "print I have been autoloaded" >myfunc
     autoload $PWD/myfunc
-    print ${functions_source[myfunc]/#$PWD/CURDIR}
+    print ${functions_source[myfunc]}
     myfunc
-    print ${functions_source[myfunc]/#$PWD/CURDIR}
+    print ${functions_source[myfunc]}
   )
-0: $functions_source
->CURDIR/myfunc
+0q: $functions_source
+>$mydir/myfunc
 >I have been autoloaded
->CURDIR/myfunc
+>$mydir/myfunc
 
 %clean
 


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-29 17:58                 ` Peter Stephenson
@ 2017-01-30 11:37                   ` Sebastian Gniazdowski
  2017-01-30 11:55                     ` Peter Stephenson
  0 siblings, 1 reply; 35+ messages in thread
From: Sebastian Gniazdowski @ 2017-01-30 11:37 UTC (permalink / raw)
  To: zsh-workers

On Sun, Jan 29, 2017, at 09:58 AM, Peter Stephenson wrote:
> That's roughly my thinking --- any attempt to magic it gets very messy
> very quickly --- and you don't need to.  Here's how it can work.  I
> think it puts the onus to do the clever stuff on the function code
> writer, which is the right place.


Look, your new functionality is a disaster currently. You can autoload
only one function via absolute-path, any descent autoload will fail, and
you non-FPATH function directory is useless.

-- 
  Sebastian Gniazdowski


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-30 11:37                   ` Sebastian Gniazdowski
@ 2017-01-30 11:55                     ` Peter Stephenson
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-30 11:55 UTC (permalink / raw)
  To: zsh-workers

On Mon, 30 Jan 2017 03:37:02 -0800
Sebastian Gniazdowski <psprint2@fastmail.com> wrote:
> Look, your new functionality is a disaster currently. You can autoload
> only one function via absolute-path, any descent autoload will fail, and
> you non-FPATH function directory is useless.

OK, I've now worked on this for some time, explained in some detail how
it's used, all of which does work as it's been tested, and simply got
abuse with no technical content.

That's the end of the discussion.  Do not bring this up again.  Do not
reply to this message.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
       [not found]                   ` <CAHYJk3SB1NDj6y5TRHHsAVsyjHfZQhTzMRzTR2c-SVEc9oAwzA@mail.gmail.com>
@ 2017-01-24 11:10                     ` Peter Stephenson
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-24 11:10 UTC (permalink / raw)
  To: Zsh hackers list

On Tue, 24 Jan 2017 11:17:34 +0100
Mikael Magnusson <mikachu@gmail.com> wrote:
> This patch is probably a bad idea, it's extremely common to do this:
> PROGRAM="$(which program)"
> and with the above change that no longer works.

Yes, that was supposed to be limited to the chattier -v output.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index 2fb1a70..219fbc9 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3774,9 +3774,11 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 		    if (wd) {
 			printf("%s: command\n", *argv);
 		    } else {
-			if (v && !csh)
+			if (v && !csh) {
 			    zputs(*argv, stdout), fputs(" is ", stdout);
-			quotedzputs(buf, stdout);
+			    quotedzputs(buf, stdout);
+			} else
+			    zputs(buf, stdout);
 			if (OPT_ISSET(ops,'s') || OPT_ISSET(ops, 'S'))
 			    print_if_link(buf, OPT_ISSET(ops, 'S'));
 			fputc('\n', stdout);
@@ -3806,9 +3808,11 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 	    if (wd) {
 		printf("%s: command\n", *argv);
 	    } else {
-		if (v && !csh)
+		if (v && !csh) {
 		    zputs(*argv, stdout), fputs(" is ", stdout);
-		quotedzputs(cnam, stdout);
+		    quotedzputs(cnam, stdout);
+		} else
+		    zputs(cnam, stdout);
 		if (OPT_ISSET(ops,'s') || OPT_ISSET(ops,'S'))
 		    print_if_link(cnam, OPT_ISSET(ops,'S'));
 		fputc('\n', stdout);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-16 15:22               ` Bart Schaefer
@ 2017-01-16 15:59                 ` Peter Stephenson
       [not found]                   ` <CAHYJk3SB1NDj6y5TRHHsAVsyjHfZQhTzMRzTR2c-SVEc9oAwzA@mail.gmail.com>
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Stephenson @ 2017-01-16 15:59 UTC (permalink / raw)
  To: Zsh hackers list

On Mon, 16 Jan 2017 07:22:23 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:

> On Mon, Jan 16, 2017 at 2:37 AM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
> >
> > % whence -v my\ stupid\ function\ name
> > my stupid function name is a shell function from /export/home/pws/src/zsh-git/code/'my stupid function name'
> >
> > You might even consider that a feature.
> 
> When was this pushed?  I don't get any quoting.
> 
> % path+=(/tmp/one\ space)
> % whence -v two\ space
> two space is /tmp/one space/two space

That's not a shell function, that's a command.  Looks like that doesn't
do quoting.; I presume that's just a historical oversight.

This would handle the other verbose "is" cases.  However, in this case
the entire path is passed back by findcmd, so it's all quoted.  Updating
findcmd() would make it more efficient (don't need to copy the
directory) and more consistent with the function case, but it's too
minor for me to feel like doing.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index a683032..b1b6e2e 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3747,7 +3747,7 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 		    } else {
 			if (v && !csh)
 			    zputs(*argv, stdout), fputs(" is ", stdout);
-			zputs(buf, stdout);
+			quotedzputs(buf, stdout);
 			if (OPT_ISSET(ops,'s') || OPT_ISSET(ops, 'S'))
 			    print_if_link(buf, OPT_ISSET(ops, 'S'));
 			fputc('\n', stdout);
@@ -3779,7 +3779,7 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 	    } else {
 		if (v && !csh)
 		    zputs(*argv, stdout), fputs(" is ", stdout);
-		zputs(cnam, stdout);
+		quotedzputs(cnam, stdout);
 		if (OPT_ISSET(ops,'s') || OPT_ISSET(ops,'S'))
 		    print_if_link(cnam, OPT_ISSET(ops,'S'));
 		fputc('\n', stdout);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-16 15:04               ` Daniel Shahaf
@ 2017-01-16 15:48                 ` Peter Stephenson
  0 siblings, 0 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-16 15:48 UTC (permalink / raw)
  To: Zsh hackers list

On Mon, 16 Jan 2017 15:04:52 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Peter Stephenson wrote on Mon, Jan 16, 2017 at 10:37:08 +0000:
> > One very minor change is that to
> > avoid the intermediate string quotedzputs() now runs separately on the
> > directory and the filename, e.g.
> > 
> > % whence -v my\ stupid\ function\ name
> > my stupid function name is a shell function from /export/home/pws/src/zsh-git/code/'my stupid function name'
> > 
> > You might even consider that a feature.
> 
> Does it do the right thing when RC_QUOTES is set?

You'll notice there's a "/" between the directory and the file.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-16 10:37             ` Peter Stephenson
  2017-01-16 15:04               ` Daniel Shahaf
@ 2017-01-16 15:22               ` Bart Schaefer
  2017-01-16 15:59                 ` Peter Stephenson
  1 sibling, 1 reply; 35+ messages in thread
From: Bart Schaefer @ 2017-01-16 15:22 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Mon, Jan 16, 2017 at 2:37 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
>
> % whence -v my\ stupid\ function\ name
> my stupid function name is a shell function from /export/home/pws/src/zsh-git/code/'my stupid function name'
>
> You might even consider that a feature.

When was this pushed?  I don't get any quoting.

% path+=(/tmp/one\ space)
% whence -v two\ space
two space is /tmp/one space/two space
%

% git whatchanged
commit f90a0447aada3ad2fa114210c62b855a3e60cb1d
Author: Peter Stephenson <pws@zsh.org>
Date:   Mon Jan 16 10:31:56 2017 +0000

    40353 with tweaks to whence -v: extend directory cache use.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-16 10:37             ` Peter Stephenson
@ 2017-01-16 15:04               ` Daniel Shahaf
  2017-01-16 15:48                 ` Peter Stephenson
  2017-01-16 15:22               ` Bart Schaefer
  1 sibling, 1 reply; 35+ messages in thread
From: Daniel Shahaf @ 2017-01-16 15:04 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

Peter Stephenson wrote on Mon, Jan 16, 2017 at 10:37:08 +0000:
> One very minor change is that to
> avoid the intermediate string quotedzputs() now runs separately on the
> directory and the filename, e.g.
> 
> % whence -v my\ stupid\ function\ name
> my stupid function name is a shell function from /export/home/pws/src/zsh-git/code/'my stupid function name'
> 
> You might even consider that a feature.

Does it do the right thing when RC_QUOTES is set?


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-13 18:04           ` Peter Stephenson
@ 2017-01-16 10:37             ` Peter Stephenson
  2017-01-16 15:04               ` Daniel Shahaf
  2017-01-16 15:22               ` Bart Schaefer
  0 siblings, 2 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-16 10:37 UTC (permalink / raw)
  To: Zsh hackers list

On Fri, 13 Jan 2017 18:04:23 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Wed, 11 Jan 2017 20:51:22 +0000
> Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > With a bit of work this can be expanded to paths for already loaded
> > functions, and possibly elsewhere though I think the pickings are
> > a lot smaller.
> 
> This does the former of that --- it was less work than I expected.
>...
> The only significant extra requirement is use of the heap to get the
> filename in "whence -v" --- that's a bit of laziness, we could just print
> out the components using the same logic as getshfuncfile().  Might fix
> that later.

I fixed this up when I pushed it.  One very minor change is that to
avoid the intermediate string quotedzputs() now runs separately on the
directory and the filename, e.g.

% whence -v my\ stupid\ function\ name
my stupid function name is a shell function from /export/home/pws/src/zsh-git/code/'my stupid function name'

You might even consider that a feature.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-11 20:51         ` Peter Stephenson
  2017-01-12 20:42           ` Peter Stephenson
@ 2017-01-13 18:04           ` Peter Stephenson
  2017-01-16 10:37             ` Peter Stephenson
  1 sibling, 1 reply; 35+ messages in thread
From: Peter Stephenson @ 2017-01-13 18:04 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 11 Jan 2017 20:51:22 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> With a bit of work this can be expanded to paths for already loaded
> functions, and possibly elsewhere though I think the pickings are
> a lot smaller.

This does the former of that --- it was less work than I expected.

The upshot of this is that with the new absolute path scheme, storing
the filename at the point of loading the function is a no-op since we've
already got the components and keep them.  With a traditional
fpath-style load, we use the new directory cache at the point where we
load the file from the path, so this now also avoids an extra directory
allocation (and, unlike the similar mechanism used for the command hash,
avoids the brittle association with the original path variable).

There is a heap allocation when we enter the function to put the name on
the function stack (see getshfuncfile()), but there always was --- the
only difference is a little extra calculation for the three components.
So I think the penalty is trivial to non-existent.

The only significant extra requirement is use of the heap to get the
filename in "whence -v" --- that's a bit of laziness, we could just print
out the components using the same logic as getshfuncfile().  Might fix
that later.

Note that the PM_LOADDIR flag still represents exactly what it did,
it's just that the period of possible validity now lasts throughout the
function [nearly said product] life cycle.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 7a5d2bd..6c82643 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5124,12 +5124,12 @@ execautofn_basic(Estate state, UNUSED(int do_exec))
      * defined yet.
      */
     if (funcstack && !funcstack->filename)
-	funcstack->filename = dupstring(shf->filename);
+	funcstack->filename = getshfuncfile(shf);
 
     oldscriptname = scriptname;
     oldscriptfilename = scriptfilename;
     scriptname = dupstring(shf->node.nam);
-    scriptfilename = dupstring(shf->filename);
+    scriptfilename = getshfuncfile(shf);
     execode(shf->funcdef, 1, 0, "loadautofunc");
     scriptname = oldscriptname;
     scriptfilename = oldscriptfilename;
@@ -5150,13 +5150,47 @@ execautofn(Estate state, UNUSED(int do_exec))
     return execautofn_basic(state, 0);
 }
 
+/*
+ * Helper function to install the source file name of a shell function
+ * just autoloaded.
+ *
+ * We attempt to do this efficiently as the typical case is the
+ * directory part is a well-known directory, which is cached, and
+ * the non-directory part is the same as the node name.
+ */
+
+/**/
+static void
+loadautofnsetfile(Shfunc shf, char *fdir)
+{
+    /*
+     * If shf->filename is already the load directory ---
+     * keep it as we can still use it to get the load file.
+     * This makes autoload with an absolute path particularly efficient.
+     */
+    if (!(shf->node.flags & PM_LOADDIR) ||
+	strcmp(shf->filename, fdir) != 0) {
+	/* Old directory name not useful... */
+	dircache_set(&shf->filename, NULL);
+	if (fdir) {
+	    /* ...can still cache directory */
+	    shf->node.flags |= PM_LOADDIR;
+	    dircache_set(&shf->filename, fdir);
+	} else {
+	    /* ...no separate directory part to cache, for some reason. */
+	    shf->node.flags &= ~PM_LOADDIR;
+	    shf->filename = ztrdup(shf->node.nam);
+	}
+    }
+}
+
 /**/
 Shfunc
 loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 {
     int noalias = noaliases, ksh = 1;
     Eprog prog;
-    char *fname;
+    char *fdir;			/* Directory path where func found */
 
     pushheap();
 
@@ -5167,13 +5201,13 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	char *spec_path[2];
 	spec_path[0] = dupstring(shf->filename);
 	spec_path[1] = NULL;
-	prog = getfpfunc(shf->node.nam, &ksh, &fname, spec_path, 0);
+	prog = getfpfunc(shf->node.nam, &ksh, &fdir, spec_path, 0);
 	if (prog == &dummy_eprog &&
 	    (current_fpath || (shf->node.flags & PM_CUR_FPATH)))
-	    prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0);
+	    prog = getfpfunc(shf->node.nam, &ksh, &fdir, NULL, 0);
     }
     else
-	prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0);
+	prog = getfpfunc(shf->node.nam, &ksh, &fdir, NULL, 0);
     noaliases = noalias;
 
     if (ksh == 1) {
@@ -5192,7 +5226,6 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	return NULL;
     }
     if (!prog) {
-	zsfree(fname);
 	popheap();
 	return NULL;
     }
@@ -5205,10 +5238,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 		shf->funcdef = prog;
 	    else
 		shf->funcdef = dupeprog(prog, 0);
-	    shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR);
-	    dircache_set(&shf->filename, NULL);
-	    /* Full filename, don't use dircache */
-	    shf->filename = fname;
+	    shf->node.flags &= ~PM_UNDEFINED;
+	    loadautofnsetfile(shf, fdir);
 	} else {
 	    VARARR(char, n, strlen(shf->node.nam) + 1);
 	    strcpy(n, shf->node.nam);
@@ -5220,7 +5251,6 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 		zwarn("%s: function not defined by file", n);
 		locallevel++;
 		popheap();
-		zsfree(fname);
 		return NULL;
 	    }
 	}
@@ -5230,10 +5260,8 @@ 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|PM_LOADDIR);
-	dircache_set(&shf->filename, NULL);
-	/* Full filename, don't use dircache */
-	shf->filename = fname;
+	shf->node.flags &= ~PM_UNDEFINED;
+	loadautofnsetfile(shf, fdir);
     }
     popheap();
 
@@ -5453,7 +5481,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcstack = &fstack;
 
 	fstack.flineno = shfunc->lineno;
-	fstack.filename = dupstring(shfunc->filename);
+	fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;
 	if (prog->flags & EF_RUN) {
@@ -5623,16 +5651,17 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name)
  * Search fpath for an undefined function.  Finds the file, and returns the
  * list of its contents.
  *
- * If test is 0, load the function; *fname is set to zalloc'ed location.
+ * If test is 0, load the function.
  *
  * If test_only is 1, don't load function, just test for it:
- * - Non-null return means function was found
- * - *fname points to path at which found (not duplicated)
+ * Non-null return means function was found
+ *
+ * *fdir points to path at which found (as passed in, not duplicated)
  */
 
 /**/
 Eprog
-getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
+getfpfunc(char *s, int *ksh, char **fdir, char **alt_path, int test_only)
 {
     char **pp, buf[PATH_MAX+1];
     off_t len;
@@ -5650,8 +5679,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
 	else
 	    strcpy(buf, s);
 	if ((r = try_dump_file(*pp, s, buf, ksh, test_only))) {
-	    if (fname)
-		*fname = test_only ? *pp : ztrdup(buf);
+	    if (fdir)
+		*fdir = *pp;
 	    return r;
 	}
 	unmetafy(buf, NULL);
@@ -5661,7 +5690,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
 		(len = lseek(fd, 0, 2)) != -1) {
 		if (test_only) {
 		    close(fd);
-		    *fname = *pp;
+		    if (fdir)
+			*fdir = *pp;
 		    return &dummy_eprog;
 		}
 		d = (char *) zalloc(len + 1);
@@ -5677,8 +5707,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
 		    r = parse_string(d, 1);
 		    scriptname = oldscriptname;
 
-		    if (fname)
-			*fname = ztrdup(buf);
+		    if (fdir)
+			*fdir = *pp;
 
 		    zfree(d, len + 1);
 
diff --git a/Src/hashtable.c b/Src/hashtable.c
index a3a38f7..0fc87f3 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -926,10 +926,12 @@ printshfuncnode(HashNode hn, int printflags)
 	       (f->node.flags & PM_UNDEFINED) ?
 	       " is an autoload shell function" :
 	       " is a shell function");
-	if (f->filename && (printflags & PRINT_WHENCE_VERBOSE) &&
-	    strcmp(f->filename, f->node.nam) != 0) {
-	    printf(" from ");
-	    quotedzputs(f->filename, stdout);
+	if (printflags & PRINT_WHENCE_VERBOSE) {
+	    char *fname = getshfuncfile(f);
+	    if (fname && strcmp(fname, f->node.nam) != 0) {
+		printf(" from ");
+		quotedzputs(fname, stdout);
+	    }
 	}
 	putchar('\n');
 	return;
@@ -959,7 +961,7 @@ printshfuncnode(HashNode hn, int printflags)
 	    zputs("builtin autoload -X", stdout);
 	    for (fl=0;fopt[fl];fl++)
 		if (f->node.flags & flgs[fl]) putchar(fopt[fl]);
-	    if (f->filename) {
+	    if (f->filename && (f->node.flags & PM_LOADDIR)) {
 		putchar(' ');
 		zputs(f->filename, stdout);
 	    }
@@ -1041,6 +1043,24 @@ printshfuncexpand(HashNode hn, int printflags, int expand)
     text_expand_tabs = save_expand;
 }
 
+/*
+ * Get a heap-duplicated name of the shell function, for
+ * use in tracing.
+ */
+
+/**/
+mod_export char *
+getshfuncfile(Shfunc shf)
+{
+    if (shf->node.flags & PM_LOADDIR) {
+	return zhtricat(shf->filename, "/", shf->node.nam);
+    } else if (shf->filename) {
+	return dupstring(shf->filename);
+    } else {
+	return NULL;
+    }
+}
+
 /**************************************/
 /* Reserved Word Hash Table Functions */
 /**************************************/
diff --git a/Src/signals.c b/Src/signals.c
index 2de5743..a717677 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -811,8 +811,11 @@ dosavetrap(int sig, int level)
 	    newshf->node.nam = ztrdup(shf->node.nam);
 	    newshf->node.flags = shf->node.flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
-	    /* Assume this is a real filename, don't use dircache */
-	    newshf->filename = ztrdup(shf->filename);
+	    if (shf->node.flags & PM_LOADDIR) {
+		dircache_set(&newshf->filename, shf->filename);
+	    } else {
+		newshf->filename = ztrdup(shf->filename);
+	    }
 	    if (shf->sticky) {
 		newshf->sticky = sticky_emulation_dup(shf->sticky, 0);
 	    } else


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-11 20:51         ` Peter Stephenson
@ 2017-01-12 20:42           ` Peter Stephenson
  2017-01-13 18:04           ` Peter Stephenson
  1 sibling, 0 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-12 20:42 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 11 Jan 2017 20:51:22 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Wed, 11 Jan 2017 11:42:32 +0000
> Peter Stephenson <p.stephenson@samsung.com> wrote:
> > The main disadvantage is each function record now contains the name of
> > the directory, so this takes extra memory --- it looks like a couple of
> > dozen k in the case of my function directory which is quite full.  This
> > could be optimised with indirection but I'm not sure it's worth the work.
> 
> This seemed like too good an idea not to follow up.  Now "autoload
> /blah/blah/*" is as efficient as the old way of doing it, too.

Update based on the previous fix, also fix a memory leak in the cache
and add a test to check what happens when bits of the cache are
deleted.  I think this is sane now.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index 716ddd4..a683032 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2958,13 +2958,13 @@ check_autoload(Shfunc shf, char *name, Options ops, int func)
 	    }
 	}
 	if (getfpfunc(shf->node.nam, NULL, &dir_path, NULL, 1)) {
-	    zsfree(shf->filename);
+	    dircache_set(&shf->filename, NULL);
 	    if (*dir_path != '/') {
 		dir_path = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP),
 				    "/", dir_path);
 		dir_path = xsymlink(dir_path, 1);
 	    }
-	    shf->filename = ztrdup(dir_path);
+	    dircache_set(&shf->filename, dir_path);
 	    shf->node.flags |= PM_LOADDIR;
 	    return 0;
 	}
@@ -3029,8 +3029,8 @@ add_autoload_function(Shfunc shf, char *funcname)
 	    *nam++ = '\0';
 	    dir = funcname;
 	}
-	zsfree(shf->filename);
-	shf->filename = ztrdup(dir);
+	dircache_set(&shf->filename, NULL);
+	dircache_set(&shf->filename, dir);
 	shf->node.flags |= PM_LOADDIR;
 	shfunctab->addnode(shfunctab, ztrdup(nam), shf);
     } else {
@@ -3280,8 +3280,8 @@ bin_functions(char *name, char **argv, Options ops, int func)
 		shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
 	    }
 	    if (*argv) {
-		zsfree(shf->filename);
-		shf->filename = ztrdup(*argv);
+		dircache_set(&shf->filename, NULL);
+		dircache_set(&shf->filename, *argv);
 		on |= PM_LOADDIR;
 	    }
 	    shf->node.flags = on;
diff --git a/Src/exec.c b/Src/exec.c
index 68c455b..7a5d2bd 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4902,6 +4902,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 	shf = (Shfunc) zalloc(sizeof(*shf));
 	shf->funcdef = prog;
 	shf->node.flags = 0;
+	/* No dircache here, not a directory */
 	shf->filename = ztrdup(scriptfilename);
 	shf->lineno = lineno;
 	/*
@@ -4934,7 +4935,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 		    freeeprog(shf->funcdef);
 		    if (shf->redir) /* shouldn't be */
 			freeeprog(shf->redir);
-		    zsfree(shf->filename);
+		    dircache_set(&shf->filename, NULL);
 		    zfree(shf, sizeof(*shf));
 		    state->pc = end;
 		    return 1;
@@ -4965,7 +4966,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 	    freeeprog(shf->funcdef);
 	    if (shf->redir) /* shouldn't be */
 		freeeprog(shf->redir);
-	    zsfree(shf->filename);
+	    dircache_set(&shf->filename, NULL);
 	    zfree(shf, sizeof(*shf));
 	    break;
 	} else {
@@ -4974,7 +4975,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 		(signum = getsignum(s + 4)) != -1) {
 		if (settrap(signum, NULL, ZSIG_FUNC)) {
 		    freeeprog(shf->funcdef);
-		    zsfree(shf->filename);
+		    dircache_set(&shf->filename, NULL);
 		    zfree(shf, sizeof(*shf));
 		    state->pc = end;
 		    return 1;
@@ -5205,7 +5206,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	    else
 		shf->funcdef = dupeprog(prog, 0);
 	    shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR);
-	    zsfree(shf->filename);
+	    dircache_set(&shf->filename, NULL);
+	    /* Full filename, don't use dircache */
 	    shf->filename = fname;
 	} else {
 	    VARARR(char, n, strlen(shf->node.nam) + 1);
@@ -5229,7 +5231,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	else
 	    shf->funcdef = dupeprog(stripkshdef(prog, shf->node.nam), 0);
 	shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR);
-	zsfree(shf->filename);
+	dircache_set(&shf->filename, NULL);
+	/* Full filename, don't use dircache */
 	shf->filename = fname;
     }
     popheap();
@@ -5620,6 +5623,8 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name)
  * Search fpath for an undefined function.  Finds the file, and returns the
  * list of its contents.
  *
+ * If test is 0, load the function; *fname is set to zalloc'ed location.
+ *
  * If test_only is 1, don't load function, just test for it:
  * - Non-null return means function was found
  * - *fname points to path at which found (not duplicated)
diff --git a/Src/hashtable.c b/Src/hashtable.c
index 2a8b585..22ebe34 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -889,7 +889,7 @@ freeshfuncnode(HashNode hn)
 	freeeprog(shf->funcdef);
     if (shf->redir)
 	freeeprog(shf->redir);
-    zsfree(shf->filename);
+    dircache_set(&shf->filename, NULL);
     if (shf->sticky) {
 	if (shf->sticky->n_on_opts)
 	    zfree(shf->sticky->on_opts,
@@ -1442,3 +1442,140 @@ freehistdata(Histent he, int unlink)
 	}
     }
 }
+
+
+/***********************************************************************
+ * Directory name cache mechanism
+ *
+ * The idea of this is that there are various shell structures,
+ * notably functions, that record the directories with which they
+ * are associated.  Rather than store the full string each time,
+ * we store a pointer to the same location and count the references.
+ * This is optimised so that retrieval is quick at the expense of
+ * searching the list when setting up the structure, which is a much
+ * rarer operation.
+ *
+ * There is nothing special about the fact that the strings are
+ * directories, except for the assumptions for efficiency that many
+ * structures will point to the same one, and that there are not too
+ * many different directories associated with the shell.
+ **********************************************************************/
+
+struct dircache_entry
+{
+    /* Name of directory in cache */
+    char *name;
+    /* Number of references to it */
+    int refs;
+};
+
+/*
+ * dircache is the cache, of length dircache_size.
+ * dircache_lastentry is the last entry used, an optimisation
+ * for multiple references to the same directory, e.g
+ * "autoload /blah/blah/\*".
+ */
+struct dircache_entry *dircache, *dircache_lastentry;
+int dircache_size;
+
+/*
+ * Set *name to point to a cached version of value.
+ * value is copied so may come from any source.
+ *
+ * If value is NULL, look for the existing value of *name (safe if this
+ * too is NULL) and remove a reference to it from the cache. If it's
+ * not found in the cache, it's assumed to be an allocated string and
+ * freed --- this currently occurs for a shell function that's been
+ * loaded as the filename is now a full path, not just a directory,
+ * though we may one day optimise this to a cached directory plus a
+ * name, too.  Note --- the function does *not* otherwise check
+ * if *name points to something already cached, so this is
+ * necessary any time *name may already be in the cache.
+ */
+
+/**/
+mod_export void
+dircache_set(char **name, char *value)
+{
+    struct dircache_entry *dcptr, *dcnew;
+
+    if (!value) {
+	if (!*name)
+	    return;
+	if (!dircache_size) {
+	    zsfree(*name);
+	    *name = NULL;
+	    return;
+	}
+
+	for (dcptr = dircache; dcptr < dircache + dircache_size; dcptr++)
+	{
+	    /* Must be a pointer much, not a string match */
+	    if (*name == dcptr->name)
+	    {
+		--dcptr->refs;
+		if (!dcptr->refs) {
+		    ptrdiff_t ind = dcptr - dircache;
+		    zsfree(dcptr->name);
+		    --dircache_size;
+
+		    if (!dircache_size) {
+			zfree(dircache, sizeof(*dircache));
+			dircache = NULL;
+			dircache_lastentry = NULL;
+			return;
+		    }
+		    dcnew = (struct dircache_entry *)
+			zalloc(dircache_size * sizeof(*dcnew));
+		    if (ind)
+			memcpy(dcnew, dircache, ind * sizeof(*dcnew));
+		    if (ind < dircache_size)
+			memcpy(dcnew + ind, dcptr + 1,
+			       (dircache_size - ind) * sizeof(*dcnew));
+		    zfree(dircache, (dircache_size+1)*sizeof(*dcnew));
+		    dircache = dcnew;
+		    dircache_lastentry = NULL;
+		}
+		*name = NULL;
+		return;
+	    }
+	}
+	zsfree(*name);
+	*name = NULL;
+    } else {
+	/*
+	 * We'll maintain the cache at exactly the right size rather
+	 * than overallocating.  The rationale here is that typically
+	 * we'll get a lot of functions in a small number of directories
+	 * so the complexity overhead of maintaining a separate count
+	 * isn't really matched by the efficiency gain.
+ 	 */
+	if (dircache_lastentry &&
+	    !strcmp(value, dircache_lastentry->name)) {
+	    *name = dircache_lastentry->name;
+	    ++dircache_lastentry->refs;
+	    return;
+	} else if (!dircache_size) {
+	    dircache_size = 1;
+	    dcptr = dircache =
+		(struct dircache_entry *)zalloc(sizeof(*dircache));
+	} else {
+	    for (dcptr = dircache; dcptr < dircache + dircache_size; dcptr++)
+	    {
+		if (!strcmp(value, dcptr->name)) {
+		    *name = dcptr->name;
+		    ++dcptr->refs;
+		    return;
+		}
+	    }
+	    ++dircache_size;
+	    dircache = (struct dircache_entry *)
+		zrealloc(dircache, sizeof(*dircache) * dircache_size);
+	    dcptr = dircache + dircache_size - 1;
+	}
+	dcptr->name = ztrdup(value);
+	*name = dcptr->name;
+	dcptr->refs = 1;
+	dircache_lastentry = dcptr;
+    }
+}
diff --git a/Src/signals.c b/Src/signals.c
index 9e05add..2de5743 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -811,6 +811,7 @@ dosavetrap(int sig, int level)
 	    newshf->node.nam = ztrdup(shf->node.nam);
 	    newshf->node.flags = shf->node.flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
+	    /* Assume this is a real filename, don't use dircache */
 	    newshf->filename = ztrdup(shf->filename);
 	    if (shf->sticky) {
 		newshf->sticky = sticky_emulation_dup(shf->sticky, 0);
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index 7100280..370394b 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -429,6 +429,45 @@
 0: autoload -X interaction with absolute filename used for source location
 >Function was loaded correctly.
 
+  (
+    fpath=()
+    mkdir extra2
+    for f in fun2a fun2b; do
+      print "print $f" >extra2/$f
+    done
+    repeat 3; do
+      autoload $PWD/extra2/fun2{a,b} $PWD/extra/spec
+      fun2a
+      fun2b
+      spec
+      unfunction fun2a fun2b spec
+      autoload $PWD/extra2/fun2{a,b} $PWD/extra/spec
+      spec
+      fun2b
+      fun2a
+      unfunction fun2a fun2b spec
+    done
+  )
+0: Exercise the directory name cache for autoloads
+>fun2a
+>fun2b
+>I have been loaded by explicit path.
+>I have been loaded by explicit path.
+>fun2b
+>fun2a
+>fun2a
+>fun2b
+>I have been loaded by explicit path.
+>I have been loaded by explicit path.
+>fun2b
+>fun2a
+>fun2a
+>fun2b
+>I have been loaded by explicit path.
+>I have been loaded by explicit path.
+>fun2b
+>fun2a
+
 %clean
 
  rm -f file.in file.out


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-11 11:42       ` Peter Stephenson
  2017-01-11 20:51         ` Peter Stephenson
@ 2017-01-11 21:13         ` Peter Stephenson
  1 sibling, 0 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-11 21:13 UTC (permalink / raw)
  To: Zsh hackers list

Completion for this.

Didn't really understand what I was doing here, but it seems to work...

pws

diff --git a/Completion/Zsh/Command/_typeset b/Completion/Zsh/Command/_typeset
index 94f63cf..0961e98 100644
--- a/Completion/Zsh/Command/_typeset
+++ b/Completion/Zsh/Command/_typeset
@@ -21,6 +21,7 @@ allargs=(
   X '+X[immediately autoload function]'
   Z "($fopts -A -E -F -i)-Z+[right justify and fill with leading zeros]:width"
   a "($fopts -A -E -F -T -i)-a[specify that arguments refer to arrays]"
+  df "-d[default absolute path autoload to fpath]"
   f "($popts)-f[specify that arguments refer to functions]"
   g "($fopts -T)-+g[do not restrict parameter to local scope]"
   h "($fopts -T)-+h[hide specialness of parameter]"
@@ -31,6 +32,8 @@ allargs=(
   m '(-A -E -F -T -i)-m[treat arguments as patterns]'
   p '-p[output parameters in form of calls to typeset]'
   r '(-f)-+r[mark parameters as readonly]'
+  rf '-r[remember autoload path]'
+  Rf '-R[remember autoload path, error if not found]'
   t '(-T)-+t[tag parameters and turn on execution tracing for functions]'
   tf '(-T)-+t[turn on execution tracing for functions]'
   tp '(-T)-+t[tag parameters]'
@@ -46,7 +49,7 @@ use="AEFHLRTUZafghiklmprtuxz"
 
 case ${service} in
   autoload)
-    use="UTXktwz"
+    use="URTXdkrtwz"
     func=f
   ;;
   float) use="EFHghlprtux";;
@@ -74,7 +77,7 @@ onopts=${(j..)${${words[1,CURRENT-1]:#^-*}##-}}
 offopts=${(j..)${${words[1,CURRENT-1]:#^+*}##+}}
 
 for ((i=1;i<=$#use;++i)); do
-  args+=( ${allargs[${use[$i]}${${(s::)use[$i]}[(r)[UutT]]:+$func}]} )
+  args+=( ${allargs[${use[$i]}${${(s::)use[$i]}[(r)[dUurRtT]]:+$func}]} )
 done
 
 _arguments -C -s -A "-*" -S "${args[@]}" '*::vars:= ->vars_eq'
@@ -90,12 +93,17 @@ if [[ "$state" = vars_eq ]]; then
     elif (( $+opt_args[-w] )); then
       _wanted files expl 'zwc file' _files -g '*.zwc(-.)'
     elif [[ $service = autoload || -n $opt_args[(i)-[uU]] ]]; then
-      args=(${^fpath}/*(-.:t))
-      # Filter out functions already loaded or marked for autoload.
-      local -a funckeys
-      funckeys=(${(k)functions})
-      args=(${args:|funckeys})
-      _wanted functions expl 'shell function' compadd -a args
+      if [[ $PREFIX[1] = / ]]; then
+	# Autoload by absolute path
+	_files
+      else
+	  args=(${^fpath}/*(-.:t))
+	  # Filter out functions already loaded or marked for autoload.
+	  local -a funckeys
+	  funckeys=(${(k)functions})
+	  args=(${args:|funckeys})
+	  _wanted functions expl 'shell function' compadd -a args
+      fi
     elif [[ -n $onopts$offopts ]]; then
       if [[ -n $offopts ]]; then
 	args=(${(f)"$(functions +${offopts//[^UXkmtTuz]/})"})


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-11 11:42       ` Peter Stephenson
@ 2017-01-11 20:51         ` Peter Stephenson
  2017-01-12 20:42           ` Peter Stephenson
  2017-01-13 18:04           ` Peter Stephenson
  2017-01-11 21:13         ` Peter Stephenson
  1 sibling, 2 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-11 20:51 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 11 Jan 2017 11:42:32 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> The main disadvantage is each function record now contains the name of
> the directory, so this takes extra memory --- it looks like a couple of
> dozen k in the case of my function directory which is quite full.  This
> could be optimised with indirection but I'm not sure it's worth the work.

This seemed like too good an idea not to follow up.  Now "autoload
/blah/blah/*" is as efficient as the old way of doing it, too.

With a bit of work this can be expanded to paths for already loaded
functions, and possibly elsewhere though I think the pickings are
a lot smaller.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index b986dd8..0ed081a 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2957,13 +2957,13 @@ check_autoload(Shfunc shf, char *name, Options ops, int func)
 	    }
 	}
 	if (getfpfunc(shf->node.nam, NULL, &dir_path, NULL, 1)) {
-	    zsfree(shf->filename);
+	    dircache_set(&shf->filename, NULL);
 	    if (*dir_path != '/') {
 		dir_path = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP),
 				    "/", dir_path);
 		dir_path = xsymlink(dir_path, 1);
 	    }
-	    shf->filename = ztrdup(dir_path);
+	    dircache_set(&shf->filename, dir_path);
 	    return 0;
 	}
 	if (OPT_ISSET(ops,'R')) {
@@ -3026,8 +3026,8 @@ add_autoload_function(Shfunc shf, char *funcname)
 	    *nam++ = '\0';
 	    dir = funcname;
 	}
-	zsfree(shf->filename);
-	shf->filename = ztrdup(dir);
+	dircache_set(&shf->filename, NULL);
+	dircache_set(&shf->filename, dir);
 	shfunctab->addnode(shfunctab, ztrdup(nam), shf);
     } else {
 	shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
@@ -3276,8 +3276,8 @@ bin_functions(char *name, char **argv, Options ops, int func)
 		shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
 	    }
 	    if (*argv) {
-		zsfree(shf->filename);
-		shf->filename = ztrdup(*argv);
+		dircache_set(&shf->filename, NULL);
+		dircache_set(&shf->filename, *argv);
 	    }
 	    shf->node.flags = on;
 	    ret = eval_autoload(shf, funcname, ops, func);
diff --git a/Src/exec.c b/Src/exec.c
index 7bec7ce..10a5249 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4902,6 +4902,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 	shf = (Shfunc) zalloc(sizeof(*shf));
 	shf->funcdef = prog;
 	shf->node.flags = 0;
+	/* No dircache here, not a directory */
 	shf->filename = ztrdup(scriptfilename);
 	shf->lineno = lineno;
 	/*
@@ -4934,7 +4935,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 		    freeeprog(shf->funcdef);
 		    if (shf->redir) /* shouldn't be */
 			freeeprog(shf->redir);
-		    zsfree(shf->filename);
+		    dircache_set(&shf->filename, NULL);
 		    zfree(shf, sizeof(*shf));
 		    state->pc = end;
 		    return 1;
@@ -4965,7 +4966,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 	    freeeprog(shf->funcdef);
 	    if (shf->redir) /* shouldn't be */
 		freeeprog(shf->redir);
-	    zsfree(shf->filename);
+	    dircache_set(&shf->filename, NULL);
 	    zfree(shf, sizeof(*shf));
 	    break;
 	} else {
@@ -4974,7 +4975,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 		(signum = getsignum(s + 4)) != -1) {
 		if (settrap(signum, NULL, ZSIG_FUNC)) {
 		    freeeprog(shf->funcdef);
-		    zsfree(shf->filename);
+		    dircache_set(&shf->filename, NULL);
 		    zfree(shf, sizeof(*shf));
 		    state->pc = end;
 		    return 1;
@@ -5204,7 +5205,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	    else
 		shf->funcdef = dupeprog(prog, 0);
 	    shf->node.flags &= ~PM_UNDEFINED;
-	    zsfree(shf->filename);
+	    dircache_set(&shf->filename, NULL);
+	    /* Full filename, don't use dircache */
 	    shf->filename = fname;
 	} else {
 	    VARARR(char, n, strlen(shf->node.nam) + 1);
@@ -5228,7 +5230,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	else
 	    shf->funcdef = dupeprog(stripkshdef(prog, shf->node.nam), 0);
 	shf->node.flags &= ~PM_UNDEFINED;
-	zsfree(shf->filename);
+	dircache_set(&shf->filename, NULL);
+	/* Full filename, don't use dircache */
 	shf->filename = fname;
     }
     popheap();
@@ -5619,6 +5622,8 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name)
  * Search fpath for an undefined function.  Finds the file, and returns the
  * list of its contents.
  *
+ * If test is 0, load the function; *fname is set to zalloc'ed location.
+ *
  * If test_only is 1, don't load function, just test for it:
  * - Non-null return means function was found
  * - *fname points to path at which found (not duplicated)
diff --git a/Src/hashtable.c b/Src/hashtable.c
index 2a8b585..110cea6 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -889,7 +889,7 @@ freeshfuncnode(HashNode hn)
 	freeeprog(shf->funcdef);
     if (shf->redir)
 	freeeprog(shf->redir);
-    zsfree(shf->filename);
+    dircache_set(&shf->filename, NULL);
     if (shf->sticky) {
 	if (shf->sticky->n_on_opts)
 	    zfree(shf->sticky->on_opts,
@@ -1442,3 +1442,134 @@ freehistdata(Histent he, int unlink)
 	}
     }
 }
+
+
+/***********************************************************************
+ * Directory name cache mechanism
+ *
+ * The idea of this is that there are various shell structures,
+ * notably functions, that record the directories with which they
+ * are associated.  Rather than store the full string each time,
+ * we store a pointer to the same location and count the references.
+ * This is optimised so that retrieval is quick at the expense of
+ * searching the list when setting up the structure, which is a much
+ * rarer operation.
+ *
+ * There is nothing special about the fact that the strings are
+ * directories, except for the assumptions for efficiency that many
+ * structures will point to the same one, and that there are not too
+ * many different directories associated with the shell.
+ **********************************************************************/
+
+struct dircache_entry
+{
+    /* Name of directory in cache */
+    char *name;
+    /* Number of references to it */
+    int refs;
+};
+
+/*
+ * dircache is the cache, of length dircache_size.
+ * dircache_lastentry is the last entry used, an optimisation
+ * for multiple references to the same directory, e.g
+ * "autoload /blah/blah/\*".
+ */
+struct dircache_entry *dircache, *dircache_lastentry;
+int dircache_size;
+
+/*
+ * Set *name to point to a cached version of value.
+ * value is copied so may come from any source.
+ *
+ * If value is NULL, look for the existing value of *name (safe if this
+ * too is NULL) and remove a reference to it from the cache. If it's
+ * not found in the cache, it's assumed to be an allocated string and
+ * freed --- this currently occurs for a shell function that's been
+ * loaded as the filename is now a full path, not just a directory,
+ * though we may one day optimise this to a cached directory plus a
+ * name, too.  Note --- the function does *not* otherwise check
+ * if *name points to something already cached, so this is
+ * necessary any time *name may already be in the cache.
+ */
+
+/**/
+mod_export void
+dircache_set(char **name, char *value)
+{
+    struct dircache_entry *dcptr, *dcnew;
+
+    if (!value) {
+	if (!dircache_size || !*name)
+	    return;
+
+	for (dcptr = dircache; dcptr < dircache + dircache_size; dcptr++)
+	{
+	    /* Must be a pointer much, not a string match */
+	    if (*name == dcptr->name)
+	    {
+		--dcptr->refs;
+		if (!dcptr->refs) {
+		    ptrdiff_t ind = dcptr - dircache;
+		    zsfree(dcptr->name);
+		    --dircache_size;
+
+		    if (!dircache_size) {
+			zfree(dircache, sizeof(*dircache));
+			dircache = NULL;
+			return;
+		    }
+		    dcnew = (struct dircache_entry *)
+			zalloc(dircache_size * sizeof(*dcnew));
+		    if (ind)
+			memcpy(dcnew, dircache, ind * sizeof(*dcnew));
+		    if (ind < dircache_size)
+			memcpy(dcnew + ind, dcptr + 1,
+			       (dircache_size - ind) * sizeof(*dcnew));
+		    zfree(dircache, (dircache_size+1)*sizeof(*dcnew));
+		    dircache = dcnew;
+		    dircache_lastentry = NULL;
+		}
+		*name = NULL;
+		return;
+	    }
+	}
+	zsfree(*name);
+	*name = NULL;
+    } else {
+	/*
+	 * We'll maintain the cache at exactly the right size rather
+	 * than overallocating.  The rationale here is that typically
+	 * we'll get a lot of functions in a small number of directories
+	 * so the complexity overhead of maintaining a separate count
+	 * isn't really matched by the efficiency gain.
+ 	 */
+	if (!dircache_size) {
+	    dircache_size = 1;
+	    dcptr = dircache =
+		(struct dircache_entry *)zalloc(sizeof(*dircache));
+	} else if (dircache_lastentry &&
+		   !strcmp(value, dircache_lastentry->name)) {
+	    *name = dircache_lastentry->name;
+	    ++dircache_lastentry->refs;
+	    return;
+	} else {
+	    for (dcptr = dircache; dcptr < dircache + dircache_size; dcptr++)
+	    {
+		if (!strcmp(value, dcptr->name)) {
+		    *name = dcptr->name;
+		    ++dcptr->refs;
+		    return;
+		}
+	    }
+	    ++dircache_size;
+	    dircache = (struct dircache_entry *)
+		zrealloc(dircache, sizeof(*dircache) * dircache_size);
+	    dcptr = dircache + dircache_size - 1;
+	}
+	dcptr->name = ztrdup(value);
+	*name = dcptr->name;
+	dcptr->refs = 1;
+	dircache_lastentry = dcptr;
+    }
+}
diff --git a/Src/signals.c b/Src/signals.c
index 9e05add..2de5743 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -811,6 +811,7 @@ dosavetrap(int sig, int level)
 	    newshf->node.nam = ztrdup(shf->node.nam);
 	    newshf->node.flags = shf->node.flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
+	    /* Assume this is a real filename, don't use dircache */
 	    newshf->filename = ztrdup(shf->filename);
 	    if (shf->sticky) {
 		newshf->sticky = sticky_emulation_dup(shf->sticky, 0);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2017-01-10 19:31     ` Peter Stephenson
@ 2017-01-11 11:42       ` Peter Stephenson
  2017-01-11 20:51         ` Peter Stephenson
  2017-01-11 21:13         ` Peter Stephenson
  0 siblings, 2 replies; 35+ messages in thread
From: Peter Stephenson @ 2017-01-11 11:42 UTC (permalink / raw)
  To: Zsh hackers list

On Tue, 10 Jan 2017 19:31:02 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>     Add features associated with autoloading a function using an absolute
>     path.

I pushed this, with the merge fixed.


The main feature most people might be interested in is that you can
now replace

fpath=(/path/to/my/functions)
autoload -Uz /path/to/my/functions/*(.:t)

with the more direct

autoload -Uz /path/to/my/functions/*(.)

The advantages are

- Your are guaranteed the right file will be autoloaded as $fpath is
not used.  If you previously autoloaded the file generically the record
is updated.

- "functions", "which" etc. tell you the directory that will be used.

The main disadvantage is each function record now contains the name of
the directory, so this takes extra memory --- it looks like a couple of
dozen k in the case of my function directory which is quite full.  This
could be optimised with indirection but I'm not sure it's worth the work.


The other possibility I can see being of general interest is that if you
have code that needs functions as a prerequisite, you can make the
calling code fail if the function isn't found immediately at the point
of the autoload by using the -R option, which also ensures that later
changes to fpath don't change the function that will be loaded.

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2016-12-12 16:31   ` Peter Stephenson
  2016-12-12 18:09     ` Bart Schaefer
@ 2017-01-10 19:31     ` Peter Stephenson
  2017-01-11 11:42       ` Peter Stephenson
  1 sibling, 1 reply; 35+ messages in thread
From: Peter Stephenson @ 2017-01-10 19:31 UTC (permalink / raw)
  To: Zsh hackers list

Here is an improved patch for this with the features noted in the commit
message from the branch.

commit 6f74a54d0cc48e9d9928f18eaead0460963f7d95
Author: Peter Stephenson <p.w.stephenson@ntlworld.com>
Date:   Thu Dec 15 18:57:57 2016 +0000

    Add features associated with autoloading a function using an absolute
    path.
    
    -d defaults to normal fpath
    
    -r remembers the path without actually loading.  May be combined with -d.
    
    -R does the same but it's an error if not found
    
    -X can now take a directory path: this is used to output not yet loaded
    functions that have an associated path.

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index af336ef..23ccf76 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -147,20 +147,39 @@ ifnzman(noderef(Aliasing)).
 findex(autoload)
 cindex(functions, autoloading)
 cindex(autoloading functions)
-item(tt(autoload) [ {tt(PLUS())|tt(-)}tt(TUXkmtz) ] [ tt(-w) ] [ var(name) ... ])(
+item(tt(autoload) [ {tt(PLUS())|tt(-)}tt(RTUXdkmrtz) ] [ tt(-w) ] [ var(name) ... ])(
 vindex(fpath, searching)
-Equivalent to tt(functions -u), with the exception of tt(-X)/tt(+X) and
-tt(-w).  See the section `Autoloading Functions' in ifzman(zmanref(zshmisc))\
+See the section `Autoloading Functions' in ifzman(zmanref(zshmisc))\
 ifnzman(noderef(Functions)) for full details.  The tt(fpath) parameter
 will be searched to find the function definition when the function is
 first referenced.
 
-The flag tt(-X) may be used only inside a shell function, and may not be
-followed by a var(name).  It causes the calling function to be marked for
-autoloading and then immediately loaded and executed, with the current
-array of positional parameters as arguments.  This replaces the previous
-definition of the function.  If no function definition is found, an error
-is printed and the function remains undefined and marked for autoloading.
+If var(name) consists of an absolute path, the function is defined to
+load from the file given (searching as usual for dump files in the given
+location).  The name of the function is the base name (non-directory
+part) of the file.  It is normally an error if the function is not found
+in the given location; however, if the option tt(-d) is given, searching
+for the function defaults to tt($fpath).
+
+If the option tt(-r) or tt(-R) is given, the function is searched for
+immediately and the location is recorded internally for use when the
+function is executed; a relative path is expanded using the value of
+tt($PWD).  This protects against a change to tt($fpath) after the call
+to tt(autoload).  With tt(-r), if the function is not found, it is
+silently left unresolved until execution; with tt(-R), an error message
+is printed and command processing aborted immediately the search fails,
+i.e. at the tt(autoload) command rather than at function execution..
+
+The flag tt(-X) may be used only inside a shell function.  It causes the
+calling function to be marked for autoloading and then immediately
+loaded and executed, with the current array of positional parameters as
+arguments.  This replaces the previous definition of the function.  If
+no function definition is found, an error is printed and the function
+remains undefined and marked for autoloading.  If an argument is given,
+it is used as a directory (i.e. it does not include the name of the
+function) in which the function is to be found; this may be combined
+with the tt(-d) option to allow the function search to default to tt($fpath)
+if it is not in the given location.
 
 The flag tt(+X) attempts to load each var(name) as an autoloaded function,
 but does em(not) execute it.  The exit status is zero (success) if the
@@ -176,6 +195,13 @@ If the tt(-m) flag is also given each var(name) is treated as a
 pattern and all functions already marked for autoload that match the
 pattern are loaded.
 
+With the tt(-t) flag, turn on execution tracing; with tt(-T), turn on
+execution tracing only for the current function, turning it off on entry
+to any called functions that do not also have tracing enabled.
+
+With the tt(-U) flag, alias expansion is suppressed when the function is
+loaded.
+
 With the tt(-w) flag, the var(name)s are taken as names of files compiled
 with the tt(zcompile) builtin, and all functions defined in them are
 marked for autoloading.
@@ -193,6 +219,10 @@ example(emulate zsh -c 'autoload -Uz var(func)')
 
 arranges that when var(func) is loaded the shell is in native tt(zsh)
 emulation, and this emulation is also applied when var(func) is run.
+
+Some of the functions of tt(autoload) are also provided by tt(functions
+-u) or tt(functions -U), but tt(autoload) is a more comprehensive
+interface.
 )
 findex(bg)
 cindex(jobs, backgrounding)
@@ -811,7 +841,8 @@ xitem(tt(functions -M) var(mathfn) [ var(min) [ var(max) [ var(shellfn) ] ] ])
 xitem(tt(functions -M) [ tt(-m) var(pattern) ... ])
 item(tt(functions +M) [ tt(-m) ] var(mathfn) ... )(
 Equivalent to tt(typeset -f), with the exception of the tt(-x) and
-tt(-M) options.
+tt(-M) options.  For tt(functions -u) and tt(functions -U), see
+tt(autoload), which provides additional options.
 
 The tt(-x) option indicates that any functions output will have
 each leading tab for indentation, added by the shell to show syntactic
@@ -2034,7 +2065,9 @@ expansion to be suppressed when the function is loaded.  See the
 description of the `tt(autoload)' builtin for details.
 
 Note that the builtin tt(functions) provides the same basic capabilities
-as tt(typeset -f) but gives access to a few extra options.
+as tt(typeset -f) but gives access to a few extra options; tt(autoload)
+gives further additional options for the case tt(typeset -fu) and
+tt(typeset -fU).
 )
 item(tt(-h))(
 Hide: only useful for special parameters (those marked `<S>' in the table in
diff --git a/README b/README
index 414ee0b..1cce246 100644
--- a/README
+++ b/README
@@ -32,7 +32,7 @@ details, see the documentation.
 Incompatibilities since 5.3.1
 -----------------------------
 
-The default behaviour of code like the following has changed:
+1) The default behaviour of code like the following has changed:
 
   alias foo='noglob foo'
   foo() { print function body; }
@@ -52,8 +52,18 @@ ALIAS_FUNC_DEF, has been added, which can be set to make the shell
 behave as in previous versions.  It is in any case recommended to use
 the "function" keyword, as aliases are not expanded afterwards.
 
-Incompatibilities between 5.0.8 and 5.3.1
------------------------------------------
+2) It was an undocumented, and largely useless, feature that a function
+autoloaded with an absolute path was searched for along the normal fpath
+(as if the leading / was missing) and, if found, loaded under the full
+name including the leading slash.  This has been replaced with the more
+useful feature that the function is searched for only at the given
+absolute path; the name of the function is the base name of the file.
+Note that functions including a non-leading / behave as before,
+e.g. if `dir/name' is found anywhere under a directory in $fpath it is
+loaded as a function named `dir/name'.
+
+Incompatibilities between 5.0.8 and 5.3
+----------------------------------------
 
 1) In character classes delimited by "[" and "]" within patterns, whether
 used for filename generation (globbing) or other forms of pattern
@@ -182,6 +192,10 @@ following example illustrates how this differs from past versions.
      4 4 => 1  |  4 4 => 0   **
      4 5 => 1  |  4 5 => 1
 
+<<<<<<< HEAD
+=======
+
+>>>>>>> Add features associated with autoloading a function using an absolute
 The behaviour of the parameter flag (P) has changed when it appears
 in a nested parameter group, in order to make it more useful in
 such cases.  A (P) in the outermost parameter group behaves as
diff --git a/Src/builtin.c b/Src/builtin.c
index 0f04d14..78d67ca 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -46,7 +46,7 @@ static struct builtin builtins[] =
     BUILTIN(".", BINF_PSPECIAL, bin_dot, 1, -1, 0, NULL, NULL),
     BUILTIN(":", BINF_PSPECIAL, bin_true, 0, -1, 0, NULL, NULL),
     BUILTIN("alias", BINF_MAGICEQUALS | BINF_PLUSOPTS, bin_alias, 0, -1, 0, "Lgmrs", NULL),
-    BUILTIN("autoload", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "mktTUwXz", "u"),
+    BUILTIN("autoload", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "dmktrRTUwXz", "u"),
     BUILTIN("bg", 0, bin_fg, 0, -1, BIN_BG, NULL, NULL),
     BUILTIN("break", BINF_PSPECIAL, bin_break, 0, 1, BIN_BREAK, NULL, NULL),
     BUILTIN("bye", 0, bin_break, 0, 1, BIN_EXIT, NULL, NULL),
@@ -2922,9 +2922,59 @@ eval_autoload(Shfunc shf, char *name, Options ops, int func)
     }
 
     return !loadautofn(shf, (OPT_ISSET(ops,'k') ? 2 :
-			     (OPT_ISSET(ops,'z') ? 0 : 1)), 1);
+			     (OPT_ISSET(ops,'z') ? 0 : 1)), 1,
+		       OPT_ISSET(ops,'d'));
 }
 
+/* Helper for bin_functions() for -X and -r options */
+
+/**/
+static int
+check_autoload(Shfunc shf, char *name, Options ops, int func)
+{
+    if (OPT_ISSET(ops,'X'))
+    {
+	return eval_autoload(shf, name, ops, func);
+    }
+    if (OPT_ISSET(ops,'r') || OPT_ISSET(ops,'R'))
+    {
+	char *dir_path;
+	if (shf->filename) {
+	    char *spec_path[2];
+	    spec_path[0] = shf->filename;
+	    spec_path[1] = NULL;
+	    if (getfpfunc(shf->node.nam, NULL, &dir_path, spec_path, 1)) {
+		/* shf->filename is already correct. */
+		return 0;
+	    }
+	    if (!OPT_ISSET(ops,'d')) {
+		if (OPT_ISSET(ops,'R')) {
+		    zerr("%s: function definition file not found",
+			 shf->node.nam);
+		    return 1;
+		}
+		return 0;
+	    }
+	}
+	if (getfpfunc(shf->node.nam, NULL, &dir_path, NULL, 1)) {
+	    zsfree(shf->filename);
+	    if (*dir_path != '/') {
+		dir_path = zhtricat(metafy(zgetcwd(), -1, META_HEAPDUP),
+				    "/", dir_path);
+		dir_path = xsymlink(dir_path, 1);
+	    }
+	    shf->filename = ztrdup(dir_path);
+	    return 0;
+	}
+	if (OPT_ISSET(ops,'R')) {
+	    zerr("%s: function definition file not found",
+		 shf->node.nam);
+	    return 1;
+	}
+	/* with -r, we don't flag an error, just let it be found later. */
+    }
+    return 0;
+}
 
 /* List a user-defined math function. */
 static void
@@ -2962,6 +3012,28 @@ listusermathfunc(MathFunc p)
 }
 
 
+static void
+add_autoload_function(Shfunc shf, char *funcname)
+{
+    char *nam;
+    if (*funcname == '/' && funcname[1] &&
+	(nam = strrchr(funcname, '/')) && nam[1]) {
+	char *dir;
+	nam = strrchr(funcname, '/');
+	if (nam == funcname) {
+	    dir = "/";
+	} else {
+	    *nam++ = '\0';
+	    dir = funcname;
+	}
+	zsfree(shf->filename);
+	shf->filename = ztrdup(dir);
+	shfunctab->addnode(shfunctab, ztrdup(nam), shf);
+    } else {
+	shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
+    }
+}
+
 /* Display or change the attributes of shell functions.   *
  * If called as autoload, it will define a new autoloaded *
  * (undefined) shell function.                            */
@@ -3007,10 +3079,17 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	off |= PM_KSHSTORED;
 	roff |= PM_KSHSTORED;
     }
+    if (OPT_MINUS(ops,'d')) {
+	on |= PM_CUR_FPATH;
+	off |= PM_CUR_FPATH;
+    } else if (OPT_PLUS(ops,'d')) {
+	off |= PM_CUR_FPATH;
+	roff |= PM_CUR_FPATH;
+    }
 
     if ((off & PM_UNDEFINED) || (OPT_ISSET(ops,'k') && OPT_ISSET(ops,'z')) ||
 	(OPT_ISSET(ops,'x') && !OPT_HASARG(ops,'x')) ||
-	(OPT_MINUS(ops,'X') && (OPT_ISSET(ops,'m') || *argv || !scriptname))) {
+	(OPT_MINUS(ops,'X') && (OPT_ISSET(ops,'m') || !scriptname))) {
 	zwarnnam(name, "invalid option(s)");
 	return 1;
     }
@@ -3165,47 +3244,55 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	return returnval;
     }
 
-    /* If no arguments given, we will print functions.  If flags *
-     * are given, we will print only functions containing these  *
-     * flags, else we'll print them all.                         */
-    if (!*argv) {
-	int ret = 0;
-
+    if (OPT_MINUS(ops,'X')) {
+	Funcstack fs;
+	char *funcname = NULL;
+	int ret;
+	if (*argv && argv[1]) {
+	    zwarnnam(name, "-X: too many arguments");
+	    return 1;
+	}
 	queue_signals();
-	if (OPT_MINUS(ops,'X')) {
-	    Funcstack fs;
-	    char *funcname = NULL;
-	    for (fs = funcstack; fs; fs = fs->prev) {
-		if (fs->tp == FS_FUNC) {
-		    /*
-		     * dupstring here is paranoia but unlikely to be
-		     * problematic
-		     */
-		    funcname = dupstring(fs->name);
-		    break;
-		}
+	for (fs = funcstack; fs; fs = fs->prev) {
+	    if (fs->tp == FS_FUNC) {
+		/*
+		 * dupstring here is paranoia but unlikely to be
+		 * problematic
+		 */
+		funcname = dupstring(fs->name);
+		break;
 	    }
-	    if (!funcname)
-	    {
-		zerrnam(name, "bad autoload");
-		ret = 1;
+	}
+	if (!funcname)
+	{
+	    zerrnam(name, "bad autoload");
+	    ret = 1;
+	} else {
+	    if ((shf = (Shfunc) shfunctab->getnode(shfunctab, funcname))) {
+		DPUTS(!shf->funcdef,
+		      "BUG: Calling autoload from empty function");
 	    } else {
-		if ((shf = (Shfunc) shfunctab->getnode(shfunctab, funcname))) {
-		    DPUTS(!shf->funcdef,
-			  "BUG: Calling autoload from empty function");
-		} else {
-		    shf = (Shfunc) zshcalloc(sizeof *shf);
-		    shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
-		}
-		shf->node.flags = on;
-		ret = eval_autoload(shf, funcname, ops, func);
+		shf = (Shfunc) zshcalloc(sizeof *shf);
+		shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
 	    }
-	} else {
-	    if (OPT_ISSET(ops,'U') && !OPT_ISSET(ops,'u'))
+	    if (*argv)
+		shf->filename = ztrdup(*argv);
+	    shf->node.flags = on;
+	    ret = eval_autoload(shf, funcname, ops, func);
+	}
+	unqueue_signals();
+	return ret;
+    } else if (!*argv) {
+	/* If no arguments given, we will print functions.  If flags *
+	 * are given, we will print only functions containing these  *
+	 * flags, else we'll print them all.                         */
+	int ret = 0;
+
+	queue_signals();
+	if (OPT_ISSET(ops,'U') && !OPT_ISSET(ops,'u'))
 		on &= ~PM_UNDEFINED;
 	    scanshfunc(1, on|off, DISABLED, shfunctab->printnode,
 		       pflags, expand);
-	}
 	unqueue_signals();
 	return ret;
     }
@@ -3231,8 +3318,8 @@ bin_functions(char *name, char **argv, Options ops, int func)
 				!(shf->node.flags & DISABLED)) {
 				shf->node.flags = (shf->node.flags |
 					      (on & ~PM_UNDEFINED)) & ~off;
-				if (OPT_ISSET(ops,'X') &&
-				    eval_autoload(shf, shf->node.nam, ops, func)) {
+				if (check_autoload(shf, shf->node.nam,
+						   ops, func)) {
 				    returnval = 1;
 				}
 			    }
@@ -3258,8 +3345,7 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	    if (on|off) {
 		/* turn on/off the given flags */
 		shf->node.flags = (shf->node.flags | (on & ~PM_UNDEFINED)) & ~off;
-		if (OPT_ISSET(ops,'X') &&
-		    eval_autoload(shf, shf->node.nam, ops, func))
+		if (check_autoload(shf, shf->node.nam, ops, func))
 		    returnval = 1;
 	    } else
 		/* no flags, so just print */
@@ -3282,7 +3368,7 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	    shf->node.flags = on;
 	    shf->funcdef = mkautofn(shf);
 	    shfunc_set_sticky(shf);
-	    shfunctab->addnode(shfunctab, ztrdup(*argv), shf);
+	    add_autoload_function(shf, *argv);
 
 	    if (signum != -1) {
 		if (settrap(signum, NULL, ZSIG_FUNC)) {
@@ -3293,8 +3379,7 @@ bin_functions(char *name, char **argv, Options ops, int func)
 		}
 	    }
 
-	    if (ok && OPT_ISSET(ops,'X') &&
-		eval_autoload(shf, shf->node.nam, ops, func))
+	    if (ok && check_autoload(shf, shf->node.nam, ops, func))
 		returnval = 1;
 	} else
 	    returnval = 1;
diff --git a/Src/exec.c b/Src/exec.c
index a439aec..a41d05b 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3124,7 +3124,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	if (is_shfunc)
 	    shf = (Shfunc)hn;
 	else {
-	    shf = loadautofn(state->prog->shf, 1, 0);
+	    shf = loadautofn(state->prog->shf, 1, 0, 0);
 	    if (shf)
 		state->prog->shf = shf;
 	    else {
@@ -5142,7 +5142,7 @@ execautofn(Estate state, UNUSED(int do_exec))
 {
     Shfunc shf;
 
-    if (!(shf = loadautofn(state->prog->shf, 1, 0)))
+    if (!(shf = loadautofn(state->prog->shf, 1, 0, 0)))
 	return 1;
 
     state->prog->shf = shf;
@@ -5151,7 +5151,7 @@ execautofn(Estate state, UNUSED(int do_exec))
 
 /**/
 Shfunc
-loadautofn(Shfunc shf, int fksh, int autol)
+loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 {
     int noalias = noaliases, ksh = 1;
     Eprog prog;
@@ -5160,7 +5160,18 @@ loadautofn(Shfunc shf, int fksh, int autol)
     pushheap();
 
     noaliases = (shf->node.flags & PM_UNALIASED);
-    prog = getfpfunc(shf->node.nam, &ksh, &fname);
+    if (shf->filename && shf->filename[0] == '/')
+    {
+	char *spec_path[2];
+	spec_path[0] = dupstring(shf->filename);
+	spec_path[1] = NULL;
+	prog = getfpfunc(shf->node.nam, &ksh, &fname, spec_path, 0);
+	if (prog == &dummy_eprog &&
+	    (current_fpath || (shf->node.flags & PM_CUR_FPATH)))
+	    prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0);
+    }
+    else
+	prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0);
     noaliases = noalias;
 
     if (ksh == 1) {
@@ -5602,12 +5613,18 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name)
     unqueue_signals();
 }
 
-/* Search fpath for an undefined function.  Finds the file, and returns the *
- * list of its contents.                                                    */
+/*
+ * Search fpath for an undefined function.  Finds the file, and returns the
+ * list of its contents.
+ *
+ * If test_only is 1, don't load function, just test for it:
+ * - Non-null return means function was found
+ * - *fname points to path at which found (not duplicated)
+ */
 
 /**/
 Eprog
-getfpfunc(char *s, int *ksh, char **fname)
+getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
 {
     char **pp, buf[PATH_MAX+1];
     off_t len;
@@ -5616,7 +5633,7 @@ getfpfunc(char *s, int *ksh, char **fname)
     Eprog r;
     int fd;
 
-    pp = fpath;
+    pp = alt_path ? alt_path : fpath;
     for (; *pp; pp++) {
 	if (strlen(*pp) + strlen(s) + 1 >= PATH_MAX)
 	    continue;
@@ -5624,9 +5641,9 @@ getfpfunc(char *s, int *ksh, char **fname)
 	    sprintf(buf, "%s/%s", *pp, s);
 	else
 	    strcpy(buf, s);
-	if ((r = try_dump_file(*pp, s, buf, ksh))) {
+	if ((r = try_dump_file(*pp, s, buf, ksh, test_only))) {
 	    if (fname)
-		*fname = ztrdup(buf);
+		*fname = test_only ? *pp : ztrdup(buf);
 	    return r;
 	}
 	unmetafy(buf, NULL);
@@ -5634,6 +5651,11 @@ getfpfunc(char *s, int *ksh, char **fname)
 	    struct stat st;
 	    if (!fstat(fd, &st) && S_ISREG(st.st_mode) &&
 		(len = lseek(fd, 0, 2)) != -1) {
+		if (test_only) {
+		    close(fd);
+		    *fname = *pp;
+		    return &dummy_eprog;
+		}
 		d = (char *) zalloc(len + 1);
 		lseek(fd, 0, 0);
 		if ((rlen = read(fd, d, len)) >= 0) {
@@ -5661,7 +5683,7 @@ getfpfunc(char *s, int *ksh, char **fname)
 		close(fd);
 	}
     }
-    return &dummy_eprog;
+    return test_only ? NULL : &dummy_eprog;
 }
 
 /* Handle the most common type of ksh-style autoloading, when doing a      *
diff --git a/Src/hashtable.c b/Src/hashtable.c
index 7c33675..2a8b585 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -949,16 +949,20 @@ printshfuncnode(HashNode hn, int printflags)
 	    zoutputtab(stdout);
 	}
 	if (!t) {
-	    char *fopt = "UtTkz";
+	    char *fopt = "UtTkzc";
 	    int flgs[] = {
 		PM_UNALIASED, PM_TAGGED, PM_TAGGED_LOCAL,
-		PM_KSHSTORED, PM_ZSHSTORED, 0
+		PM_KSHSTORED, PM_ZSHSTORED, PM_CUR_FPATH, 0
 	    };
 	    int fl;;
 
 	    zputs("builtin autoload -X", stdout);
 	    for (fl=0;fopt[fl];fl++)
 		if (f->node.flags & flgs[fl]) putchar(fopt[fl]);
+	    if (f->filename) {
+		putchar(' ');
+		zputs(f->filename, stdout);
+	    }
 	} else {
 	    zputs(t, stdout);
 	    zsfree(t);
diff --git a/Src/parse.c b/Src/parse.c
index ed6c4a8..314cc09 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -3338,7 +3338,7 @@ cur_add_func(char *nam, Shfunc shf, LinkList names, LinkList progs,
 	    return 1;
 	}
 	noaliases = (shf->node.flags & PM_UNALIASED);
-	if (!(prog = getfpfunc(shf->node.nam, NULL, NULL)) ||
+	if (!(prog = getfpfunc(shf->node.nam, NULL, NULL, NULL, 0)) ||
 	    prog == &dummy_eprog) {
 	    noaliases = ona;
 	    zwarnnam(nam, "can't load function: %s", shf->node.nam);
@@ -3580,7 +3580,7 @@ load_dump_file(char *dump, struct stat *sbuf, int other, int len)
 
 /**/
 Eprog
-try_dump_file(char *path, char *name, char *file, int *ksh)
+try_dump_file(char *path, char *name, char *file, int *ksh, int test_only)
 {
     Eprog prog;
     struct stat std, stc, stn;
@@ -3589,7 +3589,7 @@ try_dump_file(char *path, char *name, char *file, int *ksh)
 
     if (strsfx(FD_EXT, path)) {
 	queue_signals();
-	prog = check_dump_file(path, NULL, name, ksh);
+	prog = check_dump_file(path, NULL, name, ksh, test_only);
 	unqueue_signals();
 	return prog;
     }
@@ -3608,14 +3608,14 @@ try_dump_file(char *path, char *name, char *file, int *ksh)
     if (!rd &&
 	(rc || std.st_mtime > stc.st_mtime) &&
 	(rn || std.st_mtime > stn.st_mtime) &&
-	(prog = check_dump_file(dig, &std, name, ksh))) {
+	(prog = check_dump_file(dig, &std, name, ksh, test_only))) {
 	unqueue_signals();
 	return prog;
     }
     /* No digest file. Now look for the per-function compiled file. */
     if (!rc &&
 	(rn || stc.st_mtime > stn.st_mtime) &&
-	(prog = check_dump_file(wc, &stc, name, ksh))) {
+	(prog = check_dump_file(wc, &stc, name, ksh, test_only))) {
 	unqueue_signals();
 	return prog;
     }
@@ -3643,7 +3643,7 @@ try_source_file(char *file)
 
     if (strsfx(FD_EXT, file)) {
 	queue_signals();
-	prog = check_dump_file(file, NULL, tail, NULL);
+	prog = check_dump_file(file, NULL, tail, NULL, 0);
 	unqueue_signals();
 	return prog;
     }
@@ -3654,7 +3654,7 @@ try_source_file(char *file)
 
     queue_signals();
     if (!rc && (rn || stc.st_mtime > stn.st_mtime) &&
-	(prog = check_dump_file(wc, &stc, tail, NULL))) {
+	(prog = check_dump_file(wc, &stc, tail, NULL, 0))) {
 	unqueue_signals();
 	return prog;
     }
@@ -3667,7 +3667,8 @@ try_source_file(char *file)
 
 /**/
 static Eprog
-check_dump_file(char *file, struct stat *sbuf, char *name, int *ksh)
+check_dump_file(char *file, struct stat *sbuf, char *name, int *ksh,
+		int test_only)
 {
     int isrec = 0;
     Wordcode d;
@@ -3709,6 +3710,11 @@ check_dump_file(char *file, struct stat *sbuf, char *name, int *ksh)
     if ((h = dump_find_func(d, name))) {
 	/* Found the name. If the file is already mapped, return the eprog,
 	 * otherwise map it and just go up. */
+	if (test_only)
+	{
+	    /* This is all we need.  Just return dummy. */
+	    return &dummy_eprog;
+	}
 
 #ifdef USE_MMAP
 
@@ -3745,7 +3751,7 @@ check_dump_file(char *file, struct stat *sbuf, char *name, int *ksh)
 
 #endif
 
-	    {
+	{
 	    Eprog prog;
 	    Patprog *pp;
 	    int np, fd, po = h->npats * sizeof(Patprog);
diff --git a/Src/zsh.h b/Src/zsh.h
index 2a41638..67c5a35 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1233,7 +1233,9 @@ struct cmdnam {
 
 struct shfunc {
     struct hashnode node;
-    char *filename;             /* Name of file located in */
+    char *filename;             /* Name of file located in.
+				   For not yet autoloaded file, name
+				   of explicit directory, if not NULL. */
     zlong lineno;		/* line number in above file */
     Eprog funcdef;		/* function definition    */
     Eprog redir;                /* redirections to apply */
@@ -1811,6 +1813,7 @@ struct tieddata {
 #define PM_UNALIASED	(1<<13)	/* do not expand aliases when autoloading   */
 
 #define PM_HIDE		(1<<14)	/* Special behaviour hidden by local        */
+#define PM_CUR_FPATH    (1<<14) /* (function): can use $fpath with filename */
 #define PM_HIDEVAL	(1<<15)	/* Value not shown in `typeset' commands    */
 #define PM_TIED 	(1<<16)	/* array tied to colon-path or v.v.         */
 #define PM_TAGGED_LOCAL (1<<16) /* (function): non-recursive PM_TAGGED      */
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index ab7b429..1821b78 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -330,6 +330,95 @@
 0:whence -v of zsh-style autoload
 >oops is a shell function from ./oops
 
+  (
+    fpath=(.)
+    mkdir extra
+    print 'print "I have been loaded by explicit path."' >extra/spec
+    autoload -Uz $PWD/extra/spec
+    spec
+  )
+0:autoload with explicit path
+>I have been loaded by explicit path.
+
+  (
+    fpath=(.)
+    print 'print "I have been loaded by default path."' >def
+    autoload -Uz $PWD/extra/def
+    def
+  )
+1:autoload with explicit path with function in normal path, no -d
+?(eval):5: def: function definition file not found
+
+  (
+    fpath=(.)
+    autoload -dUz $PWD/extra/def
+    def
+  )
+0:autoload with explicit path with function in normal path, with -d
+>I have been loaded by default path.
+
+  (
+    cd extra
+    fpath=(.)
+    autoload -r spec
+    cd ..
+    spec
+  )
+0:autoload -r
+>I have been loaded by explicit path.
+
+  (
+    cd extra
+    fpath=(.)
+    autoload -r def
+    cd ..
+    def
+  )
+0:autoload -r is permissive
+>I have been loaded by default path.
+
+  (
+    cd extra
+    fpath=(.)
+    autoload -R def
+  )
+1:autoload -R is not permissive
+?(eval):4: def: function definition file not found
+
+  (
+    spec() { autoload -XUz $PWD/extra; }
+    spec
+  )
+0:autoload -X with path
+>I have been loaded by explicit path.
+
+# The line number 1 here and in the next test seems suspect,
+# but this example proves it's not down to the new features
+# being tested here.
+  (
+    fpath=(.)
+    cod() { autoload -XUz; }
+    cod
+  )
+1:autoload -X with no path, failure
+?(eval):1: cod: function definition file not found
+
+  (
+    fpath=(.)
+    def() { autoload -XUz $PWD/extra; }
+    def
+  )
+1:autoload -X with wrong path and no -d
+?(eval):1: def: function definition file not found
+
+  (
+    fpath=(.)
+    def() { autoload -dXUz $PWD/extra; }
+    def
+  )
+0:autoload -dX with path
+>I have been loaded by default path.
+
 %clean
 
  rm -f file.in file.out


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2016-12-12 16:31   ` Peter Stephenson
@ 2016-12-12 18:09     ` Bart Schaefer
  2017-01-10 19:31     ` Peter Stephenson
  1 sibling, 0 replies; 35+ messages in thread
From: Bart Schaefer @ 2016-12-12 18:09 UTC (permalink / raw)
  To: Zsh hackers list

On Dec 12,  4:31pm, Peter Stephenson wrote:
}
} We're at the point where autoload isn't really a synonym of functions -u
} any more.  I'd be tempted to document "functions -u" as "implements a
} limited subset of..." rather than "equivalent to...".  (Does anyone use
} that interface except for compatibility?)

There's
  autoload
  functions -u
  typeset -f -u

but there are aready some variations of each that are not supported by
the others, so which ones say they are "equivalent to" which others is
probably overdue for an overhaul.

For example I don't think there's any way to autoload a math function in
a single command -- you have to autoload it and then mark it as math, or
something like that.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2016-12-12 16:05 ` Bart Schaefer
@ 2016-12-12 16:31   ` Peter Stephenson
  2016-12-12 18:09     ` Bart Schaefer
  2017-01-10 19:31     ` Peter Stephenson
  0 siblings, 2 replies; 35+ messages in thread
From: Peter Stephenson @ 2016-12-12 16:31 UTC (permalink / raw)
  To: Zsh hackers list

On Mon, 12 Dec 2016 08:05:50 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> If I understand what you're saying, that means
> 
>     autoload this/here/file
> 
> creates a function named "this/here/file" by loading "file" from
> $^fpath/this/here  ... yes?
> 
> And then (before this patch)
> 
>     autoload /this/here/file
> 
> creates a function named "/this/here/file" from the same location?
> 
> So after this patch there is no way to autoload a function whose name
> begins with a slash.  Not that it was especially useful to be able to
> do so, just confirming.

Yes, that all looks like it's correct.

> If I may make a suggestion -- an equally useful variation would be for
> autoload to search the $fpath at the time of the autoload command and
> store the path where the file is found *iff* it is found.  This would
> be sort of like doing
> 
>     autoload -Uz $^fpath/myfunc(N)
> 
> except that it handles the situation where $fpath does not yet contain
> the "myfunc" file.
>
> Another useful variation might be to fall back to the current fpath if
> the function isn't found at the specified location.

These need some form of new syntax, but it looks like options are
probably good enough.  -r for remember the path where found (or -s for
save/store?) and -c for use current fpath as default (or -d for default?
anything with -f or -p being a bit vague in this context).

-R for remember and also ensure it's there with a hard error if it isn't
there would be possible if it seems useful --- it gives you a way you
don't currently have of expressing the fact the function is required and
you might as well bomb out immediately rather than once you're committed
to doing whatever.

Presumably -c should remember the path, even if it found it on $fpath
rather than the specified location, to avoid surprises.

We're at the point where autoload isn't really a synonym of functions -u
any more.  I'd be tempted to document "functions -u" as "implements a
limited subset of..." rather than "equivalent to...".  (Does anyone use
that interface except for compatibility?)

pws


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: PATH: autoload with explicit path
  2016-12-11 22:18 Peter Stephenson
@ 2016-12-12 16:05 ` Bart Schaefer
  2016-12-12 16:31   ` Peter Stephenson
  0 siblings, 1 reply; 35+ messages in thread
From: Bart Schaefer @ 2016-12-12 16:05 UTC (permalink / raw)
  To: Zsh hackers list

On Dec 11, 10:18pm, Peter Stephenson wrote:
}
} I wanted to autoload a function from an explicit path.
} [...] it strikes me it's a bit mean of the shell not simply to do
} something sensible with
} 
} autoload +X /path/to/myfunc

For +X only, this can be accomplished by

    FPATH=/path/to autoload +X myfunc

Sebastian's plugin manager uses this.

} That doesn't do anything sensible at the moment --- relative paths to
} functions do, with the full relative path as the name, which is entirely
} consistent, but absolute paths don't and in fact it tries the name as a
} relative path anyway which seems Just Plain Wrong.

If I understand what you're saying, that means

    autoload this/here/file

creates a function named "this/here/file" by loading "file" from
$^fpath/this/here  ... yes?

And then (before this patch)

    autoload /this/here/file

creates a function named "/this/here/file" from the same location?

So after this patch there is no way to autoload a function whose name
begins with a slash.  Not that it was especially useful to be able to
do so, just confirming.

} autoload -Uz /path/to/myfunc
} 
} defines myfunc to be found in the directory /path/to by reusing the
} filename element of the shfunc structure that's currently unused at this
} stage.
} 
} This seemed quite a useful feature.

Yes, this is specifically what Sebastian and (I think) Ray were asking
for several months ago.

If I may make a suggestion -- an equally useful variation would be for
autoload to search the $fpath at the time of the autoload command and
store the path where the file is found *iff* it is found.  This would
be sort of like doing

    autoload -Uz $^fpath/myfunc(N)

except that it handles the situation where $fpath does not yet contain
the "myfunc" file.

Another useful variation might be to fall back to the current fpath if
the function isn't found at the specified location.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* PATH: autoload with explicit path
@ 2016-12-11 22:18 Peter Stephenson
  2016-12-12 16:05 ` Bart Schaefer
  0 siblings, 1 reply; 35+ messages in thread
From: Peter Stephenson @ 2016-12-11 22:18 UTC (permalink / raw)
  To: Zsh hackers list

Not to be committed until 5.3 is safely done and dusted, but you might
as well see it now.

I wanted to autoload a function from an explicit path.  (Not so auto, in
fact, but there isn't a trivial way of manually loading an autoload -z
format file without using autoload.)  I could rig something up in a
function, saving and restoring fpath or wrapping the contents of the
file, but it strikes me it's a bit mean of the shell not simply to do
something sensible with

autoload +X /path/to/myfunc

That doesn't do anything sensible at the moment --- relative paths to
functions do, with the full relative path as the name, which is entirely
consistent, but absolute paths don't and in fact it tries the name as a
relative path anyway which seems Just Plain Wrong.

Then I found it was pretty much as easy (easier, actually, since fewer
special cases) to implement this generally:

autoload -Uz /path/to/myfunc

defines myfunc to be found in the directory /path/to by reusing the
filename element of the shfunc structure that's currently unused at this
stage.

This seemed quite a useful feature.  No doc or tests at this stage.

Also TBD: output from "autoload" with no name.  Just needs to update the
output of the autoload -X line appropriately, so this is a minor detail,
I think.

Also TBD: I should clearly document that autoload has its normal
semantics i.e. if the function already exists that definition is kept.
Explicit path != forced reload.

Also TBD: should output a special error message if not found so
user knows where it was looking.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index 65e0cb1..96571df 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2962,6 +2962,27 @@ listusermathfunc(MathFunc p)
 }
 
 
+static void
+add_autoload_function(Shfunc shf, char *funcname)
+{
+    char *nam;
+    if (*funcname == '/' && funcname[1] &&
+	(nam = strrchr(funcname, '/')) && nam[1]) {
+	char *dir;
+	nam = strrchr(funcname, '/');
+	if (nam == funcname) {
+	    dir = "/";
+	} else {
+	    *nam++ = '\0';
+	    dir = funcname;
+	}
+	shf->filename = ztrdup(dir);
+	shfunctab->addnode(shfunctab, ztrdup(nam), shf);
+    } else {
+	shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
+    }
+}
+
 /* Display or change the attributes of shell functions.   *
  * If called as autoload, it will define a new autoloaded *
  * (undefined) shell function.                            */
@@ -3195,7 +3216,7 @@ bin_functions(char *name, char **argv, Options ops, int func)
 			  "BUG: Calling autoload from empty function");
 		} else {
 		    shf = (Shfunc) zshcalloc(sizeof *shf);
-		    shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
+		    add_autoload_function(shf, funcname);
 		}
 		shf->node.flags = on;
 		ret = eval_autoload(shf, funcname, ops, func);
@@ -3266,6 +3287,7 @@ bin_functions(char *name, char **argv, Options ops, int func)
 		printshfuncexpand(&shf->node, pflags, expand);
 	} else if (on & PM_UNDEFINED) {
 	    int signum = -1, ok = 1;
+	    char *nam;
 
 	    if (!strncmp(*argv, "TRAP", 4) &&
 		(signum = getsignum(*argv + 4)) != -1) {
@@ -3282,7 +3304,7 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	    shf->node.flags = on;
 	    shf->funcdef = mkautofn(shf);
 	    shfunc_set_sticky(shf);
-	    shfunctab->addnode(shfunctab, ztrdup(*argv), shf);
+	    add_autoload_function(shf, *argv);
 
 	    if (signum != -1) {
 		if (settrap(signum, NULL, ZSIG_FUNC)) {
diff --git a/Src/exec.c b/Src/exec.c
index a439aec..6e4e99c 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5155,12 +5155,23 @@ loadautofn(Shfunc shf, int fksh, int autol)
 {
     int noalias = noaliases, ksh = 1;
     Eprog prog;
-    char *fname;
+    char *fname, **alt_path, *spec_path[2];
 
     pushheap();
 
+    if (shf->filename && shf->filename[0] == '/')
+    {
+	spec_path[0] = dupstring(shf->filename);
+	spec_path[1] = NULL;
+	alt_path = spec_path;
+    }
+    else
+    {
+	alt_path = NULL;
+    }
+
     noaliases = (shf->node.flags & PM_UNALIASED);
-    prog = getfpfunc(shf->node.nam, &ksh, &fname);
+    prog = getfpfunc(shf->node.nam, &ksh, &fname, alt_path);
     noaliases = noalias;
 
     if (ksh == 1) {
@@ -5607,7 +5618,7 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name)
 
 /**/
 Eprog
-getfpfunc(char *s, int *ksh, char **fname)
+getfpfunc(char *s, int *ksh, char **fname, char **alt_path)
 {
     char **pp, buf[PATH_MAX+1];
     off_t len;
@@ -5616,7 +5627,7 @@ getfpfunc(char *s, int *ksh, char **fname)
     Eprog r;
     int fd;
 
-    pp = fpath;
+    pp = alt_path ? alt_path : fpath;
     for (; *pp; pp++) {
 	if (strlen(*pp) + strlen(s) + 1 >= PATH_MAX)
 	    continue;
diff --git a/Src/parse.c b/Src/parse.c
index 50a0d5f..38c6da2 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -3324,7 +3324,7 @@ cur_add_func(char *nam, Shfunc shf, LinkList names, LinkList progs,
 	    return 1;
 	}
 	noaliases = (shf->node.flags & PM_UNALIASED);
-	if (!(prog = getfpfunc(shf->node.nam, NULL, NULL)) ||
+	if (!(prog = getfpfunc(shf->node.nam, NULL, NULL, NULL)) ||
 	    prog == &dummy_eprog) {
 	    noaliases = ona;
 	    zwarnnam(nam, "can't load function: %s", shf->node.nam);
diff --git a/Src/zsh.h b/Src/zsh.h
index f22d8b1..4fdd09a 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1233,7 +1233,9 @@ struct cmdnam {
 
 struct shfunc {
     struct hashnode node;
-    char *filename;             /* Name of file located in */
+    char *filename;             /* Name of file located in.
+				   For not yet autoloaded file, name
+				   of explicit directory, if not NULL. */
     zlong lineno;		/* line number in above file */
     Eprog funcdef;		/* function definition    */
     Eprog redir;                /* redirections to apply */


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2017-01-30 12:05 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20170127151334epcas2p4c32b57f69fcae22b40b309793eb8ceb6@epcas2p4.samsung.com>
2017-01-27 15:12 ` PATH: autoload with explicit path Sebastian Gniazdowski
2017-01-27 16:24   ` Peter Stephenson
2017-01-27 18:40     ` Sebastian Gniazdowski
2017-01-27 18:44       ` Peter Stephenson
2017-01-27 19:00         ` Sebastian Gniazdowski
2017-01-28 18:05         ` Bart Schaefer
2017-01-28 19:12           ` Peter Stephenson
2017-01-28 19:45             ` Bart Schaefer
2017-01-28 19:56               ` Peter Stephenson
2017-01-28 20:37                 ` Sebastian Gniazdowski
2017-01-29 12:27             ` Sebastian Gniazdowski
2017-01-29 16:11               ` Bart Schaefer
2017-01-29 17:32                 ` Sebastian Gniazdowski
2017-01-29 18:37                   ` Bart Schaefer
2017-01-29 21:53                     ` Vin Shelton
2017-01-30 10:06                       ` Peter Stephenson
2017-01-29 17:58                 ` Peter Stephenson
2017-01-30 11:37                   ` Sebastian Gniazdowski
2017-01-30 11:55                     ` Peter Stephenson
2016-12-11 22:18 Peter Stephenson
2016-12-12 16:05 ` Bart Schaefer
2016-12-12 16:31   ` Peter Stephenson
2016-12-12 18:09     ` Bart Schaefer
2017-01-10 19:31     ` Peter Stephenson
2017-01-11 11:42       ` Peter Stephenson
2017-01-11 20:51         ` Peter Stephenson
2017-01-12 20:42           ` Peter Stephenson
2017-01-13 18:04           ` Peter Stephenson
2017-01-16 10:37             ` Peter Stephenson
2017-01-16 15:04               ` Daniel Shahaf
2017-01-16 15:48                 ` Peter Stephenson
2017-01-16 15:22               ` Bart Schaefer
2017-01-16 15:59                 ` Peter Stephenson
     [not found]                   ` <CAHYJk3SB1NDj6y5TRHHsAVsyjHfZQhTzMRzTR2c-SVEc9oAwzA@mail.gmail.com>
2017-01-24 11:10                     ` Peter Stephenson
2017-01-11 21:13         ` Peter Stephenson

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).