* Re: PATCH: autoload with explicit path [not found] <CGME20170112125605eucas1p1b2539afbacec2d28d44c6fd73b0d50af@eucas1p1.samsung.com> @ 2017-01-12 12:56 ` Peter Stephenson 2017-01-12 15:40 ` Daniel Shahaf 2017-01-12 16:05 ` Vin Shelton 0 siblings, 2 replies; 16+ messages in thread From: Peter Stephenson @ 2017-01-12 12:56 UTC (permalink / raw) To: Zsh Hackers' List (It's a bit late now, but that "PATH" in the subject line was a typo because the Subject line in my email client at home is in very small print.) Vin is reporting failures where the symptom in both cases appears to be not finding an autoload function when using "autoload -X" within the function (with fpath=(.), but that aspect is probably irrelevant). I can't reproduce this using the normal tests, but I have found a possible cause and added a test that does fail in the same way. This patch fixes that failure. The problem is the overloading of filename in struct shfunc. If the function was alrady loaded, which is the case if it contains an explicit autoload -X (rather than one generated internally by looking at the flags for an undefined function), then the filename indicates the location of the source for the function. If this came from a file with an absolute path, and there was no explicit directory path in the autoload -X statement, the file path was erroneously taken as a directory for loading. This adds an explicit flag to indicate filename is being used for that purpose, unsetting it when the filename is set to the file's path. Also be a bit more careful checking if a function wasn't let loaded when using the new functions options. If it's already loaded they're irrelevant. pws P.S. test failure reports to the list, please, so everyone can see --- thanks. > From: Vin Shelton <acs@alumni.princeton.edu> > To: Peter Stephenson <p.w.stephenson@ntlworld.com> > Cc: Zsh hackers list <zsh-workers@zsh.org> > Date: 11 January 2017 at 23:15 > Subject: Re: PATH: autoload with explicit path > > Hi, Peter - > > Thanks for this. I see two test failures: > > opt/src/zsh-2017-01-11/Test/A04redirect.ztst: all tests successful. > /opt/src/zsh-2017-01-11/Test/A05execution.ztst: starting. > Test /opt/src/zsh-2017-01-11/Test/A05execution.ztst failed: bad status 1, > expected 0 from: > unfunction functst > print "print Yet another version" >functst > functst() { autoload -X; } > functst > Error output: > (eval):1: functst: function definition file not found > Was testing: autoloading via -X > /opt/src/zsh-2017-01-11/Test/A05execution.ztst: test failed. > /opt/src/zsh-2017-01-11/Test/A06assign.ztst: starting. > > and > > /opt/src/zsh-2017-01-11/Test/C03traps.ztst: all tests successful. > /opt/src/zsh-2017-01-11/Test/C04funcdef.ztst: starting. > --- /tmp/zsh.ztst.8259/ztst.out 2017-01-11 17:56:26.309239694 -0500 > +++ /tmp/zsh.ztst.8259/ztst.tout 2017-01-11 17:56:26.309239694 -0500 > @@ -1,4 +1,4 @@ > -oops was successfully autoloaded > oops () { > > * print oops was successfully autoloaded > > * # undefined > * builtin autoload -X /opt/src/zsh-2017-01-11/Test/ztst.zsh > } > Test /opt/src/zsh-2017-01-11/Test/C04funcdef.ztst failed: output differs > from expected as shown above for: > ( > fpath=(.) > print "print oops was successfully autoloaded" >oops > oops() { eval autoload -X } > oops > which -x2 oops > ) > Error output: > (eval):1: oops: function definition file not found > Was testing: autoload containing eval > /opt/src/zsh-2017-01-11/Test/C04funcdef.ztst: test failed. > /opt/src/zsh-2017-01-11/Test/C05debug.ztst: starting. > /opt/src/zsh-2017-01-11/Test/C05debug.ztst: all tests successful. > > I see this on a variety of linux systems. Please let me know if you need > more details. diff --git a/Src/builtin.c b/Src/builtin.c index b986dd8..716ddd4 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -2936,10 +2936,11 @@ check_autoload(Shfunc shf, char *name, Options ops, int func) { return eval_autoload(shf, name, ops, func); } - if (OPT_ISSET(ops,'r') || OPT_ISSET(ops,'R')) + if ((OPT_ISSET(ops,'r') || OPT_ISSET(ops,'R')) && + (shf->node.flags & PM_UNDEFINED)) { char *dir_path; - if (shf->filename) { + if (shf->filename && (shf->node.flags & PM_LOADDIR)) { char *spec_path[2]; spec_path[0] = shf->filename; spec_path[1] = NULL; @@ -2964,6 +2965,7 @@ check_autoload(Shfunc shf, char *name, Options ops, int func) dir_path = xsymlink(dir_path, 1); } shf->filename = ztrdup(dir_path); + shf->node.flags |= PM_LOADDIR; return 0; } if (OPT_ISSET(ops,'R')) { @@ -3017,7 +3019,8 @@ add_autoload_function(Shfunc shf, char *funcname) { char *nam; if (*funcname == '/' && funcname[1] && - (nam = strrchr(funcname, '/')) && nam[1]) { + (nam = strrchr(funcname, '/')) && nam[1] && + (shf->node.flags & PM_UNDEFINED)) { char *dir; nam = strrchr(funcname, '/'); if (nam == funcname) { @@ -3028,6 +3031,7 @@ add_autoload_function(Shfunc shf, char *funcname) } zsfree(shf->filename); shf->filename = ztrdup(dir); + shf->node.flags |= PM_LOADDIR; shfunctab->addnode(shfunctab, ztrdup(nam), shf); } else { shfunctab->addnode(shfunctab, ztrdup(funcname), shf); @@ -3278,6 +3282,7 @@ bin_functions(char *name, char **argv, Options ops, int func) if (*argv) { zsfree(shf->filename); shf->filename = ztrdup(*argv); + on |= PM_LOADDIR; } shf->node.flags = on; ret = eval_autoload(shf, funcname, ops, func); diff --git a/Src/exec.c b/Src/exec.c index 7bec7ce..68c455b 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5160,7 +5160,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) pushheap(); noaliases = (shf->node.flags & PM_UNALIASED); - if (shf->filename && shf->filename[0] == '/') + if (shf->filename && shf->filename[0] == '/' && + (shf->node.flags & PM_LOADDIR)) { char *spec_path[2]; spec_path[0] = dupstring(shf->filename); @@ -5203,7 +5204,7 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) shf->funcdef = prog; else shf->funcdef = dupeprog(prog, 0); - shf->node.flags &= ~PM_UNDEFINED; + shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR); zsfree(shf->filename); shf->filename = fname; } else { @@ -5227,7 +5228,7 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath) shf->funcdef = stripkshdef(prog, shf->node.nam); else shf->funcdef = dupeprog(stripkshdef(prog, shf->node.nam), 0); - shf->node.flags &= ~PM_UNDEFINED; + shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR); zsfree(shf->filename); shf->filename = fname; } diff --git a/Src/zsh.h b/Src/zsh.h index 67c5a35..7d18333 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -1823,6 +1823,7 @@ struct tieddata { /* Remaining flags do not correspond directly to command line arguments */ #define PM_DONTIMPORT_SUID (1<<19) /* do not import if running setuid */ +#define PM_LOADDIR (1<<19) /* (function) filename gives load directory */ #define PM_SINGLE (1<<20) /* special can only have a single instance */ #define PM_LOCAL (1<<21) /* this parameter will be made local */ #define PM_SPECIAL (1<<22) /* special builtin parameter */ diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst index 1821b78..7100280 100644 --- a/Test/C04funcdef.ztst +++ b/Test/C04funcdef.ztst @@ -419,6 +419,16 @@ 0:autoload -dX with path >I have been loaded by default path. + ( + fpath=(.) + print 'loadthisfunc() { autoload -X }' >loadthisfunc_sourceme + print 'print Function was loaded correctly.' >loadthisfunc + source $PWD/loadthisfunc_sourceme + loadthisfunc + ) +0: autoload -X interaction with absolute filename used for source location +>Function was loaded correctly. + %clean rm -f file.in file.out ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-12 12:56 ` PATCH: autoload with explicit path Peter Stephenson @ 2017-01-12 15:40 ` Daniel Shahaf 2017-01-12 15:59 ` Peter Stephenson 2017-01-12 16:05 ` Vin Shelton 1 sibling, 1 reply; 16+ messages in thread From: Daniel Shahaf @ 2017-01-12 15:40 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh Hackers' List Peter Stephenson wrote on Thu, Jan 12, 2017 at 12:56:02 +0000: > The problem is the overloading of filename in struct shfunc. If the > function was alrady loaded, which is the case if it contains an > explicit autoload -X (rather than one generated internally by looking > at the flags for an undefined function), then the filename indicates the > location of the source for the function. If this came from a file with > an absolute path, and there was no explicit directory path in the > autoload -X statement, the file path was erroneously taken as a > directory for loading. > > This adds an explicit flag to indicate filename is being used for that > purpose, unsetting it when the filename is set to the file's path. I would suggest using two separate struct members, rather than one whose semantics depend on a flag. It'd be too easy to forget to check the flag in some (existing or future) location. E.g., struct shfunc { char *filename1; /* Name of file located in */ char *filename2; /* Name of file to autoload from */ }; We could also put these two members in a union{} if they are mutually exclusive (if at least one of them is NULL at all times). ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-12 15:40 ` Daniel Shahaf @ 2017-01-12 15:59 ` Peter Stephenson 2017-01-12 16:09 ` Daniel Shahaf 0 siblings, 1 reply; 16+ messages in thread From: Peter Stephenson @ 2017-01-12 15:59 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 12 Jan 2017 15:40:57 +0000 Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > We could also put these two members in a union{} if they are mutually > exclusive (if at least one of them is NULL at all times). They are mutually exclusive, as one is only useful for an autoload and one is only useful with real source, which is why it uses the same pointer. Making it a union doesn't actually change anything: it's still the same set of of reads and writes. pws ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-12 15:59 ` Peter Stephenson @ 2017-01-12 16:09 ` Daniel Shahaf 2017-01-12 16:16 ` Peter Stephenson 0 siblings, 1 reply; 16+ messages in thread From: Daniel Shahaf @ 2017-01-12 16:09 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh Hackers' List Peter Stephenson wrote on Thu, Jan 12, 2017 at 15:59:20 +0000: > On Thu, 12 Jan 2017 15:40:57 +0000 > Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > We could also put these two members in a union{} if they are mutually > > exclusive (if at least one of them is NULL at all times). > > They are mutually exclusive, as one is only useful for an autoload and > one is only useful with real source, which is why it uses the same > pointer. Making it a union doesn't actually change anything: it's still > the same set of of reads and writes. Of course it would be exactly the same machine code, but the source code would be more robust against bugs. It's a lot harder to refer to the wrong union member than to use the ->filename member without testing PM_LOADDIR first. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-12 16:09 ` Daniel Shahaf @ 2017-01-12 16:16 ` Peter Stephenson 2017-01-12 16:23 ` Daniel Shahaf 0 siblings, 1 reply; 16+ messages in thread From: Peter Stephenson @ 2017-01-12 16:16 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 12 Jan 2017 16:09:21 +0000 Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Of course it would be exactly the same machine code, but the source code > would be more robust against bugs. It's a lot harder to refer to the > wrong union member than to use the ->filename member without testing > PM_LOADDIR first. Hmm, feel free to write this if you think you can make things clearer; the bit setting will tell you what needs doing in each case. But I don't really see how it helps. If the bit happens to be set you will do it one way having exactly the same effect as if you did it the other way, so I don't see the gain. pws ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-12 16:16 ` Peter Stephenson @ 2017-01-12 16:23 ` Daniel Shahaf 2017-01-12 16:34 ` Peter Stephenson 0 siblings, 1 reply; 16+ messages in thread From: Daniel Shahaf @ 2017-01-12 16:23 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh Hackers' List Peter Stephenson wrote on Thu, Jan 12, 2017 at 16:16:51 +0000: > On Thu, 12 Jan 2017 16:09:21 +0000 > Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Of course it would be exactly the same machine code, but the source code > > would be more robust against bugs. It's a lot harder to refer to the > > wrong union member than to use the ->filename member without testing > > PM_LOADDIR first. > > Hmm, feel free to write this if you think you can make things clearer; > the bit setting will tell you what needs doing in each case. But I > don't really see how it helps. If the bit happens to be set you will do > it one way having exactly the same effect as if you did it the other > way, so I don't see the gain. The point is that the next time writes: . foo(shf->filename); . that won't compile, so he will be forced to take into account the two distinct overloaded meanings. This might have prevented the bug you fixed in 40335 from being written. I'll see if adding it makes things clearer, will post if it does. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-12 16:23 ` Daniel Shahaf @ 2017-01-12 16:34 ` Peter Stephenson 0 siblings, 0 replies; 16+ messages in thread From: Peter Stephenson @ 2017-01-12 16:34 UTC (permalink / raw) To: Zsh Hackers' List On Thu, 12 Jan 2017 16:23:38 +0000 Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > The point is that the next time writes: > . > foo(shf->filename); > . > that won't compile, so he will be forced to take into account the two > distinct overloaded meanings. OK, I can see that might be useful. > This might have prevented the bug you fixed in 40335 from being > written. No, it would still have been possible to resolve all uses into one form or the other. The problem was it was done implicitly by location in the code and needed discriminating by something else such as the new bit (or the implied NULL / non-NULL test of a separate field). > I'll see if adding it makes things clearer, will post if it does. Fine, you might want to see what happens with the change for caching the autoload directory first as it hits the same lines. pws ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-12 12:56 ` PATCH: autoload with explicit path Peter Stephenson 2017-01-12 15:40 ` Daniel Shahaf @ 2017-01-12 16:05 ` Vin Shelton 1 sibling, 0 replies; 16+ messages in thread From: Vin Shelton @ 2017-01-12 16:05 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh Hackers' List [-- Attachment #1: Type: text/plain, Size: 1164 bytes --] On Thu, Jan 12, 2017 at 7:56 AM, Peter Stephenson <p.stephenson@samsung.com> wrote: > I can't reproduce this using the normal tests, but I have found a > possible cause and added a test that does fail in the same way. This > patch fixes that failure. > > The problem is the overloading of filename in struct shfunc. If the > function was alrady loaded, which is the case if it contains an > explicit autoload -X (rather than one generated internally by looking > at the flags for an undefined function), then the filename indicates the > location of the source for the function. If this came from a file with > an absolute path, and there was no explicit directory path in the > autoload -X statement, the file path was erroneously taken as a > directory for loading. > > This adds an explicit flag to indicate filename is being used for that > purpose, unsetting it when the filename is set to the file's path. > > Also be a bit more careful checking if a function wasn't let loaded when > using the new functions options. If it's already loaded they're > irrelevant. > That has fixed the failures for me: I'm now seeing 48 successful tests. Thanks, Vin ^ permalink raw reply [flat|nested] 16+ messages in thread
* PATH: autoload with explicit path @ 2016-12-11 22:18 Peter Stephenson [not found] ` <CGME20161212160617epcas2p16960e3d95c694147035f760090e6011b@epcas2p1.samsung.com> 0 siblings, 1 reply; 16+ 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] 16+ messages in thread
[parent not found: <CGME20161212160617epcas2p16960e3d95c694147035f760090e6011b@epcas2p1.samsung.com>]
* Re: PATH: autoload with explicit path 2016-12-11 22:18 PATH: " Peter Stephenson @ 2016-12-12 16:05 ` Bart Schaefer 2016-12-12 16:31 ` Peter Stephenson 0 siblings, 1 reply; 16+ 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] 16+ messages in thread
* Re: PATH: autoload with explicit path 2016-12-12 16:05 ` Bart Schaefer @ 2016-12-12 16:31 ` Peter Stephenson 2017-01-10 19:31 ` Peter Stephenson 0 siblings, 1 reply; 16+ 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] 16+ messages in thread
* Re: PATH: autoload with explicit path 2016-12-12 16:31 ` Peter Stephenson @ 2017-01-10 19:31 ` Peter Stephenson 2017-01-17 18:36 ` PATCH: " Peter Stephenson 0 siblings, 1 reply; 16+ 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] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-10 19:31 ` Peter Stephenson @ 2017-01-17 18:36 ` Peter Stephenson 2017-01-17 22:17 ` Daniel Shahaf 2017-01-18 9:53 ` Peter Stephenson 0 siblings, 2 replies; 16+ messages in thread From: Peter Stephenson @ 2017-01-17 18:36 UTC (permalink / raw) To: Zsh hackers list Currently, autoload /path/to/foo autoload foo leaves foo marked as to be loaded from /path/to/foo. I should probably document this (and that therefore to unmark the path you need to unfunction). I think this is the right way of doing it as the explicit path should continue to override the more vague autoload with no path indicated, and this is safer in case some code decides it needs a function and inadvertently resets the path the user carefully decided to give the function. However, you could argue that "autoload foo" should reset foo to reload from $fpath, as the shell would do if it encountered that command on its own. You can in any case change an explicitly marked path seamlessly by issuing a new command with an absolute path. I have found a bug, though: "autoload /path/to/foo" takes effect even if foo is already loaded, which isn't supposed to happen. Test for this tomorrow. Second hunk is a minor efficiency change: I noticed findcmd() with second argument 1 returns a dupstring() value anyway. pws diff --git a/Src/builtin.c b/Src/builtin.c index b1b6e2e..7a04a79 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -3369,6 +3369,31 @@ bin_functions(char *name, char **argv, Options ops, int func) removetrapnode(signum); } + if (**argv == '/') { + char *base = strrchr(*argv, '/') + 1; + if (*base && + (shf = (Shfunc) shfunctab->getnode(shfunctab, base))) { + char *dir; + /* turn on/off the given flags */ + shf->node.flags = + (shf->node.flags | (on & ~PM_UNDEFINED)) & ~off; + if (shf->node.flags & PM_UNDEFINED) { + /* update path if not yet loaded */ + if (base == *argv + 1) + dir = "/"; + else { + dir = *argv; + base[-1] = '\0'; + } + dircache_set(&shf->filename, NULL); + dircache_set(&shf->filename, dir); + } + if (check_autoload(shf, shf->node.nam, ops, func)) + returnval = 1; + continue; + } + } + /* Add a new undefined (autoloaded) function to the * * hash table with the corresponding flags set. */ shf = (Shfunc) zshcalloc(sizeof *shf); diff --git a/Src/subst.c b/Src/subst.c index 737a0a9..670f3f0 100644 --- a/Src/subst.c +++ b/Src/subst.c @@ -622,7 +622,7 @@ filesub(char **namptr, int assign) char * equalsubstr(char *str, int assign, int nomatch) { - char *pp, *cnam, *cmdstr, *ret; + char *pp, *cnam, *cmdstr; for (pp = str; !isend2(*pp); pp++) ; @@ -634,10 +634,10 @@ equalsubstr(char *str, int assign, int nomatch) zerr("%s not found", cmdstr); return NULL; } - ret = dupstring(cnam); if (*pp) - ret = dyncat(ret, pp); - return ret; + return dyncat(cnam, pp); + else + return cnam; /* already duplicated */ } /**/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-17 18:36 ` PATCH: " Peter Stephenson @ 2017-01-17 22:17 ` Daniel Shahaf 2017-01-18 0:06 ` Bart Schaefer 2017-01-18 9:17 ` Peter Stephenson 2017-01-18 9:53 ` Peter Stephenson 1 sibling, 2 replies; 16+ messages in thread From: Daniel Shahaf @ 2017-01-17 22:17 UTC (permalink / raw) To: zsh-workers Peter Stephenson wrote on Tue, Jan 17, 2017 at 18:36:06 +0000: > autoload /path/to/foo > autoload foo > > leaves foo marked as to be loaded from /path/to/foo. I should probably > document this (and that therefore to unmark the path you need to > unfunction). I think this is the right way of doing it as the explicit > path should continue to override the more vague autoload with no path > indicated, and this is safer in case some code decides it needs a > function and inadvertently resets the path the user carefully decided to > give the function. Suppose $^fpath/foo/bar(N) has two matches, how do I explicitly autoload the second one? I.e., how do I disambiguate «autoload foo/bar» (with no leading slash) to load a particular copy of foo/bar? Just removing the $FPATH element containing the former foo/bar would prevent foo/bar from autoloading functions that are only available in that $FPATH element. (Which might be the system default $fpath dir) I wonder if we need «autoload -d /qux foo/bar», or perhaps cause «autoload /qux/./foo/bar» load a function named "foo/bar" (the idea of assigning a meaning to /./ elements is borrowed from rsyncd.conf). (I'm assuming «autoload foo/bar» is a first-class use-case.) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-17 22:17 ` Daniel Shahaf @ 2017-01-18 0:06 ` Bart Schaefer 2017-01-18 9:21 ` Peter Stephenson 2017-01-18 9:17 ` Peter Stephenson 1 sibling, 1 reply; 16+ messages in thread From: Bart Schaefer @ 2017-01-18 0:06 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: TEXT/PLAIN, Size: 790 bytes --] On Tue, 17 Jan 2017, Daniel Shahaf wrote: > Peter Stephenson wrote on Tue, Jan 17, 2017 at 18:36:06 +0000: > > I think this is the right way of doing it as the explicit > > path should continue to override the more vague autoload with no path > > indicated, and this is safer in case some code decides it needs a > > function and inadvertently resets the path the user carefully decided to > > give the function. > > Suppose $^fpath/foo/bar(N) has two matches, how do I explicitly autoload > the second one? I.e., how do I disambiguate «autoload foo/bar» (with no > leading slash) to load a particular copy of foo/bar? How does any of this interact with searching for functions inside files built with zcompile? (Does zcompile even support having a function name that contains a slash?) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-18 0:06 ` Bart Schaefer @ 2017-01-18 9:21 ` Peter Stephenson 0 siblings, 0 replies; 16+ messages in thread From: Peter Stephenson @ 2017-01-18 9:21 UTC (permalink / raw) To: zsh-workers On Tue, 17 Jan 2017 16:06:22 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > How does any of this interact with searching for functions inside files > built with zcompile? > > (Does zcompile even support having a function name that contains a slash?) It should be seamless. It should search the same files it always would have, except using an explicit path rather than everywhere along $fpath. The function *name* doesn't contain a slash (it can't with this mechanism). The autoload immediately gets separated into dir + name which are stored separately. Functions marked for autoload remain second class citizens in the sense that any time a full definition is encountered it is immediately used and the autoload forgotten about --- that was the point of yesterday's patch. pws ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-17 22:17 ` Daniel Shahaf 2017-01-18 0:06 ` Bart Schaefer @ 2017-01-18 9:17 ` Peter Stephenson 2017-01-18 22:26 ` Bart Schaefer 1 sibling, 1 reply; 16+ messages in thread From: Peter Stephenson @ 2017-01-18 9:17 UTC (permalink / raw) To: zsh-workers On Tue, 17 Jan 2017 22:17:54 +0000 Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Suppose $^fpath/foo/bar(N) has two matches, how do I explicitly autoload > the second one? I.e., how do I disambiguate «autoload foo/bar» (with no > leading slash) to load a particular copy of foo/bar? You don't; the mechanism only applies to the last path component so it will pick the latest version of "bar". Applying resolution by means of searching along a path is exactly what the existing $fpath mechanism is for; the new mechanism doesn't replace that. It gives you direct access to functions if you know for a fact you always want the files in a particular directory loaded. I'm guessing this will usually apply to people's own private functions, and it might in principle apply to an add-on that has its own ideas about paths. It inevitably won't be a good match for finding / resolving things that might be anywhere along a path. But we already have a mechanism for that. pws ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-18 9:17 ` Peter Stephenson @ 2017-01-18 22:26 ` Bart Schaefer 2017-01-19 9:39 ` Peter Stephenson 0 siblings, 1 reply; 16+ messages in thread From: Bart Schaefer @ 2017-01-18 22:26 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: TEXT/PLAIN, Size: 860 bytes --] On Wed, 18 Jan 2017, Peter Stephenson wrote: > On Tue, 17 Jan 2017 22:17:54 +0000 > Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Suppose $^fpath/foo/bar(N) has two matches, how do I explicitly autoload > > the second one? I.e., how do I disambiguate «autoload foo/bar» (with no > > leading slash) to load a particular copy of foo/bar? > > You don't; the mechanism only applies to the last path component so it > will pick the latest version of "bar". I think the implied question was "Given that this works if I don't use an explicit path, why isn't there a way for it to work when I do want to give an explicit path? (Now that explicit paths work at all.)" Where "this works" means having a function name with a slash in it. I think it's a rather rare case that we may not want to bother with, but we've bothered with all sorts of other rarities. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-18 22:26 ` Bart Schaefer @ 2017-01-19 9:39 ` Peter Stephenson 0 siblings, 0 replies; 16+ messages in thread From: Peter Stephenson @ 2017-01-19 9:39 UTC (permalink / raw) To: zsh-workers On Wed, 18 Jan 2017 14:26:12 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > I think the implied question was "Given that this works if I don't use > an explicit path, why isn't there a way for it to work when I do want > to give an explicit path? (Now that explicit paths work at all.)" > > Where "this works" means having a function name with a slash in it. > > I think it's a rather rare case that we may not want to bother with, but > we've bothered with all sorts of other rarities. I think the official answer is "well, you'll just have to lump it". If there's a clear use case that someone needs we can no doubt find a way. I hadn't even registered that this worked (with $fpath) until I started looking. pws ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: PATCH: autoload with explicit path 2017-01-17 18:36 ` PATCH: " Peter Stephenson 2017-01-17 22:17 ` Daniel Shahaf @ 2017-01-18 9:53 ` Peter Stephenson 1 sibling, 0 replies; 16+ messages in thread From: Peter Stephenson @ 2017-01-18 9:53 UTC (permalink / raw) To: Zsh hackers list On Tue, 17 Jan 2017 18:36:06 +0000 Peter Stephenson <p.stephenson@samsung.com> wrote: > I have found a bug, though: "autoload /path/to/foo" takes effect > even if foo is already loaded, which isn't supposed to happen. > Test for this tomorrow. I'll push these two together. This also exemplifies the behaviour I was discussing yesterday. pws diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst index 370394b..5edbe26 100644 --- a/Test/C04funcdef.ztst +++ b/Test/C04funcdef.ztst @@ -468,6 +468,31 @@ >fun2b >fun2a + not_trashed() { print This function was not trashed; } + autoload -Uz /foo/bar/not_trashed + not_trashed +0:autoload with absolute path doesn't trash loaded function +>This function was not trashed + + # keep spec from getting loaded in parent shell for simplicity + ( + if whence spec; then print spec already loaded >&2; exit 1; fi + autoload -Uz $PWD/spec + autoload -Uz $PWD/extra/spec + spec + ) +0:autoload with absolute path can be overridden if not yet loaded +>I have been loaded by explicit path. + + ( + if whence spec; then print spec already loaded >&2; exit 1; fi + autoload -Uz $PWD/extra/spec + autoload spec + spec + ) +0:autoload with absolute path not cancelled by bare autoload +>I have been loaded by explicit path. + %clean rm -f file.in file.out ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-01-19 9:49 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20170112125605eucas1p1b2539afbacec2d28d44c6fd73b0d50af@eucas1p1.samsung.com> 2017-01-12 12:56 ` PATCH: autoload with explicit path Peter Stephenson 2017-01-12 15:40 ` Daniel Shahaf 2017-01-12 15:59 ` Peter Stephenson 2017-01-12 16:09 ` Daniel Shahaf 2017-01-12 16:16 ` Peter Stephenson 2017-01-12 16:23 ` Daniel Shahaf 2017-01-12 16:34 ` Peter Stephenson 2017-01-12 16:05 ` Vin Shelton 2016-12-11 22:18 PATH: " Peter Stephenson [not found] ` <CGME20161212160617epcas2p16960e3d95c694147035f760090e6011b@epcas2p1.samsung.com> 2016-12-12 16:05 ` Bart Schaefer 2016-12-12 16:31 ` Peter Stephenson 2017-01-10 19:31 ` Peter Stephenson 2017-01-17 18:36 ` PATCH: " Peter Stephenson 2017-01-17 22:17 ` Daniel Shahaf 2017-01-18 0:06 ` Bart Schaefer 2017-01-18 9:21 ` Peter Stephenson 2017-01-18 9:17 ` Peter Stephenson 2017-01-18 22:26 ` Bart Schaefer 2017-01-19 9:39 ` Peter Stephenson 2017-01-18 9:53 ` 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).