Suggest to prioritize supplemental_path before ENV path

[johnnyshields]

Apologies in advance if I'm missing something there...

In command_line.rb, line 12, currently this:

@supplemental_environment['PATH'] = [ENV['PATH'], *supplemental_path].join(File::PATH_SEPARATOR)

I propose to switch the order of supplemental_path and ENV path, like so:

@supplemental_environment['PATH'] = [*supplemental_path, ENV['PATH']].join(File::PATH_SEPARATOR)

The reason for this change is to more closely mimic how Window/Linux/Mac cmd shells behave, i.e. your CommandLine.path takes first priority over your env path.

(The exact problem I had was C:\Windows\System32\convert.exe was trumping C:\Program Files\ImageMagick\convert.exe when running paperclip, and seems there's no way currently to force Cocaine to look somewhere other than system path at a higher priority)

Thanks for the great gem.

[johnnyshields]

Monkey-patch to work around this issue (for Rails, put in config/initializers/cocaine_monkey_patch.rb)

# Monkey patch for Cocaine 0.4.2 to use supplemental path at a
# higher priority than ENV path. Specifically, Paperclip has
# a conflict with convert.exe
module Cocaine
  class CommandLine
    class << self
      def path=(supplemental_path)
        @supplemental_path = supplemental_path
        @supplemental_environment ||= {}
        @supplemental_environment['PATH'] = [*supplemental_path, ENV['PATH']].join(File::PATH_SEPARATOR)
      end
    end
  end
end

[mike-burns]

This makes sense, @johnnyshields . Can you submit a pull request with a test, please?

[jyurek]

Sorry I hadn't gotten to this, but this is enough of a bug for us to fix it. I just pushed a fix to master. Thanks for reporting, @johnnyshields!

[johnnyshields]

Great, thanks!

She don't lie, she don't lie, she don't lie...

[johnnyshields]

@jyurek I'd recommend to update the gem dependency for Paperclip to Cocaine 0.5.0 when convenient as this issue affects many Paperclip users.

[nengine]

I did the monkeypatch as advised by @johnnyshields but still getting error:

undefined method `exitstatus' for nil:NilClass

runcocaine (0.5.1) lib/cocaine/command_line.rb

   81       rescue Errno::ENOENT
   82         raise Cocaine::CommandNotFoundError
   83       ensure
   84         @exit_status = $?.exitstatus
   85       end

[johnnyshields]

@neuralnw this has been merged into master for over 7 months... no monkey patching should be needed

[nengine]

@johnnyshields OK. thanks. But then, for some reason it is not working for windows 7, Jruby 1.7.4. I can see that it runs but still giving error.

line = Cocaine::CommandLine.new("echo hello world")
puts line.command # => "echo hello 'world'" 
puts line.run # => "hello world\n" 

C:\Users\nash>jruby -S coc.rb
echo hello world
file:/C:/Users/nash/torquebox-2.3.2/jruby/lib/jruby.jar!/jruby/kernel19/process.
rb:4 warning: unsupported spawn option: out
hello world
NoMethodError: undefined method `exitstatus' for nil:NilClass
run at C:/Users/nash/torquebox-2.3.2/jruby/lib/ruby/gems/shared/gems/cocain
e-0.5.1/lib/cocaine/command_line.rb:84
(root) at coc.rb:5

[johnnyshields]

haven't tested on JRuby. Working fine for me on MRI 1.9.3 on Windows 7

[johnnyshields]

The "$?" is a global environment variable which points to the last child process to run. (see http://jimneath.org/2010/01/04/cryptic-ruby-global-variables-and-their-meanings.html) I'm not familiar with JRuby, but it's conceivable that this would have a different implementation in Java versus C.

[nengine]

working fine with MRI 1.9.3 on Windows 7 too. Not sure how to fix this work for JRuby.

[johnnyshields]

Maybe you can extract this into a test case which passes on MRI but fails on JRuby, then raise it with the JRuby team?

[jyurek]

The problem is this line:

rb:4 warning: unsupported spawn option: out

JRuby does not support the use of the ProcessRunner or the PosixRunner as a result. It can't get the output of the spawned process. However, you can continue to use this using JRuby with the PopenRunner which was recently added. This is currently only in master and is not in a released gem.

If anyone would like to submit a PR about disallowing the above runners while running in JRuby, that would be nice, but we'll get to it eventually. Once we do, I'll release a new gem.

[nengine]

@jyurek I copied the popen_runner.rb and modified runners.rb to include this file in C:\torquebox-2.3.2\jruby\lib\ruby\gems\shared\gems\cocaine-0.5.1\lib\cocaine\command_line. but still getting the same message!

[jyurek]

Wow, if popen also doesn't work with JRuby I'm going to be mighty upset. I'll have to run some tests.

[nengine]

@johnnyshields @jyurek any way to monkey_patch so that this will work for now?

[johnnyshields]

@neuralnw I have no experience with JRuby, will defer to @jyurek