Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gracefully handle unreadable directories #86

Merged
merged 3 commits into from
Aug 22, 2018

Conversation

owst
Copy link
Contributor

@owst owst commented Aug 18, 2018

If there is an unreadable directory considered by ripper-tags, an exception is raised:

.../lib/ripper-tags/data_reader.rb:59:in `open': Permission denied @ dir_initialize - foo/bar (Errno::EACCES)
	from .../lib/ripper-tags/data_reader.rb:59:in `entries'
	from .../lib/ripper-tags/data_reader.rb:59:in `resolve_file'
	from .../lib/ripper-tags/data_reader.rb:63:in `block in resolve_file'

(tested with 0.6.0)

This PR gracefully handles such directories, by skipping them (and logging if --verbose is given).

To reproduce the existing behaviour, run:

$ mkdir -p a/b
$ touch a/b/c.rb
$ chmod 355 a/b
$ ripper-tags --tag-file=/dev/null --recursive --verbose a # Errors

with this PR, we now have

$ bundle exec bin/ripper-tags --tag-file=/dev/null --recursive --verbose a
Ignoring unreadable directory `a/b'

Copy link
Collaborator

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Thank you for handling this.

@@ -75,6 +71,21 @@ def resolve_file(file, depth = 0, &block)
end
end

def resolve_files_in_directory(directory, depth, &block)
unless File.readable?(directory)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking File.readable? could we handle an exception from Dir.entries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review - sure, I've pushed a fixup to do that

owst and others added 3 commits August 22, 2018 10:50
The `rescue Errno::EACCES` clause now only applies to `Dir.entries()`
call and not to the yielded block.
@mislav mislav force-pushed the handle_unreadable_directories branch from 209f8f7 to 131dbcf Compare August 22, 2018 08:50
@mislav mislav merged commit bcc3b1b into tmm1:master Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants