FreeBSD: Gate GNU-only API #1183
Conversation
etcwilde
requested review from
briancroom,
grynspan,
stmontgomery and
suzannaratcliff
as code owners
grynspan
left a comment
grynspan
left a comment
There was a problem hiding this comment.
I thought the module was supposed to be named FreeBSD since they don't use Glibc? Ah well.
grynspan
added
bug
|
@swift-ci test |
Yes, we should create a new module and module map for FreeBSD and call it something else, but since everything piggybacks on the Glibc module map, it was convenient to avoid re-writing the |
|
Holding off on merging until we know if we need to make OpenBSD changes too. |
FreeBSD and OpenBSD currently use the Glibc modulemap to import the C runtimes so `canImport(Glibc)` returns true. These systems are not actually glibc though and do not have the `gnu_get_libc_version` extension, resulting in build failures. Gating the use of the API on more than glibc to allow these platforms to compile.
etcwilde
force-pushed
the
ewilde/gate-gnu-api
branch
from
33f4b47 to
0895ff6
Compare
| // and https://www.austingroupbugs.net/view.php?id=411). | ||
| _ = posix_spawn_file_actions_adddup2(fileActions, fd, fd) | ||
| #if canImport(Glibc) | ||
| #if canImport(Glibc) && !os(FreeBSD) && !os(OpenBSD) |
There was a problem hiding this comment.
What do you think of some DeMorgan? && !(os(FreeBSD) || os(OpenBSD))
There was a problem hiding this comment.
It's equivalent. ¯\_(ツ)_/¯
I don't have a preference for or against it. I'm not sure it substantially helps readability here though.
There was a problem hiding this comment.
What's here is how I would have typed it. Although now it's annoying me and I'd probably rewrite the whole line as #if SWT_USES_GLIBC and define that in CMake and SPM as "the target is Linux". At least until we fix the name of the module elsewhere.
There was a problem hiding this comment.
Then you might have issues for musl platforms :)
In any event: given the 6.2 branch has this problem as well, I'd suggest improving the form of the conditional in a separate pr may be more pragmatic, however.
There was a problem hiding this comment.
With CMake, we can ask it to tell us if the symbol is available and define the macro that way. https://cmake.org/cmake/help/v4.1/module/CheckSymbolExists.html
There was a problem hiding this comment.
It's probably not worth the effort—this is mostly me griping about the module name.
| // and https://www.austingroupbugs.net/view.php?id=411). | ||
| _ = posix_spawn_file_actions_adddup2(fileActions, fd, fd) | ||
| #if canImport(Glibc) | ||
| #if canImport(Glibc) && !os(FreeBSD) && !os(OpenBSD) |
There was a problem hiding this comment.
Then you might have issues for musl platforms :)
In any event: given the 6.2 branch has this problem as well, I'd suggest improving the form of the conditional in a separate pr may be more pragmatic, however.
|
@swift-ci test |
|
@etcwilde Thus is ready to squashmerge, yes? |
|
Merged, now to pick some cherries. |
The FreeBSD builds are currently using the GlibC modulemap to import the C runtimes. FreeBSD does not have `gnu_get_libc_version` resulting in build failures. The use of this API was introduced in swiftlang#1147 (cherry picked from commit 79c22ad) - **Explanation**: The FreeBSD builds are currently using the GlibC modulemap to import the C runtimes. FreeBSD does not have `gnu_get_libc_version` resulting in build failures. - **Scope**: Build failure on platforms using Glibc modulemap that don't have GNU extensions. (FreeBSD, OpenBSD) - **Issues**: swiftlang#1193 - **Original PRs**: swiftlang#1183 - **Risk**: Low risk. Removes use of unavailable API. - **Testing**: Built swift-testing on FreeBSD and Linux. - **Reviewers**: @grynspan @3405691582 Fixes: swiftlang#1193
The FreeBSD builds are currently using the GlibC modulemap to import the C runtimes. FreeBSD does not have `gnu_get_libc_version` resulting in build failures. The use of this API was introduced in #1147 (cherry picked from commit 79c22ad) - **Explanation**: The FreeBSD builds are currently using the GlibC modulemap to import the C runtimes. FreeBSD does not have `gnu_get_libc_version` resulting in build failures. - **Scope**: Build failure on platforms using Glibc modulemap that don't have GNU extensions. (FreeBSD, OpenBSD) - **Issues**: #1193 - **Original PRs**: #1183 - **Risk**: Low risk. Removes use of unavailable API. - **Testing**: Built swift-testing on FreeBSD and Linux. - **Reviewers**: @grynspan @3405691582 Fixes: #1193
6 participants
The FreeBSD builds are currently using the GlibC modulemap to import the C runtimes. FreeBSD does not have
gnu_get_libc_versionresulting in build failures.The use of this API was introduced in #1147