JRuby

File.rename does not report correct error codes

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.1.6
  • Fix Version/s: JRuby 1.6RC1
  • Component/s: Core Classes/Modules
  • Labels:
    None
  • Number of attachments :
    0

Description

this happens to me under HPUX 11.11.

in jirb session:
bash-2.04# jirb
irb(main):001:0> File.rename '/tmp/foo', '/foo'
Errno::EACCES: Permission denied - Permission denied - /tmp/foo or /foo
from (irb):2

in irb session:
bash-2.04# irb
irb(main):001:0> File.rename '/tmp/foo', '/foo'
Errno::EXDEV: Cross-device link - /tmp/foo or /foo
from (irb):1:in `rename'
from (irb):1

tracing system calls, the underlying rename does report EXDEV

Note that FileUtils.mv relies on EXDEV: it first tries a rename and if the error is EXDEV, it does a copy.

Activity

Hide
Ittay Dror added a comment -

java is the standard java installation:
java version "1.6.0.02"
Java(TM) SE Runtime Environment (build 1.6.0.02-jinteg_26_sep_2008_17_44-b00)
Java HotSpot(TM) Server VM (build 1.6.0.02 jinteg:09.26.08-16:50 PA2.0 (aCC_AP), mixed mode)

Show
Ittay Dror added a comment - java is the standard java installation: java version "1.6.0.02" Java(TM) SE Runtime Environment (build 1.6.0.02-jinteg_26_sep_2008_17_44-b00) Java HotSpot(TM) Server VM (build 1.6.0.02 jinteg:09.26.08-16:50 PA2.0 (aCC_AP), mixed mode)
Hide
Tuomas Karkkainen added a comment -

java.io.FileSystem's rename(File file1, File file2) doesn't throw exceptions and only provides a true/false boolean. there seems to be no way to tell when an EXDEV occurs. the most frequent case of this happening is when /tmp is on a separate device. settings TMPDIR, TMP or TEMP helps in that case as a workaround,

Show
Tuomas Karkkainen added a comment - java.io.FileSystem's rename(File file1, File file2) doesn't throw exceptions and only provides a true/false boolean. there seems to be no way to tell when an EXDEV occurs. the most frequent case of this happening is when /tmp is on a separate device. settings TMPDIR, TMP or TEMP helps in that case as a workaround,
Hide
Justin Coyne added a comment -

This also affects version 1.3.1

Show
Justin Coyne added a comment - This also affects version 1.3.1
Hide
Charles Oliver Nutter added a comment -

I think our best bet for this may be to modify fileutils.rb to not rely on EXDEV. We are limited in how "POSIXy" our errors can be, and often just fake them. The lack of proper symlink support on the JVM makes it even harder, since we can't even tell whether something is a symlink without making a native call. We can use JNA/JFFI/FFI, but then we lose functionality in environments that disallow native calls.

Is there a change we could make to fileutils to handle this better?

Show
Charles Oliver Nutter added a comment - I think our best bet for this may be to modify fileutils.rb to not rely on EXDEV. We are limited in how "POSIXy" our errors can be, and often just fake them. The lack of proper symlink support on the JVM makes it even harder, since we can't even tell whether something is a symlink without making a native call. We can use JNA/JFFI/FFI, but then we lose functionality in environments that disallow native calls. Is there a change we could make to fileutils to handle this better?
Hide
Charles Oliver Nutter added a comment -

Ok, I see now it's not really symlinks, but just the way Java's file APIs report that a rename has failed. Tuomas said there seems to be no way to find out if it's this error...so maybe our hands are tied? Maybe a cleanup/hack for fileutils is the only way?

Show
Charles Oliver Nutter added a comment - Ok, I see now it's not really symlinks, but just the way Java's file APIs report that a rename has failed. Tuomas said there seems to be no way to find out if it's this error...so maybe our hands are tied? Maybe a cleanup/hack for fileutils is the only way?
Hide
Lenny Marks added a comment -

I ran into this via Rails/Paperclip upon trying to mv an uploaded file in /tmp to its permanent home in an NFS mounted FS. What about this hack as a workaround?

module FileUtils
  class << self
    alias_method :built_in_mv, :mv

    def mv(src, dest, options = {})
      puts "my mv"
      begin
        built_in_mv(src, dest, options)
      rescue Errno::EACCES
        cp(src, dest)
        rm(src)
      end
    end
  end 
end
Show
Lenny Marks added a comment - I ran into this via Rails/Paperclip upon trying to mv an uploaded file in /tmp to its permanent home in an NFS mounted FS. What about this hack as a workaround?
module FileUtils
  class << self
    alias_method :built_in_mv, :mv

    def mv(src, dest, options = {})
      puts "my mv"
      begin
        built_in_mv(src, dest, options)
      rescue Errno::EACCES
        cp(src, dest)
        rm(src)
      end
    end
  end 
end
Hide
Charles Oliver Nutter added a comment -

Lenny: I assume that fix works ok for you? If so, it may not be a bad option. The non-atomic nature of it might be a problem for someone, but it's better than not being able to do the mv at all, I think...

Show
Charles Oliver Nutter added a comment - Lenny: I assume that fix works ok for you? If so, it may not be a bad option. The non-atomic nature of it might be a problem for someone, but it's better than not being able to do the mv at all, I think...
Hide
Lenny Marks added a comment -

Actually below is the code I wound up with. The first version worked in jirb but was giving me NoSuchMethod mv errors starting up Rails. Personally I'm OK with the non-atomic nature of it since I expect that with a mv across file systems. Aside from not being able to catch the EXDEV error specifically, isn't this more or less what the MRI implementation does?

FileUtils.module_eval do
  class << self
    alias_method :built_in_mv, :mv

    def mv(src, dest, options = {})
      begin
        built_in_mv(src, dest, options)
      rescue Errno::EACCES
        cp(src, dest)
        rm(src)
      end
    end
  end
end
Show
Lenny Marks added a comment - Actually below is the code I wound up with. The first version worked in jirb but was giving me NoSuchMethod mv errors starting up Rails. Personally I'm OK with the non-atomic nature of it since I expect that with a mv across file systems. Aside from not being able to catch the EXDEV error specifically, isn't this more or less what the MRI implementation does?
FileUtils.module_eval do
  class << self
    alias_method :built_in_mv, :mv

    def mv(src, dest, options = {})
      begin
        built_in_mv(src, dest, options)
      rescue Errno::EACCES
        cp(src, dest)
        rm(src)
      end
    end
  end
end
Hide
Charles Oliver Nutter added a comment -

I like the fix but I think this is a bit risky to include in 1.5.1 since it changes the behavior of a standard library. But I think we could go ahead with landing this change on our fork of the stdlib for 1.6 (master). Marking for 1.6.

Show
Charles Oliver Nutter added a comment - I like the fix but I think this is a bit risky to include in 1.5.1 since it changes the behavior of a standard library. But I think we could go ahead with landing this change on our fork of the stdlib for 1.6 (master). Marking for 1.6.
Hide
Einar Boson added a comment - - edited

The original `mv` function can take an array as the `src` argument and will possibly already have succeeded in moving some of the files. I think that will end badly with this fix.
I also ran into this problem with paperclip and I am not sure about what to do. What I am doing for now is just overwriting the original method with my own fixed version in an initializer in my rails app:

    FileUtils.module_eval do
      class << self
        def mv(src, dest, options = {})
          fu_check_options options, FileUtils::OPT_TABLE['mv']
          fu_output_message "mv#{options[:force] ? ' -f' : ''} #{[src,dest].flatten.join ' '}" if options[:verbose]
          return if options[:noop]
          fu_each_src_dest(src, dest) do |s, d|
            destent = FileUtils::Entry_.new(d, nil, true)
            begin
              if destent.exist?
                if destent.directory?
                  raise Errno::EEXIST, dest
                else
                  destent.remove_file if rename_cannot_overwrite_file?
                end
              end
              begin
                File.rename s, d
              rescue Errno::EXDEV, Errno::EACCES 
                copy_entry s, d, true
                if options[:secure]
                  remove_entry_secure s, options[:force]
                else
                  remove_entry s, options[:force]
                end
              end
            rescue SystemCallError
              raise unless options[:force]
            end
          end
        end
      end
    end

the only difference being that I rescue the `EACCES` exception java seems to be raising and that I have to prefix the constants where I use them.
But this should really be fixed somewhere (in jruby's `File.rename` or ruby's `FileUtils.mv`, I don't know which is possible or more appropriate), I have a feeling I will forget to update this should it ever evolve.

Show
Einar Boson added a comment - - edited The original `mv` function can take an array as the `src` argument and will possibly already have succeeded in moving some of the files. I think that will end badly with this fix. I also ran into this problem with paperclip and I am not sure about what to do. What I am doing for now is just overwriting the original method with my own fixed version in an initializer in my rails app:
    FileUtils.module_eval do
      class << self
        def mv(src, dest, options = {})
          fu_check_options options, FileUtils::OPT_TABLE['mv']
          fu_output_message "mv#{options[:force] ? ' -f' : ''} #{[src,dest].flatten.join ' '}" if options[:verbose]
          return if options[:noop]
          fu_each_src_dest(src, dest) do |s, d|
            destent = FileUtils::Entry_.new(d, nil, true)
            begin
              if destent.exist?
                if destent.directory?
                  raise Errno::EEXIST, dest
                else
                  destent.remove_file if rename_cannot_overwrite_file?
                end
              end
              begin
                File.rename s, d
              rescue Errno::EXDEV, Errno::EACCES 
                copy_entry s, d, true
                if options[:secure]
                  remove_entry_secure s, options[:force]
                else
                  remove_entry s, options[:force]
                end
              end
            rescue SystemCallError
              raise unless options[:force]
            end
          end
        end
      end
    end
the only difference being that I rescue the `EACCES` exception java seems to be raising and that I have to prefix the constants where I use them. But this should really be fixed somewhere (in jruby's `File.rename` or ruby's `FileUtils.mv`, I don't know which is possible or more appropriate), I have a feeling I will forget to update this should it ever evolve.
Hide
Charles Oliver Nutter added a comment -

It's not possible (as far as I've found) to get our File.rename to report EXDEV, but I've pushed changes to our stdlib fileutils.rb (1.8 and 1.9) to rescue EACCES in addition to EXDEV, which should be a good enough change for most reported issues.

Change pushed in 71004ea.

Show
Charles Oliver Nutter added a comment - It's not possible (as far as I've found) to get our File.rename to report EXDEV, but I've pushed changes to our stdlib fileutils.rb (1.8 and 1.9) to rescue EACCES in addition to EXDEV, which should be a good enough change for most reported issues. Change pushed in 71004ea.

People

Vote (2)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: