Fetch Chicory/ASM jars from Maven instead of bundling in gem#3019
Fetch Chicory/ASM jars from Maven instead of bundling in gem#3019soutaro wants to merge 2 commits into
Conversation
headius
left a comment
There was a problem hiding this comment.
This is basically correct but a mess of conflicting comments from the LLM.
- Only the direct dependency jars need to be specified. I believe that's just
runtimeandcompiler. - Jar specifications are duplicated several places even though the comments claim they should live in the jars source file.
- rbs_jars.rb says it is hand-maintained but other places say it is generated. LLM is getting confused and contradicting itself.
This matches the work I did locally as an experiment, but quite a bit messier with nonsense comments.
|
LLM is making up nonsense, or getting confused...
I think in all scenarios jars are downloaded to local .m2 for use, so I'm not sure what this means.
This file does not exist. The actual file, rbs_jars.rb, does seem to be the single source of truth... except its contents get duplicated several other places.
Probably no need for this. The WASM interpreter will probably be unusably slow without the compiler, and there's no reason it won't load now. -- The actual changes to support jar-dependencies look fine but the documentation has a lot of nonsense in it. |
|
Has this bundled gem degenerated from methodical development to prompt engineering and botsitting? If the bot cannot do it right, throw its slop out and do it ourselves, and consult human experts rather than a black box of probabilities. Relying on bot output is not “hand-maintained”; it’s vibed. |
cdb9eaf to
91e7a0c
Compare
|
Fair concern — this PR was a complicated and confused draft, so I understand the reaction. Either way, I'm responsible for the code that lands here, whether I write it myself, an LLM does, or another contributor does. And I'll be honest: I have almost no JRuby context, and I don't think I could have gotten this one done without the AI's help. The history is squashed and the misleading description is being rewritten. Thanks for keeping the quality bar high. |
91e7a0c to
238cc50
Compare
The -java gem previously bundled the Chicory/ASM jars (~1MB) under lib/rbs/wasm/jars. Ship them from Maven via jar-dependencies instead (issue #3018): - rbs.gemspec: depend on jar-dependencies and declare only the top-level `jar` requirements (com.dylibso.chicory:compiler and :wasi); Maven resolves runtime/wasm/log/asm transitively at install. Ship rbs_parser.wasm and lib/rbs_jars.rb, not the jars. - lib/rbs_jars.rb: jar-dependencies' generated require file. Two edits are kept by hand: the com.dylibso.chicory:runtime require_jar line is corrected (the generator mangles that artifact id to `jar`), and the "generated file" marker is dropped so it is not regenerated and re-broken at gem install. - RBS::WASM::Runtime loads the jars with `require "rbs_jars"`. - Rakefile: `wasm:install_jars` downloads the jars into ~/.m2; `wasm:jruby_setup` builds rbs_parser.wasm. - CI builds the wasm on CRuby and downloads the jars on JRuby before tests. Refs #3018 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtMsDQEmuayTdn3rYsteYT
rdoc 8.0.0 added a runtime dependency on rbs. On JRuby that pulls the released ruby-platform rbs gem, whose C extension cannot build (you can't build MRI C extensions on JRuby), so `gem install rdoc` fails before the suite runs. This broke the jruby job on master when rdoc 8.0.0 was released; pin rdoc below 8 until a -java rbs gem is published. Refs #3018 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtMsDQEmuayTdn3rYsteYT
62a62c0 to
4fce08d
Compare
The
-javagem bundled the Chicory/ASM jars (~1 MB) underlib/rbs/wasm/jars. This PR declares the dependencies withjar-dependenciesrequirements instead, so they're fetched from Maven Central at install time — the published gem stays small, everyone resolves the same versions from the canonical source, and conflicting copies can't be loaded at once.Fixes #3018.