Issue #17296 has been updated by mame (Yusuke Endoh). Status changed from Assigned to Feedback It was discussed at the dev meeting. `File.chmod` and `FileUtils.chmod` have an incompatibility in their handling of symbolic links: `File.chmod` changes the permissions on the target of the symbolic link, while `FileUtils.chmod` changes the permissions on the symbolic link itself in environments with `lchmod `itself, and no-op in environments without `lchmod`. Therefore, it seems better to have `File.chmod` support `"+x"`, etc., rather than using `FileUtils.chmod`. ---------------------------------------- Feature #17296: Feature: Pathname#chmod use FileUtils.chmod instead of File https://bugs.ruby-lang.org/issues/17296#change-110045 * Author: schneems (Richard Schneeman) * Status: Feedback * Assignee: akr (Akira Tanaka) ---------------------------------------- The `FileUtils.chmod` provides the same numerical interface as `File.chmod` and it also includes a "symbolic mode" interface. With this patch you'll be able to run this code: ```ruby Pathname.new("bin/compile").chmod("+x") ``` I believe that this is backwards compatible with the existing implementation and all changes are an extension. The only difference between File.chmod and FileUtils.chmod I could find is they have different return values and the previous implementation of `Pathname#chmod` returned the result of the `File.chmod` call. From the docs `File.chmod` returns the number of files modified, since we're only ever able to pass in a maximum of one file through this interface, the return value will always be a `1` or an exception if the file does not exist. I checked and the exceptions when the file does not exist match: ``` irb(main):004:0> File.chmod(0444, "doesnotexist.txt") Traceback (most recent call last): 6: from /Users/rschneeman/.rubies/ruby-2.7.2/bin/irb:23:in `<main>' 5: from /Users/rschneeman/.rubies/ruby-2.7.2/bin/irb:23:in `load' 4: from /Users/rschneeman/.rubies/ruby-2.7.2/lib/ruby/gems/2.7.0/gems/irb-1.2.6/exe/irb:11:in `<top (required)>' 3: from (irb):3 2: from (irb):4:in `rescue in irb_binding' 1: from (irb):4:in `chmod' Errno::ENOENT (No such file or directory @ apply2files - doesnotexist.txt) irb(main):005:0> FileUtils.chmod(0444, "doesnotexist.txt") Traceback (most recent call last): 10: from /Users/rschneeman/.rubies/ruby-2.7.2/bin/irb:23:in `<main>' 9: from /Users/rschneeman/.rubies/ruby-2.7.2/bin/irb:23:in `load' 8: from /Users/rschneeman/.rubies/ruby-2.7.2/lib/ruby/gems/2.7.0/gems/irb-1.2.6/exe/irb:11:in `<top (required)>' 7: from (irb):4 6: from (irb):5:in `rescue in irb_binding' 5: from /Users/rschneeman/.rubies/ruby-2.7.2/lib/ruby/2.7.0/fileutils.rb:1016:in `chmod' 4: from /Users/rschneeman/.rubies/ruby-2.7.2/lib/ruby/2.7.0/fileutils.rb:1016:in `each' 3: from /Users/rschneeman/.rubies/ruby-2.7.2/lib/ruby/2.7.0/fileutils.rb:1017:in `block in chmod' 2: from /Users/rschneeman/.rubies/ruby-2.7.2/lib/ruby/2.7.0/fileutils.rb:1346:in `chmod' 1: from /Users/rschneeman/.rubies/ruby-2.7.2/lib/ruby/2.7.0/fileutils.rb:1346:in `chmod' Errno::ENOENT (No such file or directory @ apply2files - doesnotexist.txt) ``` If you're open to changing the interface of the return value my preference would be to return `self` from this method so that it can be chained. Otherwise this current patch is a smaller change. Diff: ``` $ git diff master diff --git a/ext/pathname/lib/pathname.rb b/ext/pathname/lib/pathname.rb index e6fb90277d..cb6e32d9ac 100644 --- a/ext/pathname/lib/pathname.rb +++ b/ext/pathname/lib/pathname.rb @@ -585,6 +585,15 @@ def mkpath nil end + # Changes file permissions. + # + # See FileUtils.chmod + def chmod(mode) + require 'fileutils' + FileUtils.chmod(mode, self) + return 1 + end + # Recursively deletes a directory, including all directories beneath it. # # See FileUtils.rm_r diff --git a/ext/pathname/pathname.c b/ext/pathname/pathname.c index f71cec1b25..6778d4f102 100644 --- a/ext/pathname/pathname.c +++ b/ext/pathname/pathname.c @@ -12,7 +12,6 @@ static ID id_binwrite; static ID id_birthtime; static ID id_blockdev_p; static ID id_chardev_p; -static ID id_chmod; static ID id_chown; static ID id_ctime; static ID id_directory_p; @@ -552,20 +551,6 @@ path_mtime(VALUE self) return rb_funcall(rb_cFile, id_mtime, 1, get_strpath(self)); } -/* - * call-seq: - * pathname.chmod(mode_int) -> integer - * - * Changes file permissions. - * - * See File.chmod. - */ -static VALUE -path_chmod(VALUE self, VALUE mode) -{ - return rb_funcall(rb_cFile, id_chmod, 2, mode, get_strpath(self)); -} - /* * call-seq: * pathname.lchmod(mode_int) -> integer @@ -1448,7 +1433,6 @@ path_f_pathname(VALUE self, VALUE str) * - #birthtime * - #ctime * - #mtime - * - #chmod(mode) * - #lchmod(mode) * - #chown(owner, group) * - #lchown(owner, group) @@ -1495,6 +1479,7 @@ path_f_pathname(VALUE self, VALUE str) * === Utilities * * These methods are a mixture of Find, FileUtils, and others: + * - #chmod(mode) * - #find(&block) * - #mkpath * - #rmtree @@ -1542,7 +1527,6 @@ Init_pathname(void) rb_define_method(rb_cPathname, "birthtime", path_birthtime, 0); rb_define_method(rb_cPathname, "ctime", path_ctime, 0); rb_define_method(rb_cPathname, "mtime", path_mtime, 0); - rb_define_method(rb_cPathname, "chmod", path_chmod, 1); rb_define_method(rb_cPathname, "lchmod", path_lchmod, 1); rb_define_method(rb_cPathname, "chown", path_chown, 2); rb_define_method(rb_cPathname, "lchown", path_lchown, 2); @@ -1618,7 +1602,6 @@ InitVM_pathname(void) id_birthtime = rb_intern("birthtime"); id_blockdev_p = rb_intern("blockdev?"); id_chardev_p = rb_intern("chardev?"); - id_chmod = rb_intern("chmod"); id_chown = rb_intern("chown"); id_ctime = rb_intern("ctime"); id_directory_p = rb_intern("directory?"); diff --git a/test/pathname/test_pathname.rb b/test/pathname/test_pathname.rb index 43cef4849f..5673691231 100644 --- a/test/pathname/test_pathname.rb +++ b/test/pathname/test_pathname.rb @@ -823,6 +823,11 @@ def test_chmod path.chmod(0444) assert_equal(0444, path.stat.mode & 0777) path.chmod(old) + + skip "Windows has different symbolic mode" if /mswin|mingw/ =~ RUBY_PLATFORM + path.chmod("u=wrx,g=rx,o=x") + assert_equal(0751, path.stat.mode & 0777) + path.chmod(old) } end ``` Github link: https://github.com/ruby/ruby/pull/3708 -- https://bugs.ruby-lang.org/