Conversation
|
tests are for sure more work |
|
@mkristian This looks right at a glance. We need to get @enebo on board and obviously get tests passing, but this is largely what I had envisioned for jruby-base. |
|
the last commit is just the idea to keep changes minimal, i.e. it keeps source in old place but /core/pom.rb is build jruby-base artifact and the /shaded/pom.rb is building jruby-core. this is not ideal but the moment I had this original commit in place I realized that it breaks open PR and it is hard to keep in sync with master. (might be wrong here as well - just an initial observation) |
|
@headius the failure one the dependency-check (JRuby CI) can be probably fixed by add |
|
interesting, so there would be a non-shaded (jruby-base), is this still being considered (e.g. for 9.3) ? |
|
@kares Yes, that is the goal. We would then have jruby-base that's just the JRuby-sources, jruby-core which is jruby-base + shaded dependencies, and jruby-complete which is jruby-core plus the standard library. Basically, it would allow packagers to include JRuby and dependencies in their own ways, as in #6205. This needs to be evaluated and merged ASAP if we intend to get it into 9.3. |
headius
left a comment
There was a problem hiding this comment.
This seems pretty mundane and fulfills what we want. I say we merge to master and proceed from there.
|
There seems to be one remaining issue with the jruby-jars artifact... it errors out with an error that looks like it can't find the shaded location of ASM. I've pushed a change to fix part of the @mkristian I'm not sure what's broken here... can you help so we can merge this? |
|
Ah there also seems to be a problem with the dependency convergence testing... so that needs to be resolved as well. |
|
@headius the dependencies convergance can be probably fixed by adding |
|
@mkristian Assuming I did what you suggested, it did not appear to help. The dependency-convergence build still fails with this message during the jruby-core build: The jruby-jars artifact also still fails to build for some reason. |
|
OK - fixed dependency-check but seems like I broke all others. |
|
Looking good now! |
|
I'm going to look into the size checks and see why jruby-core is so off, but otherwise this is darn close to being done. @enebo Any concerns? |
|
Ok, this is confusing. We shade all dependencies into the "dist" jar that goes in lib, on both master and base branches. It seems the shading for the "core" jar has changed however. From master, I see the following output when creating the jruby-core jar for the check-versions.sh script: This produces a jar that is indeed only about 10MB, as the script expects. It appears from this that the only shaded library is ASM, which confuses me because I thought we were also shading everything that depends on ASM like jitescript and jnr-ffi. The jnr-ffi shading in particular would be the cause of #6205. However, the output from the base branch now shows jruby-core including everything: Perhaps we had some miscommunication or confusion on my part, but the published jruby-core jar should be structured exactly like it was in JRuby 9.2, with the same dependencies shaded. I may have been under the impression that jruby-core shaded more than it did. If the core jar is trimmed back to the same shaded dependencies, I think this will be ready. I'll try to do that. |
I think we were confused by lib/jruby.jar when discussing the jruby-core module and the libraries it shades. The lib/jruby.jar file for the JRuby distribution shades in all Java depdendencies, while the actual published jruby-core jar only shades ASM (hidden), jitescript and jnr-ffi (because they depend on ASM). On the new "base" branch, we originally planned for "base" to be the fully-unshaded jar and "core" to be "base plus shaded Java dependencies", but this does not match the historical "core" jar we have been publishing. This commit restores the limited shading for the jruby-core jar. The jruby-core module may now be the least useful; it does not shade in all dependencies it needs, but at the same time it shades enough (jnr-ffi) to cause version conflicts with other libraries. I believe we should consider jruby-core deprecated for 9.3 in favor of jruby-base as the baseline artifact, with core continuing to be base + relocated ASM and complete being base + all Java deps and the Ruby stdlib.
|
I've pushed a commit that restores jruby-core to shading only what it did before. We may want to deprecate that artifact going forward in 9.3. It doesn't shade everything it needs (so you need to deal with that yourself) but at the same time it shades enough to cause conflicts like in #6205 because it pulls in and does not relocate jnr-ffi. The future is jruby-base with no shading or jruby-complete with everything shaded, methinks. |
The core build does not appear to trigger when building jruby-jars now after the move to the "base" jar setup. I add jruby-core as a compile dependency here to ensure it builds, and then explicitly add only the "core complete" jar that we copy from the JRuby dist lib dir.
|
jruby-jars appears to not be triggering a build of the "lib" version of the core jar. I have a patch locally that I will push, hopefully it's doing the right thing @mkristian? |
|
@mkristian Everything is green now and I believe the artifacts are being assembled correct. We can do additional tweaks, but this is now landed on master. Thank you for your help as always! |
@headius please have a look. I check both lib/jruby.jar and jruby-complete.jar and it needs more verifications. but it is a first idea how this could look like.