From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8786 invoked by alias); 12 Jan 2017 12:56:19 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 40335 Received: (qmail 18814 invoked from network); 12 Jan 2017 12:56:19 -0000 X-Qmail-Scanner-Diagnostics: from mailout1.w1.samsung.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(210.118.77.11):SA:0(-8.2/5.0):. Processed in 4.25236 secs); 12 Jan 2017 12:56:19 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-8.2 required=5.0 tests=RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: p.stephenson@samsung.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at samsung.com does not designate permitted sender hosts) X-AuditID: cbfec7f4-f79716d000006f65-92-58777ce53778 Date: Thu, 12 Jan 2017 12:56:02 +0000 From: Peter Stephenson To: Zsh Hackers' List Subject: Re: PATCH: autoload with explicit path Message-id: <20170112125602.4fa64dc6@pwslap01u.europe.root.pri> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrJIsWRmVeSWpSXmKPExsWy7djPc7pPa8ojDP6esrY42PyQyYHRY9XB D0wBjFFcNimpOZllqUX6dglcGefOOxfcsa6YtuEDWwPjG/0uRk4OCQETid333rNB2GISF+6t B7OFBJYySuy7z9XFyAVk9zJJHFq8hr2LkQOsYf2ubIj4MkaJ7xf3M0E0TGOSOLwDKnGGUeLN ll5mCOcso8Sx0xcZQapYBFQl1s5uYQWx2QQMJaZums0IMlVEQFui/aMYSFhYQF9i+udedhCb V8Be4vGuBWAL+IHiV/9+YoK41F5i5pUzjBA1ghI/Jt9jAbGZBXQktm17zA5hy0tsXvMW7AYJ gedsEu3b5jNBfCArsekAM8QcF4lX71tZIWxhiVfHt7BD2DISlyd3s0DY/YwST7p9IebMYJQ4 fWYHNLisJfpuQ/zFLMAnMWnbdGaI+bwSHW1CECUeEjMe74Ka7yhxduEbFkhgxUq83rGWcQKj wiwkL8xC8sIsJC8sYGRexSiSWlqcm55abKJXnJhbXJqXrpecn7uJEZgATv87/mUH4+JjVocY BTgYlXiAoVYWIcSaWFZcmXuIUYKDWUmEd291eYQQb0piZVVqUX58UWlOavEhRmkOFiVx3j0L roQLCaQnlqRmp6YWpBbBZJk4OKUaGO0THx+oYP91J633yoYTNbce3tv3eMbmTJdDjHsKWFgW 5c1m2r1kotIWecuQ5ptuLIpZdVtyzKdx5545IbM6tJRr7pz1yxI/3rpQW+w0Y6sGe7yieSir 4JGev4eMje1aM0w3C8Zo/sy7rtzDWGaUxSSeOeO19m6z21e6K3Z8U9hyqK1f6mR2hBJLcUai oRZzUXEiALpJogf8AgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupikeLIzCtJLcpLzFFi42I5/e/4Zd17NeURBh2rxCwONj9kcmD0WHXw A1MAY5SbTUZqYkpqkUJqXnJ+SmZeuq1SaIibroWSQl5ibqqtUoSub0iQkkJZYk4pkGdkgAYc nAPcg5X07RLcMs6ddy64Y10xbcMHtgbGN/pdjBwcEgImEut3ZXcxcgKZYhIX7q1n62Lk4hAS WMIo0XH7MDOEM4NJYm3fZKjMOUaJfVvmMoG0CAmcZZS4dcMBxGYRUJVYO7uFFcRmEzCUmLpp NiPIBhEBbYn2j2IgYWEBfYnpn3vZQWxeAXuJx7sWgI3hB4pf/fuJCeIKe4mZV84wQtQISvyY fI8FxGYW0JLYvK2JFcKWl9i85i0zxAnqEjfu7mafwCg4C0nLLCQts5C0LGBkXsUoklpanJue W2ykV5yYW1yal66XnJ+7iREYD9uO/dyyg7HrXfAhRgEORiUe3h02ZRFCrIllxZW5hxglOJiV RHj3VpdHCPGmJFZWpRblxxeV5qQWH2I0Bfp/IrOUaHI+MFbzSuINTQzNLQ2NjC0szI2MlMR5 p364Ei4kkJ5YkpqdmlqQWgTTx8TBKdUADNb9hpyNjNw3LPS/sFr8ZJ7KefNfQY/Vk6aMTrlj 8nlsikd+KKnt7fl3UWQbywuWOb0lel/PvPRmVnn+njnOqk377drrt1799rBpuBrm33D725WV wrtXC71cK6dVHDhL/7nutxPLJRWKz/zUEEq887t+A8e+rrncCzc4r3A32bZwWlnu3r+zlFiK MxINtZiLihMBHFoYo50CAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170112125605eucas1p1b2539afbacec2d28d44c6fd73b0d50af X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUb?= =?UTF-8?B?7IK87ISx7KCE7J6QG1ByaW5jaXBhbCBFbmdpbmVlciwgU29mdHdhcmU=?= X-Global-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUbU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtQcmluY2lwYWwgRW5naW5lZXIsIFNvZnR3YXJl?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDA1Q0QwNTAwNTg=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170112125605eucas1p1b2539afbacec2d28d44c6fd73b0d50af X-RootMTR: 20170112125605eucas1p1b2539afbacec2d28d44c6fd73b0d50af References: (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 > To: Peter Stephenson > Cc: Zsh hackers list > 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