build_config: give the msvc build a name of its own - #7246
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 (1)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe MSVC CI build configuration now assigns the MRuby build the explicit target name ChangesMSVC build configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change gives the MSVC configuration its own build directory without evidence of a current correctness or production-impact risk; it is merge-ready after normal checks and review. 🚥 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 |
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/ci/msvc.rbopens an anonymousMRuby::Build, and an unnamed build is calledhost(lib/mruby/build.rb:114). The build tree is keyed by that name (:127), soMRUBY_CONFIG=ci/msvcaims a full-core build withMRB_GC_FIXED_ARENAatbuild/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 pointsMRUBY_CONFIGat this config on a tree the default config has built shares one directory between two gem sets, which is the shape #7195, #7197, #7199 and #7200 took out of the other configs that had it.14d6e42 named the bintest build in
ci/gcc-clangand 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.Naming it
The build is named after its config, as
build_config/mrbc.rb,clang-asan.rbandcosmopolitan.rb(#7245) are. Nothing in the file needs the build to be calledhost. It is anMRuby::Build, and it pullsmruby-bin-mrbcin 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), thebin/*.batwrappers thatdefine_installer_if_neededwrites only whenhost?answers true (:461,:482), and the mrbtest thatrake cleanremoves fromMRUBY_ROOT/bin(tasks/test.rake:82). The job runsrake -m test:run:serial(.github/workflows/build.yml:89), which installs nothing and cleans nothing, so none of the three fires.enable_bintesthas not required the namehostsince fd113da, andtest/bintest.rbresolves every binary it runs fromBUILD_DIRrather than frombin/; thebintestbuild inci/gcc-clangis this same shape under a name, in the same workflow.Verified
There is no MSVC here to run the job with, so the Windows-VC job on this PR is the run. What can be checked on Linux is that the config still loads and that the tree it names is its own:
Summary by CodeRabbit