fix + workarounds for editor support on Linux

[e2]

• Linux/inotify: relies on close_write where
  attribs are not updated (e.g. ecryptfs)
• handle rename() calls used by Kate and
  Sublime by detecting unrelated move_to
• all local adapters: logically groups related
  fs events into single changes (or skip them
  completely)
• avoids notifying multiple events per file
• fix tests to more consistently use Pathname
  instead of strings
• additional tests for _smoosh_changes to test
  for and handle various editor-related
  differences in the future
• use "shift" instead of "pop" to process
  received events in order (for whatever future
  reason)

[e2]

TL;DR: With this, Linux editing should be fixed - including Kate and Sublime, without breaking backward compatibility (in a way that actually would affect anyone).

I've just added support for editors using rename() (Kate and Sublime) and squashed the changes with my previous pull request.

Why?

Because I consider using rename() a good approach to saving files - and since changing the Guard API could take a while, I thought this would be a good "maintenance" commit.

Will it work?

I'm 95% confident this doesn't break anything. There aren't many new tests, but with the coverage it's hard to change a single letter anywhere without breaking anything.

Implementation details

For Sublime/Kate I'm detecting an "unrelated" move_to (the only event captureable without continuously re-listening the file upon every save) - by "unrelated" I mean using the inotify cookies to detect the rename() as something not related to a move.

What's different after the fix

So the only thing broken by this (AFAIK) would be moving a file/directory from an external, untracked directory into one tracked by listen, because it would cause a "modify" instead of a "added".

I can't imagine a scenario where this would be an extreme problem for anyone, since the only thing worse than getting tests run too many times after editing - is not having them run at all.

This means everything else should work as designed - even with the unfortunate early design choice to separately notify about additions/removals (which is why this commit so big and can't be broken up into working commits, because everything is so interdependent for this to work).

I'm not that eager about further working on this (I hope I've made it as clear and modular and robust as possible), because I'd prefer to spend time on the next API's for guard and listen.

But if it doesn't work or breaks something, I'd like to know.

[e2]

Damn it - it doesn't work - it triggers an added event.

[e2]

TL;DR: NOW it works.

The amount of code and changes required for this is horrible.

Right now, the rule for detecting an "edited" file is as follows:

If something ignored is renamed to something not ignored (e.g. editor renames temp file during saving), then that's marked as "modified" instead of "added", which should trigger run_on_modifications (as it does for me now for Sublime Edit 3 after this fix).

The commit shows a few things worth reorganizing for the upcoming listen API change - and how unreasonably complex and impractical it is to support run_on_additions for all edge cases and use case. (e.g. half of Linux adapter specific code is in the listener).

The test cases for this are hideous - setting tons of mocks on pathnames, silencer, inotify cookies. I'd reorganize things, except many tests would have to be changed for the next API anyway, so it seems like useless work.

[thibaudgg]

If we know that adding that "unreasonably complex" logic is only transitional before 3.x release that's fine for me.

Thanks for your work.

[e2]

Yes - cleaning this up more isn't worthwhile or possible without removing the :removed and :added events from the API and adapters - which would make all this code (and many other parts of listen) unnecessary.

I'm now more worried I didn't screw up anywhere - but it's probably no worse than not having these patches.

On the flip side - if something stops working with this for someone after this patch, I'll help sort it out.

So this patch is for reducing issues people may have with the 2.x branch, until 3.x can replace it completely (which may take time to get right).

Aside from that it's been a very good study case for me regarding changes and considerations for the new API...

[thibaudgg]

Ready to merge then? :)

[e2]

I've reviewed it and tested it again (Sublime Editor seems happy about it - as far as I've tested it).

I'm horrified by the amount of changes and the complexity necessary to make this work ...

... but again, the changes are necessary for "everything" to work in unison:

• postponing "ignore" functionality until last step, so I can detect renaming editor swap files using rename()
• "leaking" the moved_to, moved_from rb-inotify specific events into the listener
• skipping on TCP adapter, because Pathname.exist? obviously won't work ...
• Pathname usage in tests (because of checking exist?) [ which is a good thing, actually, since strings shouldn't be used for filenames in listen ]
• rb-inotify cookies to detect rename()
• the horrible _reinterpret_related_changes where the leaked moved_to/moved_from are translated
• the complex _squash_changes method which just inverts the hash

I designed things, so in the worst case edge-case scenarios it would "default" to the previous functionality (aggressively removing the rb-inotify leaks, conservatively detecting special editor rename() calls) ..

.. while changing the behavior in some cases, where it seemed reasonable ("smart" modification detecting, single event per file), handling close_write for special file systems.

What might be broken is detecting changes on directories themselves, but since directories don't have content themselves (they're just lists of file names and attributes), I lack a good use case (failing test) to wrap my mind around how they should be handled.

In other words, I tried really hard to make as many people happy as possible, and making as few sadder as possible. If it's broke, I'll add tests and fixes (I already can't live without these patches...).

Anyway, most of this will disappear from listen 3.x without a trace.

[e2]

I rebased to #218, so this can wait until a new gem is released first (for #217).

[thibaudgg]

I'll merge that and release 2.7.2 for both #217 and #218, ok for you?

[thibaudgg]

Just saw your email, I'll release 2.7.2 with just #218 for now. Thanks!

[e2]

No prob. Thanks as well!

[e2]

If you're ok with the changes, I think this can be safely merged into master, because:

• I see no reason why it would need to ever be reverted instead of fixed
• if there are bugs, I'd like to know about them (and fix them)
• if people are having any issues with editors, master is a good starting point
• even if I've screwed up or broken something, the environment variable should make diagnosing problems much faster and easier (LISTEN_GEM_DEBUGGING=1)
• it's a good base for further pull requests
• I can switch to using master myself

Issues:

• logging through Celluloid seems wrong, especially with extra Celluloid warnings (that can be safely ignored but not avoided), but ultimately adapters would have their own logging (and options for it) and listen by itself won't have much left to do, anyway
• if this slows down performance (e.g. due to fstat calls + ignores happening later), the 3.x API changes will take care of that
• if there's something broken with directories or edge cases, we need use cases first, then tests anyway

Thoughts?

[thibaudgg]

Done, I can release 2.7.3 as well if you want.

[e2]

I think it's worth releasing 2.7.3 now - guard/guard-rspec#262

So yes, please if you have some spare time to release these, I'd appreciate it!

For remaining issues I keep concluding that focusing on Listen 3.x is the best use of time (it's tough, lots of work, but from what I browsed, many issues in guard plugins will disappear as "fixed"). Especially since the API changes for Listen 3.x are really keeping me busy.

[thibaudgg]

Sure, 2.7.3 released. Go for 3.x, thanks!

[pablox-cl]

Only wanted to say thanks, I can finally work with vim (using listen 2.7.4) =)

[e2]

I'm glad it was worth it, @pablox-cl!