Re: [gamin] gam-poll.c rewrite



On Thu, Jun 30, 2005 at 11:21:30PM +0100, Neal H. Walfield wrote:
> > basic4.py: monitors a directory and expects change events on changes
> >            of files in the directory.  The SGI documentation is clear
> >            that change events are only sent if the stat information
> >            changes.  This is not the case if a file changes.
> 
> I've changed my position on this: fthe SGI documention says:
> 
>   FAMMonitorDirectory() monitors not only changes that happens to the
>   contents of the specified directory file, but also to the files in
>   the directory. If the directory contains subdirectories,
>   FAMMonitorDirectory() monitors changes to the subdirectory files,
>   but not the contents of those subdirectories. FAMMonitorFile()
>   monitors only what happens to the specified file. Both functions
>   return 0 if successful and -1 otherwise.
> 
> I've changed the patch to reflect this.  The changes were relatively
> small (only in gam_poll.c and gam_subscription.c).  I've included the
> patch in its entirety for your convenience.
> 
> > dnotify.py: does a watch_file but should do a watch_directory
> 
> I've also changed by position on this due to the above as well as
> nautilus' which often monitors a directory as a file e.g. in
> fm-directory-view.c:load_directory.  Sending a delete event (because
> the file does not exist) instead of an exist event (because the node
> exists) causes nautilus to close the window thinking the directory has
> been deleted.  I think that is a valid interpretation and now
> understand FAMMonitorFile to mean that the FAM server should watch for
> file type changes on the node while FAMMonitorDirectory means the FAM
> server should watch for changes in the content of the node.
> 
> dnotify4.py now fails: we were testing that monitoring a file as a
> directory returns a deleted event.
> 
> > dnotify6.py: monitors a missing file then creates a directory with the
> >            same path and expects a create event.  I think this is
> >            incorrect: a create event should only be sent to file
> >            subscriptions
> 
> now passes
> 
> > dnotify12.py: similar problem as basic4.py
> 
> passes if you add a f.flush after the f.write().
> 
> > dnotify13.py: get an extra change event on temp_dir/a.  I've traced it
> >            and it seems legitimate: there is one event for a file size
> >            change and one for an mtime change.  I am not entirely
> >            clear what is happening, however it may have something to
> >            do with the way python writes to the file.
> 
> seems tto psas
> 
> > flood4.py: there is a timing problem: the thread creates the
> >            directories before gamin does the initial scan.  Creating
> >            the thread after we get the Exist events on the directory
> >            fixes this.
> 
> problem still exists
> > 
> > 4.tst: same problem as dnotify6.py
> > 9.tst: same problem as dnotify.py
> 
> both now pass
> 
> 
> And nautilus works for me.

  Okay, good. But this breaks for me and this is a huge patch.
The dnotify.py regression test breaks and nearly all subsequent ones.
Not breaking the regression tests on dnotify is really a hard condition
to apply any patch at this stage unless we can understand and justify why:

[make tests output]
bigfile.py
noexists.py
dnotify.py
Traceback (most recent call last):
  File "/u/veillard/gamin/python/gamin.py", line 107, in _internal_callback
    self.callback (path, event, self.data)
  File "./dnotify.py", line 26, in debug
    if db_expect[dbg] != type:
IndexError: list index out of range
Traceback (most recent call last):
  File "/u/veillard/gamin/python/gamin.py", line 107, in _internal_callback
    self.callback (path, event, self.data)
  File "./dnotify.py", line 26, in debug
    if db_expect[dbg] != type:
IndexError: list index out of range
Traceback (most recent call last):
  and then pages of errors.

Maybe it's just the debugging of kernel monitoring status which got broken,
but it looks like your patches changes the behaviour of the kernel monitoring
a lot, even in the simplest case. there are way too many dnotify event sent
which mean way too many transition of state for the dnotify watches, and this
occurs even on the simplest case, so in practice I think this is a serious
breakage of the dnotify back-end.
  So as is that patch which is nearly 100KB doesn't pass. Could you chunk it
into digestible pieces with a well identified purpose ? It will allow me to
review them and help pinpoint where the problem is being introduced.

  thanks !

Daniel


-- 
Daniel Veillard      | Red Hat Desktop team http://redhat.com/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]