Skip to content

GH-49176: [C++] CRAN build fail on missing std::floating_point concept #49221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jonkeane wants to merge 3 commits into
base: main
Choose a base branch
from
+41 −3
Open

GH-49176: [C++] CRAN build fail on missing std::floating_point concept #49221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
751daf0
macos 11 sdk from jonkeane/crossbow
jonkeane Feb 13, 2026
3cf3c71
Workaround on older partially compliant C++20 implementations
jonkeane Feb 11, 2026
08daa2e
linting
jonkeane Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cpp/src/arrow/compute/kernels/hash_aggregate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,16 @@ template <typename T>
concept CBooleanConcept = std::same_as<T, bool>;

// XXX: Ideally we want to have std::floating_point<Float16> = true.
// Some older standard library implementations (e.g., macOS 11.x libc++) have partial
// C++20 concepts support with std::same_as but lack std::floating_point.
#if defined(__cpp_lib_concepts) && __cpp_lib_concepts >= 202002L
template <typename T>
concept CFloatingPointConcept = std::floating_point<T> || std::same_as<T, util::Float16>;
#else
template <typename T>
concept CFloatingPointConcept =
std::is_floating_point_v<T> || std::same_as<T, util::Float16>;
#endif

template <typename T>
concept CDecimalConcept = std::same_as<T, Decimal32> || std::same_as<T, Decimal64> ||
Expand Down
36 changes: 33 additions & 3 deletions dev/tasks/r/github.macos.cran.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@

jobs:
macos-cran:
name: "macOS similar to CRAN"
name: "macOS {{ '${{ matrix.config }}' }}"
runs-on: macOS-latest
strategy:
fail-fast: false
matrix:
config: ["cran-m1", "cran-release"]

steps:
{{ macros.github_checkout_arrow()|indent }}
Expand Down Expand Up @@ -58,7 +60,35 @@ jobs:
extra-packages: |
any::rcmdcheck
any::sys
- name: Install
- name: Install MacOSX 11.3 SDK
if: matrix.config == 'cran-release'
env:
SDK_TOKEN: {{ '${{ secrets.JONKEANE_MACOS_11_SDK_DOWNLOAD_TOKEN }}' }}
run: |
# Download, Confirm integrity, expand. This will fail if the hash does not match.
curl -fsSL -H "Authorization: Bearer $SDK_TOKEN" \
-H "Accept: application/vnd.github+json" \
https://api.github.com/repos/jonkeane/crossbow_11_sdk/tarball/v0.0.1 \
-o /tmp/MacOSX11.3.sdk.tar.gz
echo "493570e56d6c6af26128e9096de738822589cc3cdb1b29aa5854f3f4c99756ac /tmp/MacOSX11.3.sdk.tar.gz" | shasum -a 256 -c -
sudo tar -xzf /tmp/MacOSX11.3.sdk.tar.gz -C /Library/Developer/CommandLineTools/SDKs/
# Move SDK from extracted folder (GitHub archives as {owner}-{repo}-{sha}/)
sudo mv /Library/Developer/CommandLineTools/SDKs/jonkeane-crossbow_11_sdk-*/MacOSX11.3.sdk \
/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk
sudo rm -rf /Library/Developer/CommandLineTools/SDKs/jonkeane-crossbow_11_sdk-*
ls -la /Library/Developer/CommandLineTools/SDKs/
Comment on lines 67 to 79
Copy link
Member Author

@jonkeane jonkeane Feb 11, 2026

Choose a reason for hiding this comment

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

Downloading this from this github site is very funky, and honestly a little sketchy. But it does actually replicate the very old macosx sdks that CRAN is building arrow with. And we can replicate it in CI

Copy link
Member

@pitrou pitrou Feb 11, 2026

Choose a reason for hiding this comment

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

Do you have a timeline on when we can remove this?

Copy link
Member Author

@jonkeane jonkeane Feb 11, 2026

Choose a reason for hiding this comment

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

The R-devel runners are already moved to a newer version of the SDK, and R4.6 should be released in April (historically that's when that happens, but technically there isn't a set deadline). I don't yet know if that will mean all of CRAN's builders will have that new SDK though. I imagine they will move forward soon given how old these are, but in that thread there is a stated desire to keep supporting macOS + x86, which will

FWIW: I am honestly moderately uncomfortable with this, even in a CI job that has restricted permissions, etc. etc. But I haven't found any other way to link against the old SDKs that CRAN is using. We could make this something that is only run manually (though there is at least one other issue that has been merged to main that the old SDKs aren't good with: #49223 / #49105

I wonder if from ^^^ you have any ideas about how else we might catch when we need a fallback?

Copy link
Member

@pitrou pitrou Feb 11, 2026

Choose a reason for hiding this comment

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

cc @kou about the CI aspect of this

Copy link
Member Author

@jonkeane jonkeane Feb 11, 2026

Choose a reason for hiding this comment

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

Not sure if this is helpful, but I did find https://developer.apple.com/xcode/cpp/#c++20 which lists when some of these features were supported (though not particularly consistently!) and once we are on xcode 14, it looks like we should be clear.

Copy link
Member

@pitrou pitrou Feb 12, 2026

Choose a reason for hiding this comment

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

Do we have some place we could host this privately (but still with access for our CI runners)? I'm happy to do that temporarily, but don't want it to be out and available on the open internet if possible. I would consider a private repo alongside usracomputing/crossbow, but I don't have access to that anymore. Do you know who does?

Perhaps @rok @raulcd @assignUser have ideas

Copy link
Member

@rok rok Feb 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Does this need to be private? We have s3://arrow-data for things like nightly report.
We can make s new s3 bucket that's only accessible to our (self-hosted) CI runners?

Copy link
Member Author

@jonkeane jonkeane Feb 12, 2026

Choose a reason for hiding this comment

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

Do we have any macos self-hosted runners? If yes, then just let me know where to put it and how to point this CI at those and I'll make it so...

Copy link
Member Author

@jonkeane jonkeane Feb 13, 2026

Choose a reason for hiding this comment

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

Ok I have changed this to point to a (private) repo and added a limited PAT to crossbow for just that repo.

Copy link
Member Author

@jonkeane jonkeane Feb 13, 2026

Choose a reason for hiding this comment

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

Oops, sorry when I commented I hadn't pushed yet, but it is now.

- name: Install (cran-release)
if: matrix.config == 'cran-release'
env:
_R_CHECK_CRAN_INCOMING_: false
SDKROOT: '/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk'
NOT_CRAN: false
run: |
sccache --start-server || echo 'sccache not found'
cd arrow/r
R CMD INSTALL . --install-tests
- name: Install (cran-m1)
if: matrix.config == 'cran-m1'
env:
_R_CHECK_CRAN_INCOMING_: false
CXX: "clang++ -mmacos-version-min=14.6"
Expand All @@ -77,6 +107,6 @@ jobs:
- name: Save the test output
uses: actions/upload-artifact@v4
with:
name: test-output
name: test-output-{{ '${{ matrix.config }}' }}
path: arrow-tests/testthat.Rout*
if: always()
Loading