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

Devkit requirement undocumented #1543

Closed
ezuhaib opened this issue May 8, 2014 · 7 comments
Closed

Devkit requirement undocumented #1543

ezuhaib opened this issue May 8, 2014 · 7 comments

Comments

@ezuhaib
Copy link
Contributor

ezuhaib commented May 8, 2014

I on Windows 7 and hadn't added my devkit to system PATH because the path is automagically enhanced when building gems. But lately I discovered that paperclip needs devkit binaries in order to work. Without devkit, I was getting this error while trying to upload an image file:

has an extension that does not match its contents

this is probably due to:
https://github.com/thoughtbot/paperclip/blob/master/lib/paperclip/media_type_spoof_detector.rb#L63

please do either of these:

  1. Document devkit requirement, and it's addition to PATH, in readme.md in the Installation section.
  2. Return better error if a required binary from devkit is not found, rather than the above misleading one.
@sikachu
Copy link
Contributor

sikachu commented May 9, 2014

Would you mind submitting a PR for both of them? The README update would be a good start, but I also like the idea of better error message.

Thank you!

@sikachu sikachu modified the milestone: v3.1.4 May 9, 2014
@sikachu sikachu added this to the v4.1.2 milestone May 9, 2014
@jonforums
Copy link

What other alternatives have you considered for windows users?

The DevKit was never meant to become a runtime requirement for MRI on windows. It's primary purpose is to enable building native gems on windows for RubyInstaller users. As such, when it is properly installed, we built it so that a RubyGems plugin brings the toolchain on and off PATH during the gem build process. The DevKit artifacts never permanently reside on your PATH potentially causing other system conflicts and odd behaviors. Minimally scoped invasion.

Frankly, this is a terrible precedent you are considering for paperclip. You're essentially saying that in order to use the paperclip gem on windows, the user must ensure they have some minimal unix-like runtime environment. It's not enough to have installed MRI or another Ruby, but now you require a user to properly hack (with no unintended side-effects) their windows system so it looks more like unix to provide just file. Worse, paperclip appears to be a pure-ruby gem and doesn't need a toolchain to be installed.

Ruby can be a decent multi-platform environment, but decisions like this lead down the path to turning Ruby into a unix-only environment. If other gems like oj, nokogiri, sqlite3, and others can make the tradeoffs to stay multi-platform, why can't paperclip?

That said, I understand tradeoffs have to be made. Multi-platform is hard, but creating a hard runtime dependency on the DevKit (or ensuring file.exe and its deps are on PATH) seems like the wrong solution.

I'd like to see you investigate a few more alternatives. For example, are there ways of gracefully degrading capabilities so that paperclip still functions "well enough" on windows without requiring unix emulation? Anything in stdlib's io/file routines to help with a "good enough" fallback on windows for spoof detection? I'm guessing not or you already would have implemented it 😸

@sikachu
Copy link
Contributor

sikachu commented May 9, 2014

@jonforums please don't get us wrong. We always aim for being multi-platform compatible. However, because of the security concern, we overlooked that the change broke Windows compatibility. "Multi-platform is hard" is never, ever, an excuse for us to break the multi-platform compatibility.

There's some suggestion on #1523 that might be interesting to see if it could be a good platform-independent to file command. However, I need more time to look into that.

This open-source project, like another open-source project, have limited resource. For me, this is the first time in months that I can work on Paperclip. We'd love if you could help us finding an alternative, or helping us bring back the compatibility for Windows user.

@jonforums
Copy link

The mimemagic gem does look like a good alternative especially if it remains actively maintained. Being a pure-ruby gem is good for MRI on Windows and JRuby.

I wonder if any of the main paperclip usage scenarios are ever part of hotspot paths so that hypothetical mimemagic vs. shell out and file speed differences, if they exist, become a real issue. If not, mimemagic looks better and better.

@jonforums
Copy link

Using a timer utility for windows from => https://github.com/thecodeshop/w32time

C:\Users\Jon\Downloads\temp>ruby --version
ruby 2.1.2p97 (2014-05-11 revision 45908) [i386-mingw32]

C:\Users\Jon\Downloads\temp>gem li mimemagic

*** LOCAL GEMS ***

mimemagic (0.2.1)

C:\Users\Jon\Downloads\temp>file --version
file-5.18
magic file from /usr/share/misc/magic

C:\Users\Jon\Downloads\temp>timer ruby -rmimemagic -e "puts MimeMagic.by_magic(File.open('cgit-0.10.1.tar.xz'))"
application/x-xz
real    0.156
system  0.046
user    0.062

C:\Users\Jon\Downloads\temp>timer file -b --mime-type cgit-0.10.1.tar.xz
application/x-xz
real    0.062
system  0.000
user    0.000

C:\Users\Jon\Downloads\temp>timer ruby -rmimemagic -e "puts MimeMagic.by_magic(File.open('le1.jpg_original'))"
image/jpeg
real    0.093
system  0.046
user    0.046

C:\Users\Jon\Downloads\temp>timer file -b --mime-type le1.jpg_original
image/jpeg
real    0.046
system  0.015
user    0.000

To jumpstart a quick usage test, how about tweaking paperclip here with a bit of require 'mimemagic' followed by MimeMagic.by_magic(...)?

@maclover7
Copy link
Contributor

@jferris @jyurek Please close issue, problem appears to be solved.

@tute
Copy link
Contributor

tute commented May 9, 2015

Fixed in attached PR. Thank you all!

@tute tute closed this as completed May 9, 2015
jthurn24 added a commit to jthurn24/ppclip-RoR that referenced this issue Aug 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants