-
Notifications
You must be signed in to change notification settings - Fork 644
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
Changes to support building with Musl #2595
Conversation
055e9e9
to
6b66587
Compare
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.
LGTM but would like someone else to take another look as well
@swift-server-bot add to allowlist |
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.
LGTM!
dependencies: [] | ||
dependencies: [], | ||
cSettings: [ | ||
.define("_GNU_SOURCE"), |
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.
Are we confident we're allowed to do this? Previously this has appeared to conflict with Swift's glibc module, which was declared without this definition.
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.
Hmmm. Well, we're definitely not safe just #define
ing it in a source file, because with modular includes you have to define it on the command line (or it won't affect the modularized headers). If it doesn't work with the Glibc module, that would be… bad.
6b66587
to
ae45493
Compare
The CI seems to be failing with
This might just be a problem for the syscall wrapper tests. @al45tair you mind taking a look? |
Will do. I also need to have another look at the |
Update: the crash wasn't that problem (it was another problem, in a different dependency), and everything seems to work without the type information being mentioned explicitly. |
ae45493
to
57719ed
Compare
I think I have a fix for that. Seems there are some extra |
Define `_GNU_SOURCE` in the `Package.swift` rather than in `shim.c` (this is required because `_GNU_SOURCE` affects modular headers). Add an import for Musl to IO.swift. Add code to disable `SIGPIPE` to `SocketProtocols.swift`. Remove types from a pile of functions in `System.swift`; Swift will use the correct type automatically (except in cases where there are multiple versions of a function, e.g. `ioctl()`, in which case we need to be explicit which one we mean).
A couple of the integration tests grab code and build it outside of the normal `Package.swift`, so they needed fixing to define `_GNU_SOURCE` themselves.
c9bf5c9
to
bac54fa
Compare
@swift-server-bot test this please |
Various changes to allow building NIO for a Linux system that is using Musl instead of Glibc.
Motivation:
We would like to be able to build NIO on top of Musl, as well as the usual Glibc.
Modifications:
Define
_GNU_SOURCE
in thePackage.swift
rather than inshim.c
(this is required because_GNU_SOURCE
affects modular headers).Add an import for Musl to IO.swift.
Add code to disable
SIGPIPE
toSocketProtocols.swift
.Remove types from a pile of functions in
System.swift
; Swift will use the correct type automatically (except in cases where there are multiple versions of a function, e.g.ioctl()
, in which case we need to be explicit which one we mean).Result:
NIO should build against Musl.