JEG2 / highline

A higher level command-line oriented interface.
Other
1.29k stars 137 forks source link

System extensions depend on now-internalized JRuby dependencies #227

Closed headius closed 6 years ago

headius commented 6 years ago

We received a report of highline failing to load on JRuby 9.1.15.0 in jruby/jruby#4889.

The issue is that system_extensions.rb accesss parts of the jline library we ship using its normal package name, and to avoid an embedding issue in JRuby 9.1.15.0 we now have "vendored" that library into a JRuby-specific package (jruby/jruby#4740).

Because of the relocation, highline's system extensions fail to load properly.

We can proceed in a few ways:

There's possibly another path that only require a small change to highline: use both the old path and the new path, since we are unlikely to change the new path ever again. (right, @mkristian?)

mkristian commented 6 years ago

@headius right it is unlikely the path will change again. but it is more likely we switch to jline-3.0 one day. and current jline-3.0 has a dependency to asm which is also something jruby puts into a relocated package.

abinoam commented 6 years ago

I let this up to you (@headius @mkristian) to decide what is best approach for JRuby, I think it will be easy to make Highline compatible with it πŸ‘

PS: I'll probably have time to work on this by monday.

abinoam commented 6 years ago

Hi @headius,

I'm seeing now that the issue is referring to 1-7-stable branch. These "system extensions" have gone in 2.0 (unreleased). I'll try to have a better look at it.

abinoam commented 6 years ago

@tomdz Could you help on this? I saw the JRuby/jline code was made initially by you.

Hooking some other folks that could also probably help: @BanzaiMan, @ai, @presidentbeef, @rsutphin, @mnzaki, @iconoclast.

mkristian commented 6 years ago

just to get the initial context is play as it could be 1-7-branch related: https://github.com/jruby/jruby/issues/4889

presidentbeef commented 6 years ago

FWIW I've been using the 2.0 development releases for JRuby compatibility for a long time.

abinoam commented 6 years ago

just to get the initial context is play as it could be 1-7-branch related: jruby/jruby#4889

The stack trace shows that @petrvalkoun-gooddata is on 1.7.10.

initialize_system_extensions at C:/jruby/lib/ruby/gems/shared/gems/highline-1.7.10/lib/highline/system_extensions.rb:21

abinoam commented 6 years ago

Hi all,

I've run bundle exec rake acceptance and the readline autocomplete test worked ok on 2.0.0-pre.

I've made some simple tests on questions with readline enabled and it's woking ok on 2.0.0-pre

What about just ask @petrvalkoun-gooddata and everybody willing to keep running the most update version of JRuby to migrate his code to highline 2.0? What do you think?

By the way, I think I've postponing the 2.0 release too much. I'm waiting for feedback from people that must have their software affected by the release. But, well, I understand how hard it is to deal with legacy software. This issue afffecting JRuby is the fuel that I need to officially release 2.0. Releasing it will push everybody that haven't set version constraints at their gemspec/Gemfiles to the most up to date HighLine version. I mean, if HighLine 2.0 was already released, perhaps this JRuby/HighLine issue wouldn't even be noticed.

Houston (@JEG2)? Agree? Release 2.0? 9, 8, 7, ...

PS: I've had some issues trying to bundle install at 2.0.0-develop with JRuby. Gem rugged didn't compile native extensions. bundle install --without code_quality for the rescue. As we don't need those gems to run the tests.

mkristian commented 6 years ago

@abinoam cool - FYI gems with native extension do not work with JRuby. Thanks for looking into all this.

abinoam commented 6 years ago

One more good new. The gem that the issue is refereeing is https://github.com/gooddata/gooddata-ruby

Having a fast look at it, it seem it is already ready for HighLine 2.0. Just because it used "best practices". It envelopes Highline use behind a local reference. Just the way HighLine 2 favors!

https://github.com/gooddata/gooddata-ruby/blob/2bd53792728329d380d96a8358cc734359c14fd5/lib/gooddata/cli/terminal.rb#L12

require 'highline'

# Define GoodData::CLI as GLI Wrapper
module GoodData
  module CLI
    DEFAULT_TERMINAL = HighLine.new unless const_defined?(:DEFAULT_TERMINAL)

    class << self
      def terminal
        DEFAULT_TERMINAL
      end
    end
  end
end

Then it uses it like this at https://github.com/gooddata/gooddata-ruby/blob/2bd53792728329d380d96a8358cc734359c14fd5/lib/gooddata/commands/auth.rb#L45

          # Read environment
          environment = GoodData::CLI.terminal.ask('Environment') do |q|
            set_default_value(q, old_credentials[:environment], GoodData::Project::DEFAULT_ENVIRONMENT)
          end

So, it seems it will probably work out of the box with HighLine 2.0 (at least for this specific gem).

abinoam commented 6 years ago

gems with native extension do not work with JRuby.

It is a pronto dependency.

I'll see if I can fine tune the gemspec|Gemfile so it autodetects this.

Thanks πŸ‘

petrvalkoun-gooddata commented 6 years ago

@abinoam many thanks for your help, I have already asked our developers to migrate to highline 2.0. I will notify the result.

abinoam commented 6 years ago

I'm closing this! πŸ‘

petrvalkoun-gooddata commented 6 years ago

Upgrade to highline 2.0 resolved the issue, thanks!