Defect #19091
closedSCM integration broken with JRuby (partial fix)
0%
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.
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
Updated by Toshi MARUYAMA about 10 years ago
Previous note test on ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-linux]
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
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.
Updated by Toshi MARUYAMA almost 10 years ago
- Status changed from Resolved to Closed
- Resolution set to Wont fix