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

Jump to bottom

Simplify Crystal::System interface by adding File.stat? and lstat? #5553

Merged
merged 1 commit into from Jan 13, 2018
Merged

Simplify Crystal::System interface by adding File.stat? and lstat? #5553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion spec/std/dir_spec.cr
View file
Open in desktop
Expand Up @@ -42,10 +42,17 @@ describe "Dir" do
end

it "tests empty? on nonexistent directory" do
expect_raises Errno do
expect_raises(Errno, /Error determining size of/) do
Dir.empty?(File.join([__DIR__, "/foo/bar/"]))
end
end

it "tests empty? on a directory path to a file" do
ex = expect_raises(Errno, /Error determining size of/) do
Dir.empty?("#{__FILE__}/")
end
ex.errno.should eq(Errno::ENOTDIR)
end
end

it "tests mkdir and rmdir with a new path" do
Expand Down
77 changes: 76 additions & 1 deletion spec/std/file_spec.cr
View file
Open in desktop
Expand Up @@ -116,10 +116,18 @@ describe "File" do

it "raises an error when the file does not exist" do
filename = "#{__DIR__}/data/non_existing_file.txt"
expect_raises Errno do
expect_raises(Errno, /Error determining size/) do
File.empty?(filename)
end
end

it "raises an error when a component of the path is a file" do
filename = "#{__DIR__}/data/non_existing_file.txt"
ex = expect_raises(Errno, /Error determining size/) do
File.empty?("#{__FILE__}/")
end
ex.errno.should eq(Errno::ENOTDIR)
end
end

describe "exists?" do
Expand All @@ -130,24 +138,52 @@ describe "File" do
it "gives false" do
File.exists?("#{__DIR__}/data/non_existing_file.txt").should be_false
end

it "gives false when a component of the path is a file" do
File.exists?("#{__FILE__}/").should be_false
end
end

describe "executable?" do
it "gives false" do
File.executable?("#{__DIR__}/data/test_file.txt").should be_false
end

it "gives false when the file doesn't exist" do
File.executable?("#{__DIR__}/data/non_existing_file.txt").should be_false
end

it "gives false when a component of the path is a file" do
File.executable?("#{__FILE__}/").should be_false
end
end

describe "readable?" do
it "gives true" do
File.readable?("#{__DIR__}/data/test_file.txt").should be_true
end

it "gives false when the file doesn't exist" do
File.readable?("#{__DIR__}/data/non_existing_file.txt").should be_false
end

it "gives false when a component of the path is a file" do
File.readable?("#{__FILE__}/").should be_false
end
end

describe "writable?" do
it "gives true" do
File.writable?("#{__DIR__}/data/test_file.txt").should be_true
end

it "gives false when the file doesn't exist" do
File.writable?("#{__DIR__}/data/non_existing_file.txt").should be_false
end

it "gives false when a component of the path is a file" do
File.writable?("#{__FILE__}/").should be_false
end
end

describe "file?" do
Expand All @@ -158,6 +194,14 @@ describe "File" do
it "gives false" do
File.file?("#{__DIR__}/data").should be_false
end

it "gives false when the file doesn't exist" do
File.file?("#{__DIR__}/data/non_existing_file.txt").should be_false
end

it "gives false when a component of the path is a file" do
File.file?("#{__FILE__}/").should be_false
end
end

describe "directory?" do
Expand All @@ -168,6 +212,14 @@ describe "File" do
it "gives false" do
File.directory?("#{__DIR__}/data/test_file.txt").should be_false
end

it "gives false when the directory doesn't exist" do
File.directory?("#{__DIR__}/data/non_existing").should be_false
end

it "gives false when a component of the path is a file" do
File.directory?("#{__FILE__}/").should be_false
end
end

describe "link" do
Expand Down Expand Up @@ -204,6 +256,14 @@ describe "File" do
File.symlink?("#{__DIR__}/data/test_file.txt").should be_false
File.symlink?("#{__DIR__}/data/unknown_file.txt").should be_false
end

it "gives false when the symlink doesn't exist" do
File.symlink?("#{__DIR__}/data/non_existing_file.txt").should be_false
end

it "gives false when a component of the path is a file" do
File.symlink?("#{__FILE__}/").should be_false
end
end

it "gets dirname" do
Expand Down Expand Up @@ -376,6 +436,21 @@ describe "File" do
file.size.should eq(240)
end
end

it "raises an error when the file does not exist" do
filename = "#{__DIR__}/data/non_existing_file.txt"
expect_raises(Errno, /Error determining size/) do
File.size(filename)
end
end

it "raises an error when a component of the path is a file" do
filename = "#{__DIR__}/data/non_existing_file.txt"
ex = expect_raises(Errno, /Error determining size/) do
File.size("#{__FILE__}/")
end
ex.errno.should eq(Errno::ENOTDIR)
end
end

