Dynamic selection for more Apple platforms#167
Conversation
…system - Supports more Apple platforms, including future ones, which will always support std::filesystem, like with visionOS - Simplifies cases. Undefined preprocessor values are guaranteed to default evaluate to 0 - Moves <Availability.h> include inside conditional, avoiding the include where not needed. - Removes reference to wchar for std headers on windows, which seemed to be out of date, since the define it might have been referring to was commented - (And some other small things.)
This makes project integration more flexible, allowing the drag-contents-of-directory project integration contemplated in the readme and allowing use via -iquote
Reduces duplication and tendency to get out of sync. As evidence of the problem, the snippet had previously (before this PR) started to diverge. It seemed more prudent to delete than to fix, given the usage instructions seem to be centralized in the readme and the dynamic selection headers should probably be recommended anyway.
Two changes: - (minor) Rename GHC_OS_MACOS -> GHC_OS_APPLE, since it is defined all apple platforms (iOS, watchOS, etc.), not just macOS. - Changed the preprocessor conditional in last_write_time to align with its presumed intent. Previously, it would always have been true, which can't be intentional, because the *_OS_VERSION_MIN_REQUIRED is undefined and thus zero for platforms besides the current one, and therefore less than the constants--except for on very old SDKs where, e.g., MAC_OS_X_VERSION_10_13 and others would be undefined and therefore 0 and therefore making the clause false when it needed to be true. Therefore, I changed the conditions to be parallel to those in the dynamic selection headers, checking for the undefined, zero case and hardcoding the version values to support old SDKs.
cpsauer
changed the title
0ac79ff to
48d46cc
Compare
|
Hi Christopher, |
|
(Any idea what's up with the (delayed) failures? Look like instant-failures, with no log, and zero elapsed time.) |
|
(Oh, some sort of CI issue? Looks like it's the same at HEAD.) |
|
Yeah, afaik the support for 18.04 in the workflows was dropped and only 20.04 and 22.04 are currently supported, so I need to update the build configuration. |
|
Makes sense! Just wanted to make sure there wasn't something I needed to fix that'd be blocking you from merging. |
|
Okay, looks good to me, thank you again for the PR! |
|
Thanks so much for vetting! I really appreciate your being great about it. |
|
I'll next spin up some Bazel build rules to make using this great library of yours maximally easy for Bazel users. (We'll need to do it anyway internally and I bet it'll help others. And since Bazel has easy remote download and compile caching it makes sense to do a little wrapping.) Would you be down to take a PR adding a one-liner to the README, linking Bazel users over to the setup code that'll make setup easy for them? |
2 participants
Hello Steffen!
Thanks so much for a wonderful library!
I'm moving my team's codebase over to your library from boost::filesystem--as a backport of std::filesystem on old Apple OSs. I love that yours is so much more compatible with std::filesystem and will try to get libCPR to move next. Along the way, I added conditionals for the other Apple OSs and fixed some things I saw, trying to propose ways to leave the code even better than I found it.
I'm hoping you might be willing to upstream the changes. The fastest review is probably to go commit-by-commit, since I tried to explain what I was up to in each commit message and keep changes independent, while batching them into one PR for fast review. But it might also be pretty fast to just skim the whole thing; there's nothing too crazy here.
Thanks again for all you do!
Tschüß,
Christopher
P.S. I've given you edit access, so feel free to just directly change anything you'd like differently!