build_config: give the glib_hal_test config a name of its own - #7200
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe ChangesBuild configuration and gem linking
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change gives the GLib HAL test build its own output tree and preserves the existing host build; no actionable merge-blocking risk remains beyond 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 |
`MRuby::Gem::List#linker_attrs` drops every gem that builds a binary, and hands the rest to each link of libmruby.a. 5c130e8 added that for mruby#5210, where a gem's options reached binaries the gem had nothing to do with. It reads `bin?` as "not a library gem", and a gem can be both. `build.libmruby_objs << @objs` (lib/mruby/gem.rb:103) is unconditional, so a gem with a src/ or an mrblib/ puts objects in libmruby.a whatever else it builds. When such a gem also declares `spec.bins`, its objects are in the archive and its linker options are not on the line that links it, so anything but its own binary fails to link. `mruby-task` is that gem as soon as `MRB_TASK_BUILD_DEMO` is set: the define gives it `spec.bins` (mrbgems/mruby-task/mrbgem.rake:52), and it carries src/ and a ports/<port>/ HAL whose glib port is what needs the `search_package` libraries. build_config/glib_hal_test.rb sets both, and mrbtest cannot be linked there: $ MRUBY_CONFIG=glib_hal_test rake -m test ld: mrbgems/mruby-task/ports/glib/task_hal.c:132: undefined reference to `g_main_loop_quit' ld: mrbgems/mruby-task/ports/glib/task_hal.c:134: undefined reference to `g_thread_join' ... rake aborted! Tasks: TOP => bin/mrbtest => build/host/bin/mrbtest gcc -fsanitize=address,undefined -o "build/host/bin/mrbtest" .../driver.o .../vformat.o .../mrbtest.o .../mrbtest.a "build/host/lib/libmruby.a" -lm Drop a binary gem only when it contributes no object. A gem that put nothing in libmruby.a still has options that belong to its own binary alone, which is what mruby#5210 asked for, and one that did needs them wherever libmruby.a is linked: $ MRUBY_CONFIG=glib_hal_test rake -m test # exit 0 bintest - Command Binary Test Total: 0 OK: 0 KO: 0 Crash: 0 Skip: 0 mrbtest - Embeddable Ruby Test Total: 832 OK: 831 KO: 0 Crash: 0 Skip: 1 gcc -fsanitize=address,undefined -o "build/host/bin/mrbtest" .../driver.o .../vformat.o .../mrbtest.o .../mrbtest.a "build/host/lib/libmruby.a" -L/usr/lib/x86_64-linux-gnu -lglib-2.0 -L/usr/lib/x86_64-linux-gnu -lgthread-2.0 -pthread -lglib-2.0 -lm Those libraries appear twice because the gem calls `search_package` twice, before and after this commit alike. The second line follows from the first. tasks/bin.rake:10 asks for `linker_attrs(gem)` when it links a gem's own binary, and a gem that now stays in the library set would be appended to it a second time, so its options would be doubled on that line. Append it only when it is not already there. build/host/bin/mruby_task_demo is linked with the same line as before: gcc -fsanitize=address,undefined -o "build/host/bin/mruby_task_demo" .../mruby_task_demo.o "build/host/lib/libmruby.a" -L/usr/lib/x86_64-linux-gnu -lglib-2.0 -L/usr/lib/x86_64-linux-gnu -lgthread-2.0 -pthread -lglib-2.0 -lm Nothing in the standard gemboxes moves. Every core binary gem ships a tools/ and no src/ or mrblib/, so `@objs` is empty for all of them (:62, :96, :97) and the set `linker_attrs` returns is the one it returned before. The default build links bin/mruby and bin/mirb with byte-identical command lines, and `rake -m test` is green: $ rake -m test # exit 0 Total: 2089 OK: 2041 KO: 0 Crash: 0 Skip: 48 The example from mruby#5210 behaves as it did. A gem carrying only a tools/foo/main.c and `spec.linker.library_paths << "/usr/lib/foo"`, built beside mruby-bin-mirb, keeps that path off every line but its own, and it appears once in the whole run: $ MRUBY_CONFIG=issue5210.rb rake --verbose > log # exit 0 $ grep -c /usr/lib/foo log 1 $ grep -o 'gcc.*bin/foo".*' log gcc -L/usr/lib/foo -o "build/issue5210/bin/foo" .../main.o "build/issue5210/lib/libmruby.a" -lm $ grep -o 'gcc.*bin/mirb".*' log gcc -o "build/issue5210/bin/mirb" .../mirb.o ... "build/issue5210/lib/libmruby.a" -lm
build_config/glib_hal_test.rb opened an anonymous `MRuby::Build`, and an
unnamed build is called 'host' (lib/mruby/build.rb:94). The build tree
is keyed by that name (:107), so MRUBY_CONFIG=glib_hal_test aimed its
objects at build/host, on top of whatever a default build had left
there. No build/glib_hal_test was ever created.
This config loads no gembox at all, so what it shares build/host with is
a tree built from a different set of gems, and the presym scan cannot
tell them apart. It reads the `.pi` files the compilers leave behind,
and those are keyed on the source timestamp alone, so a `.pi` a default
build preprocessed under its own defines is reused here unchanged. The
result is a symbol table belonging to neither build, and this config
aborts on the first core file that needs a symbol it lost:
$ rake -j8 # a default build, exit 0
$ MRUBY_CONFIG=glib_hal_test rake -j8
include/mruby/presym.h:42:23: error: 'MRB_SYM__print' undeclared
here (not in a function); did you mean 'MRB_SYM__private'?
42 | #define MRB_SYM(name) MRB_SYM__##name
src/kernel.c:811:29: note: in expansion of macro 'MRB_SYM'
811 | MRB_MT_ENTRY(mrb_print_m, MRB_SYM(print), MRB_ARGS_ANY() |
| MRB_MT_PRIVATE),
rake aborted!
Tasks: TOP => build => build/host/lib/libmruby.a
=> build/host/src/kernel.o
`MRB_SYM(p)` and `MRB_SYM(print)` sit behind `#ifndef HAVE_MRUBY_IO_GEM`
(src/kernel.c:809). A default build carries mruby-io and its kernel.pi
does not name them; this build carries no io gem and its src/kernel.c
does. The stale kernel.pi is what gets scanned, so the table is built
without them:
$ md5sum build/host/src/kernel.pi # before and after the run
358985be208e175586d7b6486c9726b9 build/host/src/kernel.pi
358985be208e175586d7b6486c9726b9 build/host/src/kernel.pi
The leak runs the other way too. `MRB_SYM(Rational)` reaches the table
from the default build's numeric.pi, preprocessed with
`-DMRB_USE_RATIONAL`, which this build never sets. Compared with the
same config on a clean tree, the list it writes is missing p, print and
the ten mrblib file names, and carries Rational that does not belong to
it: 442 symbols on a clean tree, 431 here. The file names go missing
for the same reason, one step further along: `enable_debug` appends -g
to the mrbc options (:176), so this config's mrblib.c holds them and the
default build's, reused here, does not.
Whether it gets that far is decided by the timestamps in build/host, so
the config does build against a tree it does not clobber:
$ rm -rf build/host
$ MRUBY_CONFIG=glib_hal_test rake -j8 # exit 0
$ ls build/host/bin/
mruby_task_demo
which is the shape the tree is left in either way. `MRB_TASK_BUILD_DEMO`
gives mruby-task a `spec.bins`, and bin/ symlinks are installed for a
`host?` build (:452), so the demo lands in the repo bin/ beside the
default build's commands, and build/host holds an `-fsanitize` tree at
-O0 with a 889 symbol table where the default build's has 3161.
Name the build after its config, which is what build_config/asan.rb and
build_config/gctest.rb already do, and what 8b44a52 did for host-cxx.
Naming is not enough on its own here. The internal mrbc build is
created only for a `host?` build or one carrying mruby-bin-mrbc (:153),
and this config loads no gembox to supply it, so the rename alone stops
before it compiles anything:
$ MRUBY_CONFIG=glib_hal_test rake -j8
rake aborted!
external mrbc or mruby-bin-mrbc gem in current('glib_hal_test') or
'host' build is required
tasks/mrblib.rake:9
build_config/gctest.rb is the same pair for the same reason, a named
build with an explicit mruby-bin-mrbc line.
mruby-bin-mrbc brings a bintest of its own, and the config already sets
`enable_bintest`, where before the rename it had no bintest to run at
all. One of its asserts compares the disassembly mrbc writes against
the one `mruby -v -c` writes (mrbgems/mruby-bin-mrbc/bintest/mrbc.rb:85),
and nothing here supplies that binary, so it ends the run:
Errno::ENOENT: mrbc -v disassembles like mruby -v
=> No such file or directory - build/glib_hal_test/bin/mruby
Crash: 1
Add mruby-bin-mruby as well, so the test the compiler gem carries has
what it asks for. With the three lines, the config builds into a
directory of its own, through the sequence that aborted:
$ rake -j8 # a default build, exit 0
$ MRUBY_CONFIG=glib_hal_test rake -j8 # exit 0
...
Config Name: glib_hal_test
Output Directory: build/glib_hal_test
$ ls build/
glib_hal_test host
$ ls build/glib_hal_test/bin/
mrbc mruby mruby_task_demo
$ wc -l build/glib_hal_test/include/mruby/presym/table.h
889 build/glib_hal_test/include/mruby/presym/table.h
and an existing host build is left byte-identical, with its own table
and its own bin/:
$ md5sum build/host/bin/mruby build/host/bin/mrbc build/host/lib/libmruby.a
9881d200e90c5a7df4a29a36b193fbab build/host/bin/mruby
c6b0d6d168918de337fa27a8d5d2a262 build/host/bin/mrbc
8dbb8c26becf449f5871f9e090c8a4ff build/host/lib/libmruby.a
$ wc -l build/host/include/mruby/presym/table.h
3161 build/host/include/mruby/presym/table.h
$ ls bin/
mirb mrb mrbc mrdb mruby mruby-config mruby-strip
$ rake -j8 # exit 0
$ build/glib_hal_test/bin/mruby_task_demo
[t= 0 ms] main: spawning T2 + T3; running T1 (glib-only) on main
thread
...
`rake test` passes on top of the commit before this one, which keeps
the linker options of a gem whose objects are in libmruby.a. Without
it, mrbtest cannot be linked here at all: `MRB_TASK_BUILD_DEMO` makes
mruby-task a bin gem, and `MRuby::Gem::List#linker_attrs` dropped the
`-lglib-2.0` that `search_package` contributed from the mrbtest link
line. With the two together:
$ MRUBY_CONFIG=glib_hal_test rake -m test # exit 0
bintest - Command Binary Test
Total: 53 OK: 52 KO: 0 Crash: 0 Skip: 1
mrbtest - Embeddable Ruby Test
Total: 832 OK: 831 KO: 0 Crash: 0 Skip: 1
$ ls bin/
mirb mrb mrbc mrdb mruby mruby-config mruby-strip
A default build is unaffected, and `rake -m test` on it stays green:
$ rake -m test # exit 0
Total: 2089 OK: 2041 KO: 0 Crash: 0 Skip: 48
f47da02 to
1b1c5df
Compare
build_config/ci/msvc.rb opened an anonymous `MRuby::Build`, and an unnamed build is called 'host' (lib/mruby/build.rb:114). The build tree is keyed by that name (:127), so MRUBY_CONFIG=ci/msvc aimed a full-core build with `MRB_GC_FIXED_ARENA` at build/host, the directory a default build on the same machine uses. The Windows-VC job never sees that, since it builds a fresh checkout, but a developer who points MRUBY_CONFIG at this config on a tree the default config has built shares one directory between two gem sets, which is the shape mruby#7195, mruby#7197, mruby#7199 and mruby#7200 took out of the other configs that had it. 14d6e42 named the bintest build in ci/gcc-clang and left this one alone, on the ground that a single build has no sibling for a name to tell it apart from. That was a reason about telling builds apart; the build directory is decided by the name whether or not there is a sibling, and that is the reason here. Name it after its config, as build_config/mrbc.rb, clang-asan.rb and gctest.rb are. Nothing in the file needs the build to be called 'host'. It is an `MRuby::Build`, and it pulls `mruby-bin-mrbc` in through full-core, so it lends no mrbc to any cross target and creates its own internal one either way (:174). The name reaches three other places: `install_prefix` (:625), the `bin/*.bat` wrappers that `define_installer_if_needed` writes only when `host?` answers true (:461 and :482), and the mrbtest that `rake clean` removes from MRUBY_ROOT/bin (tasks/test.rake:82). The job runs `rake -m test:run:serial` (.github/workflows/build.yml:89), which installs nothing and cleans nothing, so none of the three fires. `enable_bintest` has not required the name 'host' since fd113da, and test/bintest.rb resolves every binary it runs from BUILD_DIR rather than from bin/; the `bintest` build in ci/gcc-clang is this same shape under a name, in the same workflow. There is no MSVC here to run the job with. What can be checked on Linux is that the config still loads and that the tree it names is its own; the job on this change is the run: $ MRUBY_CONFIG=ci/msvc rake -T | grep amalgam rake amalgam # Generate amalgamated mruby.h and mruby.c in .../build/msvc/amalgam
build_config/glib_hal_test.rbopens an anonymousMRuby::Build, and an unnamed build is calledhost(lib/mruby/build.rb:94). The build tree is keyed by that name (:107), soMRUBY_CONFIG=glib_hal_testaims its objects atbuild/host, on top of whatever a default build has left there, and never creates a directory of its own. This is the same shape as #7195, #7197 and #7199, on a config none of them touches.All of the transcripts below are from today's master, gcc 13.3.0, glib 2.80.0.
Sharing
build/hostbreaks this config outrightIt loads no gembox at all, so what it shares
build/hostwith is a tree built from a different set of gems, and the presym scan cannot tell them apart. It reads the.pifiles the compilers leave behind, and those are keyed on the source timestamp alone, so a.pia default build preprocessed under its own defines is reused here unchanged. The result is a symbol table belonging to neither build, and this config aborts on the first core file that needs a symbol it lost:MRB_SYM(p)andMRB_SYM(print)sit behind#ifndef HAVE_MRUBY_IO_GEM(src/kernel.c:809). A default build carriesmruby-ioand itskernel.pidoes not name them; this build carries no io gem and itssrc/kernel.cdoes. The stalekernel.piis what gets scanned:The leak runs the other way too.
MRB_SYM(Rational)reaches the table from the default build'snumeric.pi, preprocessed with-DMRB_USE_RATIONAL, which this build never sets. Against the list the same config writes on a clean tree, this one is missingp,printand the tenmrblib/*.rbfile names, and carriesRationalthat does not belong to it: 442 symbols on a clean tree, 431 here. The file names go missing for the same reason one step further along, sinceenable_debugappends-gto the mrbc options (:176), so this config'smrblib.cholds them and the default build's, reused here, does not.What it leaves behind when it does get through
Whether it gets that far is decided by the timestamps in
build/host, so the config does build against a tree it does not clobber:which is the shape the tree is left in either way.
MRB_TASK_BUILD_DEMOgivesmruby-taskaspec.bins, andbin/symlinks are installed for ahost?build (:452), so the demo lands in the repobin/beside the default build's commands, andbuild/hostholds an-fsanitizetree at-O0with an 889 symbol table where the default build's has 3161.Naming it
The build is named after its config, which is what
build_config/asan.rbandbuild_config/gctest.rbalready do, and what 8b44a52 did forhost-cxx. Naming is not enough on its own here. The internal mrbc build is created only for ahost?build or one carryingmruby-bin-mrbc(:153), and this config loads no gembox to supply it, so the rename alone stops before it compiles anything:build_config/gctest.rbis the same pair for the same reason, a named build with an explicitmruby-bin-mrbcline.mruby-bin-mrbcbrings a bintest of its own, and the config already setsenable_bintest, where before the rename it had no bintest to run at all. One of its asserts compares the disassembly mrbc writes against the onemruby -v -cwrites (mrbgems/mruby-bin-mrbc/bintest/mrbc.rb:85), and nothing here supplies that binary, so it ends the run:Add
mruby-bin-mrubyas well, so the test the compiler gem carries has what it asks for.Verified
Through the sequence that aborted:
and an existing host build is left byte-identical, with its own table and its own
bin/:rake testIt passes on top of #7203, which keeps the linker options of a gem whose objects are in
libmruby.a. Without that commit mrbtest cannot be linked here at all:MRB_TASK_BUILD_DEMOmakesmruby-taska bin gem, andMRuby::Gem::List#linker_attrsdropped the-lglib-2.0thatsearch_packagecontributed from the mrbtest link line. With the two together:A default build is unaffected, and
rake -m teston it stays green:Environment
Details
The line each build gives
src/string.c, read offrake --verbosewith-MMD -c, the-Ilist and-odropped.glib_hal_testcallsenable_debug, which appends-g3 -O0after the gcc toolchain's own-g -O3. The gembox difference the presym scan trips over is visible here as well, in the-Dlist:Summary by CodeRabbit