describe "delete" do
Expand Down
3 changes: 0 additions & 3 deletions src/crystal/system/dir.cr
View file
Open in desktop
Expand Up @@ -19,9 +19,6 @@ module Crystal::System::Dir
# Sets the current working directory of the application.
# def self.current=(path : String)

# Returns `true` if *path* exists and is a directory.
# def self.exists?(path : String) : Bool

# Creates a new directory at *path*. The UNIX-style directory mode *node*
# must be applied.
# def self.create(path : String, mode : Int32) : Nil
Expand Down
12 changes: 0 additions & 12 deletions src/crystal/system/unix/dir.cr
View file
Open in desktop
Expand Up @@ -48,18 +48,6 @@ module Crystal::System::Dir
path
end

def self.exists?(path : String) : Bool
if LibC.stat(path.check_no_null_byte, out stat) != 0
if Errno.value == Errno::ENOENT || Errno.value == Errno::ENOTDIR
return false
else
raise Errno.new("stat")
end
end

(stat.st_mode & LibC::S_IFMT) == LibC::S_IFDIR
end

def self.create(path : String, mode : Int32) : Nil
if LibC.mkdir(path.check_no_null_byte, mode) == -1
raise Errno.new("Unable to create directory '#{path}'")
Expand Down
48 changes: 13 additions & 35 deletions src/crystal/system/unix/file.cr
View file
Open in desktop
Expand Up @@ -70,28 +70,28 @@ module Crystal::System::File
tmpdir.rchop(::File::SEPARATOR)
end

def self.stat(path)
def self.stat?(path : String) : ::File::Stat?
if LibC.stat(path.check_no_null_byte, out stat) != 0
raise Errno.new("Unable to get stat for '#{path}'")
if {Errno::ENOENT, Errno::ENOTDIR}.includes? Errno.value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, File.file?("/path/to/file.cr/some/rubbish") would raise ENOTDIR because /path/to/file.cr exists. I've changed that in this PR to allow Dir.exists? to be implemented in terms of File.stat?. Dir does not raise on the above condition, which is specified in the specs. File.file? does.

Ruby implements it how i've implemented it here:

$ ruby -e 'p File.file?("/etc/fstab/foo")'
false
$ crystal eval 'p File.file?("/etc/fstab/foo")'
stat: Not a directory (Errno)
$ bin/crystal eval 'p File.file?("/etc/fstab/foo")'
false

return nil
else
raise Errno.new("Unable to get stat for '#{path}'")
end
end
::File::Stat.new(stat)
end

def self.lstat(path)
def self.lstat?(path : String) : ::File::Stat?
if LibC.lstat(path.check_no_null_byte, out stat) != 0
raise Errno.new("Unable to get lstat for '#{path}'")
if {Errno::ENOENT, Errno::ENOTDIR}.includes? Errno.value
return nil
else
raise Errno.new("Unable to get lstat for '#{path}'")
end
end
::File::Stat.new(stat)
end

def self.empty?(path)
begin
stat(path).size == 0
rescue Errno
raise Errno.new("Error determining size of '#{path}'")
end
end

def self.exists?(path)
accessible?(path, LibC::F_OK)
end
Expand All @@ -112,19 +112,8 @@ module Crystal::System::File
LibC.access(path.check_no_null_byte, flag) == 0
end

def self.file?(path) : Bool
if LibC.stat(path.check_no_null_byte, out stat) != 0
if Errno.value == Errno::ENOENT
return false
else
raise Errno.new("stat")
end
end
::File::Stat.new(stat).file?
end

def self.chown(path, uid : Int, gid : Int, follow_symlinks)
ret = if !follow_symlinks && symlink?(path)
ret = if !follow_symlinks && ::File.symlink?(path)
LibC.lchown(path, uid, gid)
else
LibC.chown(path, uid, gid)
Expand Down Expand Up @@ -163,17 +152,6 @@ module Crystal::System::File
ret
end

def self.symlink?(path)
if LibC.lstat(path.check_no_null_byte, out stat) != 0
if Errno.value == Errno::ENOENT
return false
else
raise Errno.new("stat")
end
end
(stat.st_mode & LibC::S_IFMT) == LibC::S_IFLNK
end

def self.rename(old_filename, new_filename)
code = LibC.rename(old_filename.check_no_null_byte, new_filename.check_no_null_byte)
if code != 0
Expand Down
6 changes: 5 additions & 1 deletion src/dir.cr
View file
Open in desktop
Expand Up @@ -193,7 +193,11 @@ class Dir

# Returns `true` if the given path exists and is a directory
def self.exists?(path) : Bool
Crystal::System::Dir.exists? path
if stat = File.stat?(path)
stat.directory?
else
false
end
end

# Returns `true` if the directory at *path* is empty, otherwise returns `false`.
Expand Down