Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

fix fallback when symlink in AbstractAdapter#link_or_copy_file fails #2540

Conversation

mraidel
Copy link
Contributor

@mraidel mraidel commented Jan 31, 2018

Paperclip::AbstractAdapter provides the link_or_copy_file method which first tries to do a symlink. When the symlinking raises an exception it is rescued and it tries again with copy. The problem is that the failed symlink leaves the tempfile instance in a bad state and you cannot read from it without reopening it.

Current behavior with a file which cannot be linked (for example because it is on another filesystem):

File.open("file_which_cannot_be_linked", "w") { |f| f.print("body") }

Paperclip.io_adapters.for(File.open("file_which_cannot_be_linked")).read
[paperclip] Trying to link file_which_cannot_be_linked to /tmp/d783e2fb96492f5a9f5e9632d2987a1120180131-3949-1vavjso                                                                                                
[paperclip] Link failed with Invalid cross-device link @ rb_file_s_link - (file_which_cannot_be_linked, /tmp/d783e2fb96492f5a9f5e9632d2987a1120180131-3949-1vavjso); copying link file_which_cannot_be_linked to /tm
p/d783e2fb96492f5a9f5e9632d2987a1120180131-3949-1vavjso    
Command :: file -b --mime 'file_which_cannot_be_linked'                              
=> ""

Behavior after fix:

Paperclip.io_adapters.for(File.open("file_which_cannot_be_linked")).read                                                                                                                  
[paperclip] Trying to link file_which_cannot_be_linked to /tmp/d783e2fb96492f5a9f5e9632d2987a1120180131-3939-1cp0zud                                                                                              
[paperclip] Link failed with Invalid cross-device link; copying link file_which_cannot_be_linked to /tmp/d783e2fb96492f5a9f5e9632d2987a1120180131-3939-1cp0zud                                         
Command :: file -b --mime 'file_which_cannot_be_linked'                                                                                                                                                             
=> "body"                                                                                                                                                                                        

begin
FileUtils.ln(src, dest, force: true) # overwrite existing
rescue Errno::EXDEV, Errno::EPERM, Errno::ENOENT, Errno::EEXIST => e
Paperclip.log("Link failed with #{e.message}; copying link #{src} to #{dest}")

Choose a reason for hiding this comment

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

Line is too long. [86/80]

@mraidel mraidel force-pushed the io-adapter-link-or-copy-file-with-file branch from ac92b5c to d112813 Compare January 31, 2018 18:15
FileUtils.ln(src, dest, force: true) # overwrite existing
rescue Errno::EXDEV, Errno::EPERM, Errno::ENOENT, Errno::EEXIST => e
Paperclip.log(
"Link failed with #{e.message}; copying link #{src} to #{dest}"

Choose a reason for hiding this comment

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

Put a comma after the last parameter of a multiline method call.

@mraidel mraidel force-pushed the io-adapter-link-or-copy-file-with-file branch from d112813 to 8e77d45 Compare January 31, 2018 18:17
@garettarrowood
Copy link

Thank you for diagnosing this issue and opening this solution! Very appreciated @mraidel !

@mike-burns mike-burns added the Bug label Mar 2, 2018
sidraval pushed a commit that referenced this pull request Mar 2, 2018
@sidraval
Copy link
Contributor

sidraval commented Mar 2, 2018

Thanks @mraidel ! See #2567

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants