Project

General

Profile

Actions

Defect #19091

closed

SCM integration broken with JRuby (partial fix)

Added by Matthew Bloch about 10 years ago. Updated almost 10 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
SCM
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Wont fix
Affected version:

Description

When running Redmine (2.6.1) under JRuby (1.7.19), the SCM integration is broken due to a long-standing problem with JRuby and IO.popen's "r+" mode (described at https://github.com/jruby/jruby/issues/779).

It shows up in the front-end as a 500 error and the external command reporting "Bad file descriptor".

As far as I understand it this can't be fixed without some changes to Java, but we can avoid triggering the bug by not using the r+ mode unnecessarily.

In lib/redmine/scm/adapters/abstract_adapter.rb:254 in the "shellout" method we have this code:

mode = "r+" 
IO.popen(cmd, mode) do |io|
io.set_encoding("ASCII-8BIT") if io.respond_to?(:set_encoding)
io.close_write unless options[:write_stdin]
block.call(io) if block_given?
end

but we can avoid the JRuby bug in many situations by changing this to:

mode = options[:write_stdin] ? "r+" : "r" 
IO.popen(cmd, mode) do |io|
io.set_encoding("ASCII-8BIT") if io.respond_to?(:set_encoding)
block.call(io) if block_given?
end

i.e. not using "r+" mode when we don't need to - I think this code is equivalent.

As far as I can tell only GitAdapter.revisions needs the :write_stdin option, and we could eliminate that one usage by pushing all the revisions onto the command line instead.

I hope that makes sense.

Actions #1

Updated by Toshi MARUYAMA about 10 years ago

Tests fails on trunk r14010.

$ ruby test/unit/lib/redmine/scm/adapters/mercurial_adapter_test.rb -n test_path_space
Run options: -n test_path_space --seed 9622

# Running:

E

Finished in 0.124227s, 8.0498 runs/s, 16.0996 assertions/s.

  1) Error:
MercurialAdapterTest#test_path_space:
Redmine::Scm::Adapters::CommandFailed: closing non-duplex IO for writing
    lib/redmine/scm/adapters/abstract_adapter.rb:270:in `rescue in shellout'
    lib/redmine/scm/adapters/abstract_adapter.rb:249:in `shellout'
    lib/redmine/scm/adapters/mercurial_adapter.rb:64:in `hgversion_from_command_line'
    lib/redmine/scm/adapters/mercurial_adapter.rb:57:in `hgversion'
    lib/redmine/scm/adapters/mercurial_adapter.rb:46:in `client_version'
    lib/redmine/scm/adapters/abstract_adapter.rb:60:in `client_version_above?'
    lib/redmine/scm/adapters/mercurial_adapter.rb:50:in `client_available'
    test/unit/lib/redmine/scm/adapters/mercurial_adapter_test.rb:33:in `setup'

1 runs, 2 assertions, 0 failures, 1 errors, 0 skips

Actions #2

Updated by Toshi MARUYAMA about 10 years ago

Previous note test on ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-linux]

Actions #3

Updated by Toshi MARUYAMA about 10 years ago

JRuby team says this issue will fix in JRuby 9000.
https://github.com/jruby/jruby/issues/779#issuecomment-87487830

Actions #4

Updated by Matthew Bloch about 10 years ago

  • Status changed from New to Resolved

I've not tested it but if the POSIX implementation has changed in JRuby, this bug isn't going to be valid or useful. I'll reopen if it proves a problem later.

Actions #5

Updated by Toshi MARUYAMA almost 10 years ago

  • Status changed from Resolved to Closed
  • Resolution set to Wont fix
Actions

Also available in: Atom PDF