-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[libc++] Sync the filesystem clock_gettime handling with chrono for OpenBSD #92675
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
Conversation
…penBSD Copy the same OS checks for clock_gettime from chrono to filesystem and tweak the comment as Hurd is in the same situation.
@llvm/pr-subscribers-libcxx Author: Brad Smith (brad0) ChangesCopy the same OS checks for clock_gettime from chrono to filesystem and Full diff: https://github.com/llvm/llvm-project/pull/92675.diff 2 Files Affected:
diff --git a/libcxx/src/chrono.cpp b/libcxx/src/chrono.cpp
index e7d6dfbc22924..e8ebb67284bbb 100644
--- a/libcxx/src/chrono.cpp
+++ b/libcxx/src/chrono.cpp
@@ -31,8 +31,8 @@
# include <sys/time.h> // for gettimeofday and timeval
#endif
-// OpenBSD does not have a fully conformant suite of POSIX timers, but
-// it does have clock_gettime and CLOCK_MONOTONIC which is all we need.
+// GNU Hurd and OpenBSD do not implement a fully conformant suite of POSIX timers, but
+// they do have clock_gettime and CLOCK_MONOTONIC which are all we need.
#if defined(__APPLE__) || defined(__gnu_hurd__) || defined(__OpenBSD__) || (defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0)
# define _LIBCPP_HAS_CLOCK_GETTIME
#endif
diff --git a/libcxx/src/filesystem/filesystem_clock.cpp b/libcxx/src/filesystem/filesystem_clock.cpp
index e13b2853e367c..a120be14978a3 100644
--- a/libcxx/src/filesystem/filesystem_clock.cpp
+++ b/libcxx/src/filesystem/filesystem_clock.cpp
@@ -29,7 +29,9 @@
# include <sys/time.h> // for gettimeofday and timeval
#endif
-#if defined(__APPLE__) || defined(__gnu_hurd__) || (defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0)
+// GNU Hurd and OpenBSD do not implement a fully conformant suite of POSIX timers, but
+// they do have clock_gettime and CLOCK_MONOTONIC which are all we need.
+#if defined(__APPLE__) || defined(__gnu_hurd__) || defined(__OpenBSD__) || (defined(_POSIX_TIMERS) && _POSIX_TIMERS > 0)
# define _LIBCPP_HAS_CLOCK_GETTIME
#endif
|
In general we like to have a CI runner to test platform specific behaviour. Do you intent to add a GNU Hurd runner? (This change is small and innocent, but without CI tests there is no guarantee there won't be regressions.) Can you make the commit message a bit more descriptive. Something along the lines of |
Gentle ping -- I'll close this as inactive if there is no plan to officially support this platform. |
I'd like to have official support for the platform. But looking at the current code and support policy this will consistently be an issue for us. Our current Clang / libc++ is 16. |
@brad0 I'll need to forward-port some patches just to get the bare minimum of simple executables able to run. Maybe that is good enough even if OpenBSD is still officially hanging back on LLVM 16 for a while longer? OpenBSD is a faster-moving ABI with the security experiments (and I would not want that to change!), but I think it should be possible to have less outsanding patches against upstream without compromising that ABI agility or incurring painful upstreaming drudgery. |
What kind of downstream changes do you folks have? We may be able to accommodate some of them or at least make them easier to maintain by introducing a few "extension points" in the library. This is a common situation and we're quite used to finding ways to make it work both for the upstream project and for downstream needs. |
@ldionne I am learning as I go, as my cross compiled binaries do not yet work :). First on my docket is that LLD needs some linker script modifications so that special sections like The change that currently does this just uses To be clear, I haven't seen any critical downstream modification in libc++ itself yet, but I would consider LLD / Clang / LLVM changes blockers insofar is if binaries don't run, then the libc++ testsuite cannot run --- unless you would consider a "does it at least build" smoke test a decent first step. |
For libc++. Nothing. Various local patches for Clang, LLVM and LLD though. |
@Ericson2314 @brad0 I think that to begin with, we could start with only compiling the test suite. We could at least see how far we're able to go in that direction. I really want to avoid having "phantom" platforms in the code base (where we have 0 visibility into what's working and what's not working). |
I can provide support for adding a new OpenBSD build bot if you want. The documentation for adding a new CI job using BuildKite is here: https://libcxx.llvm.org/AddingNewCIJobs.html#addingnewcijobs It's also possible to set up a custom Github Actions runner if you prefer that, either way is fine by us. |
There is not a Buildkite binary for OpenBSD. But it built Ok. I'll see about creating a package. I'll be able to try setting something up around the end of the month. |
Ack. Please let me know when/whether I can provide support. I will set up a reminder to ping this at the end of the month. I would aim to have this done in time for the LLVM 20 release. In terms of process, if we are not able to set this up in time for LLVM 20, I would remove patches that intend to provide support for OpenBSD until we are able to actually provide support for it, to avoid an accumulation of half-supported things in the codebase. |
I'll close this for the time being but let's reopen it once we have what we need CI-wise. |
Scheduled ping. Any interest for setting up CI? Should I start cleaning up OpenBSD support from the upstream tree? |
I have the system. I just got the OS installed and it's ready to go. |
Can you reach out to me on Discord |
@ldionne I have. You don't respond. |
Sorry, new message requests on Discord end up in a weird place, I missed it but I see it now. |
Copy the same OS checks for clock_gettime from chrono to filesystem and
tweak the comment as Hurd is in the same situation.