need help with enhancement to prevent completion from stat'ing automounts

need help with enhancement to prevent completion from stat'ing automounts

[Greg Klanderman]

Hi,

I am finally getting back to an issue I posted about back in January.
I am trying to use the fake-files zstyle to configure several hundred
automounts under /net and /home.  This mostly works, until zsh decides
to stat them all in order to decide which suffix to use for
completion.  What's happening is something like this:

1. _path_files calls compfiles passing in the fake-files zstyle data.
2. by the time compfiles returns, all memory of which values in the
   result were "fake" has been lost.
3. eventually _path_files calls compadd with the '-f' argument and the
   completions, meaning that compadd will stat all the values to
   figure out which are directories and which are files, in order to
   add the correct suffix.
4. compadd stats the fake automount points, causing the automounter to
   potentially mount hundreds of locations (already not good), and
   hang for a very long while timing out on a few tens that no longer
   exist that our ops department has not cleaned up.

I would like to create some configuration (via zstyle or whatever) to
prevent zsh from stat'ing these locations, and just assume they are
directories.  I was able to hard-code some logic in the ztat()
function in compresult.c which does exactly this for /net/* and
/home/*, and it works perfectly.

Now I just need some help to figure out what a reasonable way to
create a general configuration to support this would be.

thanks,
Greg

Re: need help with enhancement to prevent completion from stat'ing automounts

[Greg Klanderman]

>>>>> Greg Klanderman <gak@klanderman.net> writes:

> Now I just need some help to figure out what a reasonable way to
> create a general configuration to support this would be.

So it looks like a zstyle would not work, because I need to access the
configuration from C.  Seems like the only real option is to create an
array parameter - does that make sense?

thanks,
Greg

Re: need help with enhancement to prevent completion from stat'ing automounts

[Peter Stephenson]

On Mon, 07 Sep 2009 22:14:40 -0400
Greg Klanderman <gak@klanderman.net> wrote:
> >>>>> Greg Klanderman <gak@klanderman.net> writes:
> 
> > Now I just need some help to figure out what a reasonable way to
> > create a general configuration to support this would be.
> 
> So it looks like a zstyle would not work, because I need to access the
> configuration from C.  Seems like the only real option is to create an
> array parameter - does that make sense?

It's fairly easy to pass an array into C, particularly if it's going into
compfiles which is the accelerator for _path_files and so has its own
exclusive API, but if you really need to grope in compresult.c it's
trickier.  I'm not sure what API you need to access; the core completion
system is heavily customized to make it possible to do clever things with
multiple sets of completions, so an array applying to all of them is
probably too gross.  Still, if the stat is that low down in the system
you've got problems.  It should be possible to add an option to compadd
giving an array of special things and store that in an appropriate group of
results, parallel to the various other things that go into compadd.  This
is all quite hairy.

Is your case fixed by not passing the "-f" to compadd?  Then the problem is
limited to _path_files and compfiles, which should be easier to trace.
Separating fake and real files into different compadd calls should be
possible (you're now using to advantage the facility for multiple matches
that I just explained above makes it harder to change the low level stuff).

There's a problem that if you're not statting the file you've completed you
obviously don't know if it's a directory, so if you complete /home/u to
/home/user you don't get the slash.  Maybe (part of) the solution is a
fake-dirs style, for stuff that is added in as directory, i.e. with a /
suffix, but without the -f argument to compadd?  That still needs code in
_path_files but possibly not elsewhere (I'm not clear what you need to do
with compfiles here).

I haven't seen an easier way around your original problem---unnecessarily
accessing files can be quite a big problem particular on some systems
(Cygwin in my case) so a fairly general solution to this would certainly be
a good thing.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: need help with enhancement to prevent completion from stat'ing automounts

[Greg Klanderman]

Hi Peter, thank you for having a look at this..

I'm quite certain that passing the 'fake' files and directories to
compadd would solve the problem here.  I will acknowledge that IMO the
most correct change would be to introduce a 'fake-dirs' zstyle and fix
all of _path_files and compfiles to maintain these fake completions
separately so they can be given to compadd with the correct suffix
rather than '-f' when the time comes.

But I find those two pieces of code so impenetrable that I'm having
trouble forcing myself down that path.  And so another idea is to add
an array parameter, say completion_automount_roots, which I could
maybe set to (/net /home) and have bound to a C variable, then in the
ztat() function return 'directory' without stat()ing any files
directly under those roots.  Not pretty, but I can see exactly what
needs to be done and it's not too hard.

I'll comment on a few bits or your reply below..

>>>>> On September 8, 2009 Peter Stephenson <pws@csr.com> wrote:

> It's fairly easy to pass an array into C, particularly if it's going into
> compfiles which is the accelerator for _path_files and so has its own
> exclusive API, but if you really need to grope in compresult.c it's
> trickier.  I'm not sure what API you need to access; the core completion
> system is heavily customized to make it possible to do clever things with
> multiple sets of completions, so an array applying to all of them is
> probably too gross.

Right, though in this case it's pretty certain you don't want to stat
all several hundred automounts no matter what the mechanism you got to
that completion was.

> Still, if the stat is that low down in the system
> you've got problems.  It should be possible to add an option to compadd
> giving an array of special things and store that in an appropriate group of
> results, parallel to the various other things that go into compadd.  This
> is all quite hairy.

The difficulty is maintaining those three sets of completions all the
way from the compfiles call to one of the compadd calls (whichever one
you get to) in _path_files.

Also you have to be a little careful - currently, it seems that
compfiles is merging a 'fake-file' of the same name as a mounted
automount which the normal completion finds.  You only want the
mounted automounts to appear in the completion list once, not twice.

> Is your case fixed by not passing the "-f" to compadd?  Then the problem is
> limited to _path_files and compfiles, which should be easier to trace.

"Easier" .. ha ha!

> Separating fake and real files into different compadd calls should be
> possible (you're now using to advantage the facility for multiple matches
> that I just explained above makes it harder to change the low level stuff).

In theory, I agree.

> There's a problem that if you're not statting the file you've completed you
> obviously don't know if it's a directory, so if you complete /home/u to
> /home/user you don't get the slash.  Maybe (part of) the solution is a
> fake-dirs style, for stuff that is added in as directory, i.e. with a /
> suffix, but without the -f argument to compadd?  That still needs code in
> _path_files but possibly not elsewhere

Yes of course you need to tell it to use '/'.

> (I'm not clear what you need to do with compfiles here).

the fake-files are passed into compfiles, and by the time it returns,
all memory of what's fake and what's not has been lost.

> I haven't seen an easier way around your original problem---unnecessarily
> accessing files can be quite a big problem particular on some systems
> (Cygwin in my case) so a fairly general solution to this would certainly be
> a good thing.

Accessing lots of files seems fine, it's accessing hundreds of
automounts many of which are no longer valid here at work and take 60
sec each to time out that it's really a problem..

thanks,
Greg

Re: need help with enhancement to prevent completion from stat'ing automounts

[Peter Stephenson]

On Tue, 08 Sep 2009 19:39:21 -0400
Greg Klanderman <gak@klanderman.net> wrote:
> I'm quite certain that passing the 'fake' files and directories to
> compadd would solve the problem here.  I will acknowledge that IMO the
> most correct change would be to introduce a 'fake-dirs' zstyle and fix
> all of _path_files and compfiles to maintain these fake completions
> separately so they can be given to compadd with the correct suffix
> rather than '-f' when the time comes.
> 
> But I find those two pieces of code so impenetrable that I'm having
> trouble forcing myself down that path.  And so another idea is to add
> an array parameter, say completion_automount_roots, which I could
> maybe set to (/net /home) and have bound to a C variable, then in the
> ztat() function return 'directory' without stat()ing any files
> directly under those roots.  Not pretty, but I can see exactly what
> needs to be done and it's not too hard.

It's high time we fixed this properly.  The whole problem with the
completion system is arbitrary bits getting added without understanding the
existing bits, and each time that happens it gets worse.

Both Bart and I have been bamboozled by the networks of special cases and
clever features in _path_files, but we need to track them down if we want
to make file completion maintainable.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: need help with enhancement to prevent completion from stat'ing automounts

[Greg Klanderman]

>>>>> On September 10, 2009 Peter Stephenson <pws@csr.com> wrote:

> It's high time we fixed this properly.  The whole problem with the
> completion system is arbitrary bits getting added without understanding the
> existing bits, and each time that happens it gets worse.

OK then, I'll give the right fix a try.. hopefully if I pick off some
easier fixes in _path_files first it will boost my confidence.

greg

Re: need help with enhancement to prevent completion from stat'ing automounts

[Greg Klanderman]

>>>>> On September 10, 2009 Peter Stephenson <pws@csr.com> wrote:

>> It's high time we fixed this properly.  The whole problem with the
>> completion system is arbitrary bits getting added without understanding the
>> existing bits, and each time that happens it gets worse.

Hi all,

Just an update on this; since actually adding support for a fake-dirs
zstyle and having both that and the fake-files zstyle not stat the
fake files and directories is quite a bit of work, as a temporary
solution for my company I made a relatively simple change so that in
the interim we can have working completion of automounts, by
introducing an array parameter to configure paths in which stating
should be avoided in the ztat function.

I'm pretty sure Peter isn't interested in committing this fix, but I
figured it was worth posting the patch just in case others out there
also need a temporary solution.

You'd use this for configuring automounts as follows:

  completion_nostat_dirs=( /net /home )
  zstyle ':completion:*' fake-files \
         '/net:netdir1 netdir2 .. netdirN' \
         '/home:homedir1 homedir2 .. homedirM'

In my actual usage I generate those arguments based on the contents of
/etc/auto.* or output of 'ypcat -k auto.*'.

cheers,
Greg


Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.106
diff -u -r1.106 init.c
--- Src/init.c	12 Jul 2009 15:10:07 -0000	1.106
+++ Src/init.c	13 Nov 2009 20:51:25 -0000
@@ -799,6 +799,8 @@
     modulestab = newmoduletable(17, "modules");
     linkedmodules = znewlinklist();
 
+    completion_nostat_dirs = mkarray(NULL);
+
     /* Set default prompts */
     if(unset(INTERACTIVE)) {
 	prompt = ztrdup("");
Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.157
diff -u -r1.157 params.c
--- Src/params.c	5 Sep 2009 19:49:20 -0000	1.157
+++ Src/params.c	13 Nov 2009 20:51:25 -0000
@@ -102,6 +102,10 @@
      zoptind,		/* $OPTIND      */
      shlvl;		/* $SHLVL       */
 
+/**/
+mod_export
+char **completion_nostat_dirs;
+
 /* $histchars */
  
 /**/
@@ -343,6 +347,9 @@
 #define IPDEF9(A,B,C) IPDEF9F(A,B,C,0)
 IPDEF9F("*", &pparams, NULL, PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT|PM_READONLY),
 IPDEF9F("@", &pparams, NULL, PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT|PM_READONLY),
+
+IPDEF9("completion_nostat_dirs", &completion_nostat_dirs, NULL),
+
 {{NULL,NULL,0},BR(NULL),NULL_GSU,0,0,NULL,NULL,NULL,0},
 
 #define IPDEF10(A,B) {{NULL,A,PM_ARRAY|PM_SPECIAL},BR(NULL),GSU(B),10,0,NULL,NULL,NULL,0}
Index: Src/Zle/compresult.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compresult.c,v
retrieving revision 1.78
diff -u -r1.78 compresult.c
--- Src/Zle/compresult.c	24 Apr 2009 09:00:38 -0000	1.78
+++ Src/Zle/compresult.c	13 Nov 2009 20:51:25 -0000
@@ -863,11 +863,37 @@
 ztat(char *nam, struct stat *buf, int ls)
 {
     int ret;
+    char **pp;
 
     nam = unmeta(nam);
     if (!nam)
 	return -1;
 
+    for (pp = completion_nostat_dirs; *pp; pp++) {
+        int len = strlen(*pp);
+
+        if ((*pp)[len-1] == '/')
+            len--;
+
+        if (strncmp(nam, *pp, len) == 0 && nam[len] == '/' && strchr(nam+len+1, '/') == NULL) {
+            buf->st_dev = 0;      /* device */
+            buf->st_ino = 0;      /* inode */
+            buf->st_mode = S_IFDIR;     /* protection */
+            buf->st_nlink = 0;    /* number of hard links */
+            buf->st_uid = 0;      /* user ID of owner */
+            buf->st_gid = 0;      /* group ID of owner */
+            buf->st_rdev = 0;     /* device type (if inode device) */
+            buf->st_size = 0;     /* total size, in bytes */
+            buf->st_blksize = 0;  /* blocksize for filesystem I/O */
+            buf->st_blocks = 0;   /* number of blocks allocated */
+            buf->st_atime = 0;    /* time of last access */
+            buf->st_mtime = 0;    /* time of last modification */
+            buf->st_ctime = 0;    /* time of last status change */
+
+            return 0;
+        }
+    }
+
     if ((ret = ls ? lstat(nam, buf) : stat(nam, buf))) {
 	char *p, *q;