-
Notifications
You must be signed in to change notification settings - Fork 4k
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.
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
base: main
Are you sure you want to change the base?
GH-49176: [C++] CRAN build fail on missing std::floating_point concept #49221
Conversation
|
@github-actions crossbow submit test-r-macos-as-cran |
|
Revision: 5ed5de1 Submitted crossbow builds: ursacomputing/crossbow @ actions-bc3fd4369a
|
|
@github-actions crossbow submit test-r-macos-as-cran |
|
Revision: 1314042 Submitted crossbow builds: ursacomputing/crossbow @ actions-cf07311e56
|
1314042 to
dc11dec
Compare
|
cc @shashbha14 here is a fix (+ a CI job that proves it) |
| run: | | ||
| curl -fsSL https://github.com/phracker/MacOSX-SDKs/releases/download/11.3/MacOSX11.3.sdk.tar.xz -o /tmp/MacOSX11.3.sdk.tar.xz | ||
| sudo tar -xf /tmp/MacOSX11.3.sdk.tar.xz -C /Library/Developer/CommandLineTools/SDKs/ | ||
| echo "Installed MacOSX11.3.sdk to /Library/Developer/CommandLineTools/SDKs/" | ||
| ls -la /Library/Developer/CommandLineTools/SDKs/ |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://download.developer.apple.com/Developer_Tools/Command_Line_Tools_for_Xcode_13.3/Command_Line_Tools_for_Xcode_13.3.dmg is another source of these headers — though this requires an Apple developer account. Maybe the best we can do is instructions (+ Makefile changes, that would work for R, I'm not sure if the C++ dev stack has a good equivalent for adding instructions on how to run these?) that make testing this locally (on a mac) possible. But we loose the catch for these issues until CRAN updates their compilers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have honestly no idea which one is preferrable. Perhaps want to hardcode some kind of sha256 checksum validation against the download? Though of course that wouldn't protect against an already malicious file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or perhaps you can build your own tarball from the aforementioned Apple download link and upload it somewhere for us to use it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way: your Apple link is for 13.3, but we need 11.3, right?
|
Thanks for working through this and wiring up the macOS-as-CRAN CI job, this makes the situation much clearer. |
Rationale for this change
Passing builds on CRAN
What changes are included in this PR?
A workaround for C++20 compatibility issues
Are these changes tested?
Yes, we can ship these tests in this PR or we can keep them in #49216 either is fine by me.
Are there any user-facing changes?
No
std::floating_pointconcept #49176