Backport 9.3 fixes for 9.2.12#6285
Merged
headius merged 11 commits intojruby:jruby-9.2from Jun 17, 2020
Merged
Conversation
Member
Author
This was referenced Jun 16, 2020
This isn't great since we're allowing a null field and checking for null, but the logic surrounding this has been designed to lazily create the channel or stream in question. This is likely due to the use of URLResource and its siblings as generic path/URI mungers that can clean up and canonicalize/abosolutize even paths that do not exist. Rather than attempt to isolate that behavior (which might be a lot more work), I've opted to mimic FileResource in raising an error along the lazy path when it's clear we don't have a resource in hand to open. This fixes jruby#6219.
Because all modules will report isOpen => true when called from the unnamed module, we need a different way of accessing them. This logic makes two changes: * Use Lookup to acquire a method handle rather than using the reflected object. This allows to be sure we now have a fully accessible mechanism for accessing the related field or method, without triggering any warnings. Lookup.unreflect* will raise an exception if the reflected object is not already accessible, rather than a warning we cannot detect. * Use addOpens to attempt to *really* make the pseudo-open packages be open. This allows us to proceed to setAccessible knowing the package has moved from "kinda-sorta-open" to really open. The fallbacks here occur in the following cases: * If we cannot acquire a reference to a non-standard JDK class, like those in sun.nio.ch. This should not ever fail; reflecting against these classes is always permitted. * If we cannot acquire the method or field in question because it is not there. Again this should not fail under normal circumstances. * If we cannot unreflect a handle and cannot addOpens for the package in question.
Member
Author
|
Looking into the build failure locally. Clearly we did something on master to fix it but I can't remember what. 🤔 |
This is a partial fix for jruby#6103. The issue there is that in 9.2.10 we updated to a version of RubyGems that no longer supports the --no-rdoc and --no-ri flags. Normally the build here would run with 9.2.7.0, which does support those flags, but when running a SNAPSHOT build we use the version of JRuby that we're building. If that version is >= 9.2.10.0 it will fail because the --no-rdoc flag is not recognized. We could fix it by enabling rdoc and bypassing the relevant logic in the InstallMojo from gem-maven-plugin, but we don't really want to install rdoc and ri for the bootstrap gems. Instead, I am forcing this to always use 9.2.7.0 until we can get the plugin updated to a newer JRuby and fix the flags appropriately. This allows a non-SNAPSHOT bootstrap to almost work; it then fails with the issue in jruby#6174. I will commit a separate workaround for that. Works around jruby#6103.
This gem was only being installed for non-snapshot (release) builds, even though we don't ever ship the results of that install. Because of jruby#6174, that install fails because no jruby.home is passed in the extension-building subprocess. This works around the issue by not installing jruby-launcher for release builds, but the problem remains: gem-maven-plugin cannot currently install gems with native builds due to the build command line not providing a jruby.home. This relates to jruby#6103, which prevented us from getting to this point before if building a non-snapshot build. With this change, both snapshots and releases will bootstrap correctly, and neither will install jruby-launcher.
Member
Author
|
See #6103 and #6174 which were the causes of build failures. The trigger was not having the branch pointing at a SNAPSHOT build, which caused the bootstrap build to use the current JRuby (which doesn't support I have worked around these issues on the branch, and my workarounds should be safe to merge to master. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR will pull in small backports from master that we want to release as part of 9.2.12.