Repository navigation
fix(gcc,llvm,rust): let each compiler build hello-world from its own closure - #805
norrietaylor wants to merge 3 commits into
Conversation
…closure A box that adds only gcc, llvm or rust could not compile a trivial program: gcc and clang had no assembler or linker, C and C++ code could not include kernel headers, and rustc found no cc to link through. - gcc: runtime-depend on binutils and linux_headers, and install a cc symlink (gcc already installs c++). The existing tests no longer add binutils themselves, and a new test covers cc and g++ with <linux/types.h>. - llvm: runtime-depend on binutils and linux_headers so clang finds ld and the kernel headers. - rust: runtime-depend on toolchain, the same C toolchain the rust stack adds, so rustc's default cc linker resolves. Refs gominimal/inbox#843
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughGCC and LLVM add runtime dependencies and standalone compiler tests. GCC also creates a ChangesCompiler Package Changes
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change lets each compiler package build a simple program from its own dependency closure. No merge-blocking issue was identified, and CI should confirm the LLVM and Rust builds. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oss a buildbot restart) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
A box that adds one compiler package on its own can't build a trivial program:
gccgcc h.ccannot execute 'as': posix_spawnp: No such file or directorygccg++on code that includes<linux/types.h>llvmclang h.cunable to execute command: posix_spawn failedat the link steprustrustc h.rslinker 'cc' not foundThe assembler and linker come from
binutilsand the kernel headers fromlinux_headers, and neither is in these packages' runtime closures.gccalso installs nocc, andccis the linkerrustcinvokes by default. Theruststack already works around all of this: it addsgcc,binutilsandlinux_headersand sets the linker togcc. Packages can't set environment variables, so the package fix has to supply a realcc.Change
gccbinutilsandlinux_headerstoruntime_deps.build.shinstallsusr/bin/cc -> gcc. The existingusr/bin/*glob covers it. gcc already installsc++, so onlyccwas missing.binutilsto theirtest_deps(that was theTODO: Some of these need to be runtime_deps). A new test builds and runs a C program withccand a C++ program that includes<linux/types.h>withg++, using onlybash-bootstrapandcoreutilsastest_deps.llvmbinutils(clang's driver links throughld) andlinux_headerstoruntime_deps.<linux/types.h>withclang.rusttoolchaintoruntime_deps. This is the same C toolchain set theruststack adds (gcc, which now providescc, plus binutils, glibc and linux_headers), sorustc's defaultcclinker resolves without-C linker=gcc.toolchainmoves out ofbuild_deps, because runtime deps are already injected into the build.rustc.Design note:
ccTwo alternatives were considered and rejected:
rust.default-linker = "gcc"inbootstrap.toml. rustc would still need a C toolchain at runtime, and cargo-driven C builds (thecccrate) would still look forcc.ccshim intoolchain. Agcc-only box would still lackcc.A
ccsymlink ingccfixes all three cases with one line.Things a reviewer should weigh
binutilsalready runtime-depends onsubsetOf gcc ["libstdcpp", "libgcc"], so this change adds the cycle gcc → binutils → gcc. The planner handles it: bothgccandbinutilsdeclare areplace_on_cycle. Locally,min checkpasses and a branch session resolves and installs the new gcc closure. CI's full-graph build is the authoritative proof.subsetOf gccusers. AsubsetOfdependency inherits the source package's fullruntime_deps; it already does this for gmp, mpc, mpfr, zlib and zstd. About 128 packages runtime-depend onsubsetOf gcc [...]to getlibgcc_sorlibstdc++, so their runtime closures now also gainbinutilsandlinux_headers. That adds about 28 MiB for binutils and about 6.5 MiB for the headers, measured withdu --apparent-sizeon arm64. Sessions already includegcc, so they barely change, but the closures of packaged programs grow. If that is not acceptable, the alternative is to splitlibgcc/libstdc++into their own package, or to fall back to pointing gcc-only users attoolchain. Either is a larger change than this PR.Test plan
Run on an arm64 Minimal VM (macOS host, 10 vCPU, 16 GiB), in sessions synced from this branch:
main:gcconly:asnot found, and<linux/types.h>not found.llvmonly: clang fails at link (posix_spawn failed), and clang++ can't find<linux/types.h>.rustonly:linker 'cc' not found.main:gcc+binutils+linux_headers:gcc h.candg++with<linux/types.h>build and run.llvm+binutils+linux_headers:clang h.candclang++with<linux/types.h>build and run.rust+toolchain:rustc h.rsstill fails oncc. With acc -> gccsymlink onPATH, plainrustc h.rsbuilds and the binary printshi.min check --packages gcc,llvm,rust,toolchain,binutilspasses, including the output checks forgccafter its build.min package patched-build gccsucceeds. In a fresh branch session,/usr/bin/cc -> gcc,as,ldand/usr/include/linux/types.hare present without addingbinutilsorlinux_headers.gcc h.c,cc h.candg++with<linux/types.h>all build and run.min package patched-build rustsucceeds withtoolchainmoved toruntime_deps.llvmbuild, and installing the rebuiltrust. The gcc change re-keysllvm-bootstrap, whichllvmandrustneed. Its rebuild was OOM-killed in the 16 GiB VM (g++: fatal error: Killed signal terminated program cc1plus). That is a memory limit of this VM, not a compile error. Somin package patched-build llvmcouldn't run, andmin add --session rustcouldn't install the rebuilt rust. Theclangandrustcresults above come from the equivalent-closure runs, not from the rebuilt packages.testsblocks (there's no in-session test runner), and amd64. CI is the first run of both.Refs gominimal/inbox#843
Summary by CodeRabbit
cccommand as an alternative way to invoke its C compiler.