Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Use lstat instead of stat when calculating mtime #41

Merged
merged 1 commit into from

5 participants

Evan Broder Don't Add Me To Your Organization a.k.a The Travis Bot Thibaud Guillaume-Gentil Rémy Coutable Maher Sallam
Evan Broder

Prevents an exception when getting the mtime of a broken symlink.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged 78e0f2d4 into 7a2b49e).

Thibaud Guillaume-Gentil
Owner

@Maher4Ever sounds good to you?

Rémy Coutable
Owner

Could it be possible to add a spec with a non-broken symlink and a broken symlink?

Maher Sallam
Collaborator

Thanks for the patch. It does work, but I would also appreciate it if you could add some specs as @rymai suggested.

Evan Broder

Ok. I've added a few specs that document the current behavior, although I don't entirely have a good understanding of whether the current behavior is desired.

In particular, it seems to handle broken symlinks somewhat inconsistently - for example, changing the target of a broken symlink (while leaving it broken) is picked up as removing the symlink. I think this is sufficient for my current use case (my primary interest is that it does not crash), but it seems like there's potentially room for improvement.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 27e11932 into 7a2b49e).

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged f4810948 into 7a2b49e).

Rémy Coutable
Owner

Thanks @ebroder for the specs, could you rebase your branch on current master (and possibly squash all the commits into a single one)?

@Maher4Ever what do you think about the behavior for broken symlinks?

Evan Broder ebroder Use lstat instead of stat when calculating mtime
Prevents an exception when getting the mtime of a broken symlink.
9bc5136
Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 9bc5136 into 795bbce).

Thibaud Guillaume-Gentil thibaudgg merged commit ddb3ee5 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 21, 2012
  1. Evan Broder

    Use lstat instead of stat when calculating mtime

    ebroder authored
    Prevents an exception when getting the mtime of a broken symlink.
This page is out of date. Refresh to see the latest.
2  lib/listen/directory_record.rb
View
@@ -312,7 +312,7 @@ def existing_path?(path)
# @return [Fixnum, Float] the mtime of the file
#
def mtime_of(file)
- File.mtime(file).send(HIGH_PRECISION_SUPPORTED ? :to_f : :to_i)
+ File.lstat(file).mtime.send(HIGH_PRECISION_SUPPORTED ? :to_f : :to_i)
end
end
end
31 spec/listen/directory_record_spec.rb
View
@@ -1103,5 +1103,36 @@
}.should_not raise_error(Errno::ENOENT)
end
end
+
+ context 'with symlinks' do
+ it 'looks at symlinks not their targets' do
+ fixtures do |path|
+ touch 'target'
+ symlink 'target', 'symlink'
+
+ record = described_class.new(path)
+ record.build
+
+ sleep 1
+ touch 'target'
+
+ record.fetch_changes([path], :relative_paths => true)[:modified].should == ['target']
+ end
+ end
+
+ it 'handles broken symlinks' do
+ fixtures do |path|
+ symlink 'target', 'symlink'
+
+ record = described_class.new(path)
+ record.build
+
+ sleep 1
+ rm 'symlink'
+ symlink 'new-target', 'symlink'
+ record.fetch_changes([path], :relative_paths => true)
+ end
+ end
+ end
end
end
Something went wrong with that request. Please try again.