Skip to content

fix(MacOS CI): enable run on Mac OS#874

Merged
yangxk1 merged 4 commits intoapache:mainfrom
SYaoJun:224_mac_ci
Feb 25, 2026
Merged

fix(MacOS CI): enable run on Mac OS#874
yangxk1 merged 4 commits intoapache:mainfrom
SYaoJun:224_mac_ci

Conversation

@SYaoJun
Copy link
Copy Markdown
Contributor

@SYaoJun SYaoJun commented Feb 24, 2026

Reason for this PR

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.21%. Comparing base (95ac39b) to head (d811766).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #874   +/-   ##
=========================================
  Coverage     80.21%   80.21%           
  Complexity      615      615           
=========================================
  Files            93       93           
  Lines         10255    10255           
  Branches       1049     1049           
=========================================
  Hits           8226     8226           
  Misses         1789     1789           
  Partials        240      240           
Flag Coverage Δ
cpp 71.51% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@SYaoJun SYaoJun force-pushed the 224_mac_ci branch 2 times, most recently from 087f998 to 22d7106 Compare February 24, 2026 11:30
@SYaoJun
Copy link
Copy Markdown
Contributor Author

SYaoJun commented Feb 24, 2026

brew list --versions show dependencies version in CI.

@SYaoJun
Copy link
Copy Markdown
Contributor Author

SYaoJun commented Feb 24, 2026

fix: #873

Comment thread cpp/CMakeLists.txt
set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake)
include_directories(${CMAKE_CURRENT_SOURCE_DIR})
add_library(${PROJECT_NAME} INTERFACE)
target_compile_features(${PROJECT_NAME} INTERFACE cxx_std_17)

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables macOS CI builds by introducing dynamic C++ standard selection based on Apache Arrow version. The main issue being addressed is that Arrow 23.0.0+ requires C++20, while earlier versions can use C++17. The changes make the C++ standard configurable and automatically adapt to the Arrow version being used.

Changes:

  • Introduced GAR_CXX_STANDARD variable (default 17) that dynamically updates to 20 when Arrow >= 23.0.0 is detected or built
  • Removed hardcoded -std=c++17 from CMAKE_CXX_FLAGS and interface library target features, replaced with per-target cxx_std_${GAR_CXX_STANDARD} configuration
  • Added brew list --versions to macOS CI workflow for better debugging visibility

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
cpp/CMakeLists.txt Introduces GAR_CXX_STANDARD variable and dynamic C++ standard selection based on Arrow version; updates all target_compile_features to use the variable; removes hardcoded C++17 flags
.github/workflows/ci.yml Adds brew package version listing for debugging in macOS CI

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cpp/CMakeLists.txt
Comment thread cpp/CMakeLists.txt Outdated
Comment thread cpp/CMakeLists.txt Outdated
Comment thread cpp/CMakeLists.txt
Comment thread cpp/CMakeLists.txt Outdated
SYaoJun and others added 2 commits February 24, 2026 23:12
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jason <libevent@yeah.net>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jason <libevent@yeah.net>
Signed-off-by: syaojun <libevent@yeah.net>
Copy link
Copy Markdown
Contributor

@yangxk1 yangxk1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yangxk1 yangxk1 merged commit c7950b0 into apache:main Feb 25, 2026
12 checks passed
yangxk1 pushed a commit to SYaoJun/incubator-graphar that referenced this pull request Feb 25, 2026
* fix(CI): Enable CI run success on Mac OS, change C++ standard based on Apache-Arrow version

* Update cpp/CMakeLists.txt

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jason <libevent@yeah.net>

* Update cpp/CMakeLists.txt

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jason <libevent@yeah.net>

* chore: remove unnecessary comments

Signed-off-by: syaojun <libevent@yeah.net>

---------

Signed-off-by: Jason <libevent@yeah.net>
Signed-off-by: syaojun <libevent@yeah.net>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

4 participants