mruby-bin-mrbc: take the target define from the compiler gem - #7172
Conversation
`MRC_TARGET_*` decides the layout of `struct mrc_ccontext`, so this tool's
translation units and `mruby-compiler`'s have to carry the same one. Each
derived it on its own from the same three questions, and each read its own
copy of the compiler's define list, taken at a different moment:
`mruby-compiler` reads its copy while the gems are being set up, this gem's
is reset to the build's when `build_settings` runs, once every gem is set up.
A `PICORB_VM_*` contributed in between is in one answer and not the other,
and the disagreement reaches the link as field offsets that differ, not as
a build error:
```ruby
MRuby::Build.new('probe') do |conf|
conf.toolchain
conf.gembox 'full-core'
conf.gem "#{MRUBY_ROOT}/examples/mrbgems/c_extension_example" do |g|
g.build.cc.defines << 'PICORB_VM_MRUBYC'
end
end
```
`mruby-compiler` answers `MRC_TARGET_MRUBY` there and the tool answers
`MRC_TARGET_MRUBYC`, both compiled with `-DPICORB_VM_MRUBYC`.
Copy what that gem settled on instead. It is set up by the time this block
runs, whatever order the gems were processed in, so the copy holds whatever
it decided, and a build that defines `MRB_NO_GEMS` with no `PICORB_VM_*`
still gets no target define on either side. `spec.cc.defines.flatten!` goes
with the `include?` calls it was there for; the compile line flattens the
list again when it assembles the flags.
### Verified
Both gems answer `MRC_TARGET_MRUBY` on the probe above, and the `mrbc`
sub-build, which defines `MRB_NO_GEMS`, gets no target define on either
side, as before.
`MRUBY_CONFIG=ci/gcc-clang rake -m test`, four builds and the bintests, KO 0:
bintest 117 bintests
bintest 2303 Skip 10
byte-string 2239 Skip 29
cxx_abi 2303 Skip 10
full-debug 2302 Skip 2
`MRUBY_CONFIG=default rake -m test`, Total 2085 KO 0, plus 106 bintests.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesCompiler define propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change makes mruby-bin-mrbc reuse the compiler gem's target definition, keeping both components aligned without introducing an identified user or production impact; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MRC_TARGET_*decides the layout ofstruct mrc_ccontext, somruby-bin-mrbc's translation units andmruby-compiler's have to carry the same one. Each derives it on its own today, from the same three questions:The two derivations read the same list at different moments.
mruby-compilerreads its copy while the GEMs are being set up;mruby-bin-mrbcasks frombuild_settings, whose reset hands it a copy taken once every GEM is set up. APICORB_VM_*that arrives in between is in one answer and not the other:mruby-compileranswersMRC_TARGET_MRUBYthere and the tool answersMRC_TARGET_MRUBYC, both compiling under-DPICORB_VM_MRUBYC. Nothing fails: the two disagree about where the fields ofstruct mrc_ccontextare, and that reaches the link as offsets, not as an error.This PR copies what
mruby-compilersettled on instead of deriving it a second time:The GEM is set up by the time this block runs, whatever order the GEMs were processed in, so the copy holds whatever it decided. A build that defines
MRB_NO_GEMSwith noPICORB_VM_*still gets no target define on either side, which is what themrbcsub-build does.spec.cc.defines.flatten!goes with theinclude?calls it was there for: a build configuration may push an array, whichinclude?cannot see through. The compile line flattens the list again when it assembles the flags.Verified
Both GEMs answer
MRC_TARGET_MRUBYon the probe above, and themrbcsub-build gets no target define on either side, as before.MRUBY_CONFIG=ci/gcc-clang rake -m test, four builds and the bintests, KO 0:MRUBY_CONFIG=default rake -m test,Total 2085 KO 0plus 106 bintests.Summary by CodeRabbit