fix(link): pass -lm on the Linux application-dylib link - #20
Conversation
LLVM lowers Math.floor/Math.log10 in application code to libm calls, and Perry's shared-library link on Linux does not add -lm the way its executable link does; macOS never notices because libSystem carries libm. The Next fixture's link on Linux died with `undefined reference to 'floor'` after every other symbol resolved (found once PerryTS/perry#8942 removed the duplicate string-constant definitions). The tiny CI fixture calls no libm function, which is why the Linux proof never saw it. Claude-Session: https://claude.ai/code/session_01UZJbhb2FTuakurTHPAKQgd
|
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 provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Linux compiler wrapper now adds ChangesLinux shared-library linking
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This change adds libm when building Linux shared libraries so application math functions link correctly. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ 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 |
LLVM lowers
Math.floor/Math.log10in application code to libm calls (floor,log10). Perry's--output-type dyliblink on Linux does not add-lm(its executable link does —link/build_and_run.rs), and macOS never notices because libSystem carries libm. On the fresh Linux box the Next fixture's link died withafter every other symbol resolved — it surfaced the moment PerryTS/perry#8942 removed the duplicate string-constant definitions. The tiny fixture the Linux proof compiles calls no libm function, which is why CI never saw it.
Coop's
ccshim now passes-lmon the Linux shared-library branch. The compiler-side fix (add the system libraries to Perry's Linux-sharedlink, as its executable link already does) is being added to PerryTS/perry#8942; this shim change is the defensive half and stays correct either way.Verified on the Linux box: with this change (and PerryTS/perry#8942's string-constant fix) the fixture links and publishes (
app.so, 362 s end to end). The three-appin_processtable is not yet possible there: the daemon SIGSEGVs during the app's module init (js_throw → _Unwind_RaiseException, independent of these link flags) — tracked separately.https://claude.ai/code/session_01UZJbhb2FTuakurTHPAKQgd
Summary by CodeRabbit
floorandlog10.