Skip to content

mruby-bin-mrbc: take the target define from the compiler gem - #7172

Merged
matz merged 1 commit into
mruby:masterfrom
takumin:mrbc-copy-target-define
Aug 14, 2026
Merged

mruby-bin-mrbc: take the target define from the compiler gem#7172
matz merged 1 commit into
mruby:masterfrom
takumin:mrbc-copy-target-define

Conversation

@takumin

@takumin takumin commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

MRC_TARGET_* decides the layout of struct mrc_ccontext, so mruby-bin-mrbc's translation units and mruby-compiler's have to carry the same one. Each derives it on its own today, from the same three questions:

if cc.defines.include?('PICORB_VM_MRUBY')
  cc.defines << 'MRC_TARGET_MRUBY'
elsif cc.defines.include?('PICORB_VM_MRUBYC')
  cc.defines << 'MRC_TARGET_MRUBYC'
elsif !cc.defines.include?('MRB_NO_GEMS')
  cc.defines << 'MRC_TARGET_MRUBY'
end

The two derivations read the same list at different moments. mruby-compiler reads its copy while the GEMs are being set up; mruby-bin-mrbc asks from build_settings, whose reset hands it a copy taken once every GEM is set up. A PICORB_VM_* that arrives in between is in one answer and not the other:

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 compiling under -DPICORB_VM_MRUBYC. Nothing fails: the two disagree about where the fields of struct mrc_ccontext are, and that reaches the link as offsets, not as an error.

This PR copies what mruby-compiler settled on instead of deriving it a second time:

spec.cc.defines += compiler.cc.defines.flatten.grep(/\AMRC_TARGET_/)

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_GEMS with no PICORB_VM_* still gets no target define on either side, which is what the mrbc sub-build does.

spec.cc.defines.flatten! goes with the include? calls it was there for: a build configuration may push an array, which include? cannot see through. 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 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.

Summary by CodeRabbit

  • Bug Fixes
    • Improved compiler configuration handling during builds.
    • Ensured target-specific compiler definitions are applied consistently, improving build reliability across supported configurations.

`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.
@takumin
takumin requested a review from matz as a code owner August 14, 2026 12:19
@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 12e2bdea-08dc-48cd-b6f8-04011400cd15

📥 Commits

Reviewing files that changed from the base of the PR and between f90657d and 7b177a1.

📒 Files selected for processing (1)
  • mrbgems/mruby-bin-mrbc/mrbgem.rake

📝 Walkthrough

Walkthrough

Changes

Compiler define propagation

Layer / File(s) Summary
mrbc compiler setup
mrbgems/mruby-bin-mrbc/mrbgem.rake
mrbc retrieves mruby-compiler before collecting objects and copies its MRC_TARGET_* defines directly. The previous conditional define derivation was removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7b177

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

  • mruby/mruby#7159: Adds compiler-define tracking and querying in MRuby::Build, which relates to compiler-define propagation.

Suggested labels: build

Suggested reviewers: matz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: taking the target define from the compiler gem.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matz
matz merged commit 48562cd into mruby:master Aug 14, 2026
21 checks passed
@takumin
takumin deleted the mrbc-copy-target-define branch August 14, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants