[GR-59866] Add Dir.from_fd, Dir.fchdir and Dir#chdir methods

PullRequest: truffleruby/4420

CHANGELOG.md

@@ -37,6 +37,9 @@ Compatibility:
 * Add `Range#reverse_each` (#3681, @andrykonchin).
 * Emit a warning when `it` call without arguments is used in a block without parameters (#3681, @andrykonchin).
 * Add `rb_syserr_fail_str()` (#3732, @andrykonchin).
+* Add `Dir.for_fd` (#3681, @andrykonchin).
+* Add `Dir.fchdir` (#3681, @andrykonchin).
+* Add `Dir#chdir` (#3681, @andrykonchin).
 
 Performance:
 

spec/ruby/core/dir/chdir_spec.rb

@@ -124,3 +124,96 @@ def to_str;  DirSpecs.mock_dir; end
     Dir.pwd.should == @original
   end
 end
+
+ruby_version_is '3.3' do
+  describe "Dir#chdir" do
+    before :all do
+      DirSpecs.create_mock_dirs
+    end
+
+    after :all do
+      DirSpecs.delete_mock_dirs
+    end
+
+    before :each do
+      @original = Dir.pwd
+    end
+
+    after :each do
+      Dir.chdir(@original)
+    end
+
+    it "changes the current working directory to self" do
+      dir = Dir.new(DirSpecs.mock_dir)
+      dir.chdir
+      Dir.pwd.should == DirSpecs.mock_dir
+    ensure
+      dir.close
+    end
+
+    it "changes the current working directory to self for duration of the block when a block is given" do
+      dir = Dir.new(DirSpecs.mock_dir)
+      pwd_in_block = nil
+
+      dir.chdir { pwd_in_block = Dir.pwd }
+
+      pwd_in_block.should == DirSpecs.mock_dir
+      Dir.pwd.should == @original
+    ensure
+      dir.close
+    end
+
+    it "returns 0 when successfully changing directory" do
+      dir = Dir.new(DirSpecs.mock_dir)
+      dir.chdir.should == 0
+    ensure
+      dir.close
+    end
+
+    it "returns the value of the block when a block is given" do
+      dir = Dir.new(DirSpecs.mock_dir)
+      dir.chdir { :block_value }.should == :block_value
+    ensure
+      dir.close
+    end
+
+    it "raises an Errno::ENOENT if the original directory no longer exists" do
+      dir_name1 = tmp('/testdir1')
+      dir_name2 = tmp('/testdir2')
+      File.should_not.exist?(dir_name1)
+      File.should_not.exist?(dir_name2)
+      Dir.mkdir dir_name1
+      Dir.mkdir dir_name2
+
+      dir1 = Dir.new(dir_name1)
+
+      begin
+        -> {
+          dir1.chdir do
+            Dir.chdir(dir_name2) { Dir.unlink dir_name1 }
+          end
+        }.should raise_error(Errno::ENOENT)
+      ensure
+        Dir.unlink dir_name1 if File.exist?(dir_name1)
+        Dir.unlink dir_name2 if File.exist?(dir_name2)
+      end
+    ensure
+      dir1.close
+    end
+
+    it "always returns to the original directory when given a block" do
+      dir = Dir.new(DirSpecs.mock_dir)
+
+      begin
+        dir.chdir do
+          raise StandardError, "something bad happened"
+        end
+      rescue StandardError
+      end
+
+      Dir.pwd.should == @original
+    ensure
+      dir.close
+    end
+  end
+end

spec/ruby/core/dir/close_spec.rb

@@ -12,8 +12,28 @@
   it "does not raise an IOError even if the Dir instance is closed" do
     dir = Dir.open DirSpecs.mock_dir
     dir.close
-    -> {
-      dir.close
-    }.should_not raise_error(IOError)
+    dir.close
+
+    -> { dir.fileno }.should raise_error(IOError, /closed directory/)
+  end
+
+  it "returns nil" do
+    dir = Dir.open DirSpecs.mock_dir
+    dir.close.should == nil
+  end
+
+  ruby_version_is '3.3' do
+    guard -> { Dir.respond_to? :for_fd } do
+      it "does not raise an error even if the file descriptor is closed with another Dir instance" do
+        dir = Dir.open DirSpecs.mock_dir
+        dir_new = Dir.for_fd(dir.fileno)
+
+        dir.close
+        dir_new.close
+
+        -> { dir.fileno }.should raise_error(IOError, /closed directory/)
+        -> { dir_new.fileno }.should raise_error(IOError, /closed directory/)
+      end
+    end
   end
 end

spec/ruby/core/dir/fchdir_spec.rb

@@ -13,46 +13,51 @@
       end
 
       before :each do
-        @dirs = [Dir.new('.')]
-        @original = @dirs.first.fileno
+        @original = Dir.pwd
       end
 
       after :each do
-        Dir.fchdir(@original)
-        @dirs.each(&:close)
+        Dir.chdir(@original)
       end
 
-      it "changes to the specified directory" do
+      it "changes the current working directory to the directory specified by the integer file descriptor" do
         dir = Dir.new(DirSpecs.mock_dir)
-        @dirs << dir
         Dir.fchdir dir.fileno
         Dir.pwd.should == DirSpecs.mock_dir
+      ensure
+        dir.close
       end
 
       it "returns 0 when successfully changing directory" do
-        Dir.fchdir(@original).should == 0
+        dir = Dir.new(DirSpecs.mock_dir)
+        Dir.fchdir(dir.fileno).should == 0
+      ensure
+        dir.close
       end
 
       it "returns the value of the block when a block is given" do
-        Dir.fchdir(@original) { :block_value }.should == :block_value
+        dir = Dir.new(DirSpecs.mock_dir)
+        Dir.fchdir(dir.fileno) { :block_value }.should == :block_value
+      ensure
+        dir.close
       end
 
       it "changes to the specified directory for the duration of the block" do
-        pwd = Dir.pwd
         dir = Dir.new(DirSpecs.mock_dir)
-        @dirs << dir
         Dir.fchdir(dir.fileno) { Dir.pwd }.should == DirSpecs.mock_dir
-        Dir.pwd.should == pwd
+        Dir.pwd.should == @original
+      ensure
+        dir.close
       end
 
       it "raises a SystemCallError if the file descriptor given is not valid" do
-        -> { Dir.fchdir(-1) }.should raise_error(SystemCallError)
-        -> { Dir.fchdir(-1) { } }.should raise_error(SystemCallError)
+        -> { Dir.fchdir(-1) }.should raise_error(SystemCallError, "Bad file descriptor - fchdir")
+        -> { Dir.fchdir(-1) { } }.should raise_error(SystemCallError, "Bad file descriptor - fchdir")
       end
 
       it "raises a SystemCallError if the file descriptor given is not for a directory" do
-        -> { Dir.fchdir $stdout.fileno }.should raise_error(SystemCallError)
-        -> { Dir.fchdir($stdout.fileno) { } }.should raise_error(SystemCallError)
+        -> { Dir.fchdir $stdout.fileno }.should raise_error(SystemCallError, /(Not a directory|Invalid argument) - fchdir/)
+        -> { Dir.fchdir($stdout.fileno) { } }.should raise_error(SystemCallError, /(Not a directory|Invalid argument) - fchdir/)
       end
     end
   end

spec/ruby/core/dir/for_fd_spec.rb

@@ -0,0 +1,77 @@
+require_relative '../../spec_helper'
+require_relative 'fixtures/common'
+
+ruby_version_is '3.3' do
+  guard -> { Dir.respond_to? :for_fd } do
+    describe "Dir.for_fd" do
+      before :all do
+        DirSpecs.create_mock_dirs
+      end
+
+      after :all do
+        DirSpecs.delete_mock_dirs
+      end
+
+      before :each do
+        @original = Dir.pwd
+      end
+
+      after :each do
+        Dir.chdir(@original)
+      end
+
+      it "returns a new Dir object representing the directory specified by the given integer directory file descriptor" do
+        dir = Dir.new(DirSpecs.mock_dir)
+        dir_new = Dir.for_fd(dir.fileno)
+
+        dir_new.should.instance_of?(Dir)
+        dir_new.children.should == dir.children
+        dir_new.fileno.should == dir.fileno
+      ensure
+        dir.close
+      end
+
+      it "returns a new Dir object without associated path" do
+        dir = Dir.new(DirSpecs.mock_dir)
+        dir_new = Dir.for_fd(dir.fileno)
+
+        dir_new.path.should == nil
+      ensure
+        dir.close
+      end
+
+      it "calls #to_int to convert a value to an Integer" do
+        dir = Dir.new(DirSpecs.mock_dir)
+        obj = Object.new
+        obj.singleton_class.define_method(:to_int) { dir.fileno }
+
+        dir_new = Dir.for_fd(obj)
+        dir_new.fileno.should == dir.fileno
+      ensure
+        dir.close
+      end
+
+      it "raises TypeError when value cannot be converted to Integer" do
+        -> {
+          Dir.for_fd(nil)
+        }.should raise_error(TypeError, "no implicit conversion from nil to integer")
+      end
+
+      it "raises a SystemCallError if the file descriptor given is not valid" do
+        -> { Dir.for_fd(-1) }.should raise_error(SystemCallError, "Bad file descriptor - fdopendir")
+      end
+
+      it "raises a SystemCallError if the file descriptor given is not for a directory" do
+        -> { Dir.for_fd $stdout.fileno }.should raise_error(SystemCallError, "Not a directory - fdopendir")
+      end
+    end
+  end
+
+  guard_not -> { Dir.respond_to? :for_fd } do
+    describe "Dir.for_fd" do
+      it "raises NotImplementedError" do
+        -> { Dir.for_fd 1 }.should raise_error(NotImplementedError)
+      end
+    end
+  end
+end

src/main/ruby/truffleruby/core/dir.rb

@@ -62,6 +62,16 @@ def initialize(path, options = undefined)
     @ptr.null? ? nil : self
   end
 
+  private def initialize_from_file_descriptor(fd)
+    @path = nil
+    @encoding = Encoding.filesystem
+    @ptr = Truffle::POSIX.fdopendir(fd)
+
+    if @ptr.null?
+      Errno.handle('fdopendir')
+    end
+  end
+
   private def ensure_open
     raise IOError, 'closed directory' if closed?
   end
@@ -101,15 +111,18 @@ def read
     Truffle::DirOperations.readdir_name(self)
   end
 
+  def chdir(&block)
+    Dir.fchdir(fileno, &block)
+  end
+
   def close
     unless closed?
-      ret = Truffle::POSIX.closedir(@ptr)
-      Errno.handle if ret == -1
+      Truffle::POSIX.closedir(@ptr)
       @ptr = nil
     end
   end
 
-  def closed?
+  private def closed?
     Primitive.nil? @ptr
   end
 
@@ -315,6 +328,12 @@ def foreach(path, **options)
       nil
     end
 
+    def for_fd(fd)
+      dir = Dir.allocate
+      dir.send(:initialize_from_file_descriptor, Primitive.rb_num2int(fd))
+      dir
+    end
+
     def chdir(path = ENV['HOME'])
       path = Truffle::Type.coerce_to_path path
       path = path.dup.force_encoding(Encoding::LOCALE) if path.encoding == Encoding::BINARY
@@ -340,6 +359,33 @@ def chdir(path = ENV['HOME'])
       end
     end
 
+    def fchdir(fd)
+      fd = Primitive.rb_num2int(fd)
+
+      if block_given?
+        original_path = getwd
+        original_dir = Dir.new(original_path)
+
+        ret = Truffle::POSIX.fchdir fd
+        Errno.handle('fchdir') if ret != 0
+        Primitive.dir_set_truffle_working_directory(getwd)
+
+        begin
+          yield
+        ensure
+          Primitive.dir_set_truffle_working_directory(original_path)
+          ret = Truffle::POSIX.fchdir original_dir.fileno
+          Errno.handle('fchdir') if ret != 0
+          original_dir.close
+        end
+      else
+        ret = Truffle::POSIX.fchdir fd
+        Errno.handle('fchdir') if ret != 0
+        Primitive.dir_set_truffle_working_directory(getwd)
+        ret
+      end
+    end
+
     def mkdir(path, mode = 0777)
       path = Truffle::Type.coerce_to_path(path)
       if mode

src/main/ruby/truffleruby/core/posix.rb

@@ -185,9 +185,11 @@ def self.attach_function_eagerly(native_name, argument_types, return_type,
   attach_function :dirfd, [:pointer], :int
   attach_function :dup, [:int], :int
   attach_function :dup2, [:int, :int], :int
+  attach_function :fchdir, [:int], :int
   attach_function :fchmod, [:int, :mode_t], :int
   attach_function :fchown, [:int, :uid_t, :gid_t], :int
   attach_function :fcntl, [:int, :int, varargs(:int)], :int
+  attach_function :fdopendir, [:int], :pointer
   attach_function :flock, [:int, :int], :int, LIBC, true
   attach_function :truffleposix_fstat, [:int, :pointer], :int, LIBTRUFFLEPOSIX
   attach_function :truffleposix_fstat_mode, [:int], :mode_t, LIBTRUFFLEPOSIX

test/mri/excludes/TestDir.rb

@@ -6,5 +6,5 @@
 exclude :test_glob_gc_for_fd, "Expected [] to not be empty."
 exclude :test_glob_ignore_casefold_invalid_encoding, "Errno::EILSEQ: Invalid or incomplete multibyte or wide character - /private/var/folders/gr/3kff5w4s7779h6ycnt4gxfgm0000gn/T/__test_dir__20240910-84824-6ncred/�a123" if RUBY_PLATFORM.include?('darwin')
 exclude :test_glob_too_may_open_files, "Errno::EMFILE expected but nothing was raised."
-exclude :test_instance_chdir, "NoMethodError: undefined method `chdir' for #<Dir:/b/b/e/.tmp/__test_dir__20241021-77734-dc54dg>"
+exclude :test_instance_chdir, "expected: /conflicting chdir during another chdir block/"
 exclude :test_unknown_keywords, "ArgumentError expected but nothing was raised."