Skip to content

Fix eigen#163

Merged
poshul merged 9 commits intomasterfrom
fix_eigen
Dec 4, 2025
Merged

Fix eigen#163
poshul merged 9 commits intomasterfrom
fix_eigen

Conversation

@poshul
Copy link
Contributor

@poshul poshul commented Dec 4, 2025

Summary by CodeRabbit

Release Notes

  • Chores
    • Improved Windows build error handling with strict exit code checking.
    • Optimized build dependency sequencing for enhanced reliability.
    • Enhanced compiler configuration and error reporting for better build diagnostics.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This PR adds stricter PowerShell error handling in the Windows contrib build, reorders contrib library build steps to build ARROW earlier (removing a duplicate ARROW block), and updates the MSVC Arrow build to pass ARROW_CXXFLAGS, adjust ZIP args, add CMake flags (including CMAKE_INSTALL_LIBDIR/CMAKE_CXX_FLAGS), and emit build output before fatal errors.

Changes

Cohort / File(s) Summary
Windows Build Error Handling
​.github/workflows/main.yml
Set PowerShell error action with $ErrorActionPreference = "Stop" and added explicit if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } checks after each CMake invocation in the Windows Build contrib step.
Contrib Build Order
CMakeLists.txt
Reordered contrib build blocks to move ARROW earlier, removed duplicate ARROW block, and shifted LIBSVM/XERCES placement relative to ZLIB/BOOST; adjusted final messaging to reflect new ordering.
MSVC Arrow Configuration & Error Handling
libraries.cmake/arrow.cmake
In MSVC branch: changed ZIP_ARGS to include -snld20, introduced ARROW_CXXFLAGS as "/I${PROJECT_BINARY_DIR}/include", added -D CMAKE_INSTALL_LIBDIR=${PROJECT_BINARY_DIR}/lib and -D CMAKE_CXX_FLAGS=${ARROW_CXXFLAGS} to Arrow configure, and print ARROW_BUILD_OUT/ARROW_BUILD_ERR before invoking fatal errors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify reordering does not create unmet or circular build dependencies (CMakeLists.txt).
  • Confirm MSVC ARROW_CXXFLAGS and CMAKE_CXX_FLAGS usage is compatible with Arrow's CMake and does not conflict with other flags (libraries.cmake/arrow.cmake).
  • Validate PowerShell error handling and explicit exit checks behave as intended in CI (​.github/workflows/main.yml).
  • Check the added printing of build output/error does not leak sensitive or overly verbose logs.

Possibly related PRs

Poem

🐰 I hopped through CMake paths today,
PowerShell hushes errors on the way.
Arrow springs up before the rest,
MSVC flags suit it best.
A little rabbit cheers the build bouquet. ✨

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_eigen

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2855524 and 7a90d01.

📒 Files selected for processing (3)
  • .github/workflows/main.yml (1 hunks)
  • CMakeLists.txt (2 hunks)
  • libraries.cmake/arrow.cmake (4 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@poshul poshul merged commit e91abe7 into master Dec 4, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant