Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled did_you_mean, install during build, and clear gem paths #3892

Merged
merged 2 commits into from May 20, 2016

Conversation

@headius
Copy link
Member

headius commented May 16, 2016

This enables did_you_mean and installs it during the build. Fixes #3480.

@mkristian I could not figure out how to make this work without being a default gem. We do not have any logic to preinstall gems that aren't default, do we?

This also includes the Gem.clear_paths hack to address too-early booting of RubyGems in embedded scenarios.

@headius headius added this to the JRuby 9.1.2.0 milestone May 16, 2016
@mkristian
Copy link
Member

mkristian commented May 17, 2016

@headius what is the advantage of preinstalled gems vs. default gem. a preinstalled gem is gone the moment you set GEM_HOME and GEM_PATH, i.e. you want to install gems with jruby-complete.jar.

@headius
Copy link
Member Author

headius commented May 18, 2016

@mkristian Not a great deal, I guess. Being preinstalled means you won't be able to require it if gems are disabled...that's the only real difference. But for a library being activated immediately, being preinstalled also boots much more of RG than we want.

MRI chose not to make this a default gem, and they are actively trying to move away from default gems. I don't know their reasons, though.

@headius
Copy link
Member Author

headius commented May 19, 2016

I'm open for a vote on what to do from here... merge this now to master as a default gem (and probably remove the gem and Gem.clear_paths calls), or wait and do something different?

@enebo @mkristian @kares

@mkristian
Copy link
Member

mkristian commented May 20, 2016

+1 merge,

we have a number of default gems and the question preinstalled gem vs. default gem is a more general topic

@headius
Copy link
Member Author

headius commented May 20, 2016

Might as well let it bake now.

@headius headius merged commit bccbfd0 into jruby:master May 20, 2016
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@yuki24
Copy link
Contributor

yuki24 commented May 21, 2016

I'm very honored to see the did_you_mean gem become part of JRuby. Thank you for making this happen! Please let me know if there's anything I can help. I'm always happy to make JRuby even better!

@enebo enebo modified the milestones: JRuby 9.1.2.0, JRuby 9.1.3.0 May 25, 2016
@headius
Copy link
Member Author

headius commented May 26, 2016

@yuki24 Thank you for working through issues with us!

@headius headius deleted the headius:did_you_mean branch May 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.