-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Bundle did_you_mean gem #3480
Comments
need to disable Ruby version validation at RubyGems due : ``` Gem::InstallError: did_you_mean requires Ruby version >= 2.3.0 ```
The creator of the |
@yuki24 great. Thanks for the update! |
Ok, now the only change we need to add to JRuby is this change: https://bugs.ruby-lang.org/issues/11777. It's a little weird change as this is a special case just for the |
@yuki24 We are not in a position to make that change right now, since it would require significant rework of how we do method invocation. Method call logic does not have access to the local state of the caller, in order to allow us to optimize the caller independent of the call. I notice matz wanted it called "internal" but these things have a way of becoming official APIs. We won't be making any changes for this in 9.1. It may be something we can add afterwards, since we have already been forced to provide more caller data to support refinements. The structures here are basically the same. |
#3816 is the ongoing bug for missing 2.3 features, including |
I'm actually ok with giving up on that particular feature. It's the only feature that is not enabled yet and everything else is green: https://travis-ci.org/yuki24/did_you_mean/jobs/126229792#L311-L321 I don't think we should treat it as a blocker. We can just disable it for the time being, and when JRuby is ready we can just turn it back on. |
@yuki24 Due to other issues (we don't like how DYM gets loaded by RubyGems right now) we will do this in 9.1.1.0. Thank you for your help getting DYM working on JRuby! |
Oh FWIW, we still have the logic to load DYM on startup in JRuby 9.1, but it is disabled by default. If you are a user who really wants DYM on by default in your environment, you'll need to install it manually and add |
@headius Thank you for the update! So is it safe to assume JRuby 9.1.1.0 will come with this change? |
@yuki24 We might be able to enable it for very limited use (i.e. interpreter-only) but I don't think any The problem is that we don't always have access to the local variable names for an arbitrary executing method. In many cases we can optimize Ruby code down to straightforward JVM bytecode without any Ruby runtime structures required to execute. I may spike something this week that just embeds the variable names into each call site. The other problem with us including DYM into 9.1.1.0 is #3828...loading DYM on every boot as a normal gem causes RubyGems to initialize many internal structures before we have an opportunity to tweak them for JRuby's more exotic deployment modes. This needs to be addressed in RubyGems, most likely. |
@headius Thank you for the clarification! I'll just kill the local variable name correction on JRuby for the time being. Does the use of |
@yuki24 Currently all that enable/disable of We could make it a "default gem" that's "installed" directly into stdlib, but MRI has been reluctant to introduce more default gems. I'm hoping @tenderlove or the folks currently maintaining RubyGems will have some ideas for us. |
@headius default gem might not work, as rubygems need to look for newer versions of the default gem inside the installed gems first, i.e. the but we just could |
@mkristian Ahh that's pretty clever. |
@headius it will reset all we know gives problems so far :) |
Worth giving it a try, but I want to do it in 9.1.2.0 and let it bake for a while. |
Speaking of the |
I've managed to add an event hook that captures a binding object for the JRuby runtime. This way we wouldn't have to wait for I'm not very familiar with RubyGems and JRuby, so there isn't much I can do for the other issue... But I would be more than happy to wait until 9.1.2.0. |
@yuki24 Clever! Unfortunately this suffers from a different issue: normally we don't trigger all event hook trace points without --debug, since it would add needless overhead even when there's no hook installed. The code you wrote should work fine with --debug, however. I'm not sure what our ETA is for NameError#local_variables. Thoughts on that and this hook, @enebo? |
@headius @yuki24 I think as far as this goes it will not work without --debug and perhaps that is ok but at the same time I think longer term we should be passing something through our call path which contains (or can access static scope). So perhaps we can land this short term and the long term fix will be much much larger and in the future? |
I just realized this hook patch is still dependent on us not throwing away any dynamic scopes, so it probably won't help enough to merge. |
An additional snag: we don't seem to have the appropriate plumbing in our build to install a gem without making it a default gem. I'm working on that now and will push a branch/PR with |
This is from https://bugs.ruby-lang.org/issues/11252. See #3479 for full Ruby 2.3 checklist.
The
did_you_mean
gem usesTracePoint
to interceptNameError
(I think) and report a more useful message, guessing at what error you might have meant.JRuby implements
TracePoint
, but most tracing is disabled by default for performance. Exception tracing might be ok, though...we need to confirm. And we may need to modify the "tracing is disabled" warning when the requested trace is 100% ok (if that's the case for all tracingdid_you_mean
uses).I started the process by adding the beta3 version of the gem to lib/pom.rb, but it didn't seem to be able to find it via our proxies. Perhaps the proxy doesn't support beta gems?
The text was updated successfully, but these errors were encountered: