Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Renaming directory doesn't work #212

Closed
tdreyno opened this Issue · 18 comments

5 participants

@tdreyno

Renaming a directory correctly sends the run_on_deletion hooks for the old path, but never sends run_on_change for the new paths.

Using guard 0.9.4 with :watch_all_modifications => true

@thibaudgg
Owner

Changes are fired at files level so it's quite normal to not receive run_on_change for the new path, I think the problem rather comes form receiving the run_on_deletion hook on directory event.
Do you got the same behavior for file renaming?

@tdreyno

run_on_deletion is getting paths for each of the files in the renamed directory. This is correct behavior in my opinion.

Why would it be expected not to receive run_on_change for renamed directories, but it is fired on new files and renamed files? I think run_on_change should work like run_on_deletion and trigger with paths for each of the new files in the directory.

dir1/file1.txt -> dir2/file1.txt should trigger:

run_on_deletion with "dir1/file1.txt"

and

run_on_change with "dir2/file1.txt"

@thibaudgg
Owner

Ok I understand it better now, I think your right.
Can you provide a failing spec and a patch for that? It would be awesome, thanks.

@tdreyno

BTW, rb-fsevent does return these events correctly and in the right order. I'll work up a test spec.

@thibaudgg
Owner

Great, thanks

@thibaudgg
Owner

Which rb-fsevent version do you use? Because the version of rb-fsevent "vendored" in Guard should only return the parent directory of the renamed directory.

@thibaudgg
Owner

After some time passed to study this issue, I think I understand well the problem and :watch_all_modifications => true feature must be certainly completely rewrited using a Tree structure like fssm to keep trace of all created/moved/deleted paths.
@ttilley maybe it's time to think about extracting/merging the listening stuff (windows support, checksums_hash,...) of Guard inside fssm or the other way around :). There's certainly a lot of overlapping here.

I think this issue is also related with #174 and #218. So for now watch_all_modifications option is not to be considerated as rock solid (sadly).

PS: After this rewrite maybe we'll be able to remove this option and watch all modifications all the time, that would be great!

@ttilley
@ttilley

@thibaudgg - I forgot you were already given repo access to FSSM. Added @netzpirat and @rymai to the list of people with commit rights. My preference is to integrate functionality from guard into FSSM, as this then improves multiple apps and libraries, not just guard (compass, sass, middleman, etc).

What I meant earlier by inefficient is that, to get a list of changes, it ends up rebuilding a fairly large subsection of the tree on an event. A subsection that's far larger than it strictly has to be. I've had a TODO item to improve that for some time now, but until you reach a certain size of watched content it really doesn't matter much unless you use polling. Performing checksums would put the "too many files" point at a significantly lower number though...

My personal priority list is:

  1. rewrite ttilley/TSITString so that it performs exactly zero Range(0,0) insertions, as the CFData/NSData APIs hide the fact that this just results in a very expensive memcpy() rather than a new allocation and subsequent management of non-contiguous memory (transparent handling of non-contiguous memory only exists in lion, as part of dispatch IO). The canonical implementation of tagged netstrings handles this by writing to memory backwards, then flipping, which results in less copying around memory... but it's kinda icky. I'm going to use a singly linked list with a doubly linked header, which seems wasteful for some of the sections that get prepended/appended, but anything will be better than the amount of copying i'm doing... (ew ew ew https://github.com/ttilley/TSITString/blob/master/TSITString/TSICTString.c#L91 )

  2. clean up fsevent_watch into something that looks more like a reusable library wrapped in a CLI interface rather than the monolithic mass of code it is currently

  3. add the ability to send configuration directives to fsevent_watch via a tnetstring or yaml or json or w/e on STDIN. handling CLI options in anything approaching sanity makes it really difficult to handle different options for different paths. I really don't feel like writing my own option parser, and nothing out there supports this kind of parsing (or options being parsed as unsigned long long) without major refactoring.

  4. update ruby half of rb-fsevent to represent the functionality exposed by fsevent_watch

  5. finish cleanup and refactoring of FSSM, including the removal of previously necessary hacks surrounding rb-fsevent now that it supports what FSSM needs directly (multi-path support on the ruby side of things, not just in fsevent_watch)

With my glacial pace and tendency to get distracted by shiny objects, a timeline for the above would be nothing but a blatant lie. Let me know if I should flip that list and get FSSM on top. ;)

I recently did some backend cleanup that'd help you in making use of FSSM as it currently exists if that's your desire: changing the default backend is now easy, and you can add your own backends before a default is chosen. Performance might not be where you want for mid to large trees until the update structure is rethought though. I'm sure there are all kinds of ideas and improvements in guard that'd be great to pull in above and beyond backend support. =]

@thibaudgg
Owner

@ttilley after a discussion on #guard (irc.freenode.net) with @netzpirat and @rymai we all agree that extracting Guard Listeners to its own gems it's a smart move, but we're not sure if it's a good idea to merge it directly inside FSSM. What do you think about creating a new gem (@netzpirat has proposed Listener) with its own github organization (or maybe in Guard organization if that doesn't bother you) so we can begin from scratch and merge all good idea from FSSM and Guard?
I also think there are some pretty good ideas to take from vigilo from @wycats, like using a two-level hash instead of a tree structure (way more simple I think!).

To accomplish that, I was thinking to that todo list:

  1. Define Gem name / repository / organization stuff (using Pivotal Tracker or just Github Issues...)
  2. Write README and defining first public API version (maybe something between FSSM and Vigilo)
  3. Write RSpec
  4. Write Polling listeners
  5. Add systems backend support (Darwin, Linux, Windows)
  6. Found a Windows user to test it (point featured by @netzpirat)

This new gem must be Thread safe and work on all major Ruby VMs (MRI 1.8.7/1.9.x, JRuby, Rubinius). Obviously it would be great if the migration for FSSM users is simple to do.

All that is clearly open to discussion, but I think we have all what's needed to build a great gem!

@thibaudgg
Owner

BTW we should maybe continue this discussion to a new issue or in the guard google group. :)

@wycats

Thanks for the compliment. At one point I was frustrated by the lack of centralized infrastructure for this sort of thing, and by the size of the implementations on offer. I never got around to finishing it, but am thankful that my initial implementation is useful for someone :heart:

@ttilley

@thibaudgg - fire up a new repo under guard org and start a new issue there. FSSM has only received attention in the past year or two when someone files an issue or makes a suggestion on github. Starting from scratch and just taking over the good parts might be a good idea. My concern being, of course, the gems that either bundle or depend upon FSSM: google search (not many, but still)

@netzpirat
Owner

I didn't know viglio until now, but I looks like a simple and straight forward solution. I will start writing some specs and see how much it conforms to what we need for Guard.

I like the public API of viglio, but we can also add a compatibility layer to FSSM, so that it can be used as a drop in replacement.

@thibaudgg
Owner

Ok there is the new Listener repository under the Guard repo with a new issue about defining the API.

@ttilley once the listener gem is ready to production, maybe you can deprecate FSSM and add a deprecation warning message in the gem + README asking people to upgrade to the new gem. We must also trying to contact all FSSM/Vigilo users via Github directly so they could participate to the new API elaboration and talk about their need (multi-directories, directory events support…)

@wycats are you interested to have a collaborator access to this new repository and participate? It would be an honor! :)

@thibaudgg
Owner

Fixed in Guard 1.1

@thibaudgg thibaudgg closed this
@tdreyno

<3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.