Repository navigation
build(cmake): stop compiling routing/mathopt sources twice - #1960
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/cuopt/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughCMake now builds routing and mathopt through hidden-visibility object libraries. Shared targets receive their dependencies during assembly. Static builds embed the component objects directly. ChangesComponent build restructuring
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
cuopt_routing and cuopt_mathopt were built directly from source, while cuopt_objs separately recompiled the same files (CUOPT_ROUTING_SRC_FILES + CUOPT_MATHOPT_SRC_FILES) to feed cuopt_static. Give routing and mathopt the same OBJECT-library treatment cuopt_client already has (cuopt_routing_objs / cuopt_mathopt_objs), compiled once and reused by both the shared component library and cuopt_static. Removes cuopt_objs. Also fixes an ordering bug: CUDSS_MT_LIB_FILE_NAME was recomputed (basename -> full path, NVIDIA#1883) after cuopt_mathopt's own target_compile_definitions had already captured the stale value. Verified locally: full build succeeds, ninja target count drops 631 -> 350. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
8a25c92 to
f9c346d
Compare
CI Test Summary✅ All 32 test job(s) passed. |
|
/merge |
cuopt_routing and cuopt_mathopt were compiled directly from source, while cuopt_objs separately recompiled the same files a second time just to feed cuopt_static. Gives routing/mathopt the same OBJECT-library treatment cuopt_client already uses, so each domain compiles once and cuopt_static links the objects instead of rebuilding them. Removes cuopt_objs.
Also fixes an ordering bug this touched:
CUDSS_MT_LIB_FILE_NAMEwas recomputed aftercuopt_mathopt's compile definitions had already captured the stale value.Verified locally: full build succeeds, ninja target count drops 631 → 350.
Verified in CI (
conda-cpp-build / 13.3.0, amd64): 631 → 474 targets, 0 errors, 0 failed distributed compilations.🤖 Generated with Claude Code