Add support for Android #183
Conversation
|
@swift-server-bot test this please |
|
Great! Thank you! |
|
CI appears to have issues, as this change is compiled out on non-Android and will have no effect on the CI. |
|
5.8, 5.9 and 5.10 fail because allocations have improved which I can fix real quick #184 However, nightly unit tests fail and eventually time out: https://ci.swiftserver.group/job/swift-certificates-swift-nightly-prb/542/console cc: @Lukasa have you seen this before? |
|
@swift-server-bot test this please |
|
Hold off, I'm seeing issues with this when building with latest trunk. |
Pull request was converted to draft
| @@ -23,6 +23,9 @@ import Musl | |||
| import CoreFoundation | |||
There was a problem hiding this comment.
I'm a bit confused by whether these imports need to be there at all, as I've been seeing changes in what the trunk compiler I build natively on Android expects here in the last month.
To compare to other platforms, I commented out the imports of CoreFoundation and the three linux C libraries in this file- Bionic, Musl, and Glibc- and tried building the latter two on Fedora 40, where both compiled fine without these imports (Musl was built with the latest July 2 SDK snapshot, Glibc starting from the latest trunk snapshot toolchain back to 5.9.2). This file uses the inet_pton() C function, which clearly comes from those C libraries.
The issue is that a trunk source snapshot of the compiler from June and July built natively on Android worked the same, ie these two imports were unnecessary, but an August trunk snapshot now requires import Android.
I don't know if something changed in the compiler's ClangImporter or LLVM in the last month, but the Android module map is identical from the July trunk snapshot to August.
Anyone know what is going on here, if not with my Android issue, at least why these imports seem unneeded for Glibc/Musl?
@ian-twilightcoder, @compnerd, @etcwilde, @al45tair, perhaps one of you knows what's going on with these overlay imports.
There was a problem hiding this comment.
The glibc modulemap/header thing that we have today isn't properly modularizing glibc. Instead of using that mega header, each of the top-level C headers should end up in its own top-level module so if that header is seen through a transitive include, that's the module definition that will get used. I don't know what changed specifically, but what we have was technically undefined, so we're still in the wrong with glibc. It just means that something pulled in something from CoreFoundation that pulled in the necessary header before something else pulled in the bit of glibc/bionic that pulled in that same header, which ends up with CoreFoundation saying that it owns the canonical definition of that thing rather than the libc. We ran into this in corelibs-foundation as well: swiftlang/swift-corelibs-foundation#5005
The musl modulemap should be better behaved though as I seem to remember Alastair putting quite a lot of work to modularize it correctly. @ian-twilightcoder can probably give you more details though on how to proceed.
There was a problem hiding this comment.
I tried a little experiment to see if these libc imports are needed at all, deleting every import Musl and import Glibc in the swift-nio repo and then trying to build with each libc on linux with the July 2 trunk snapshot toolchain, as that's the latest Musl SDK available.
The good news is that caused a bunch of errors, so I then put back each import one by one until I was left with this cleaned-up patch, ie it still built fine with these imports deleted, even running and passing all the same tests for the Glibc build:
diff --git a/Sources/NIOFileSystem/FileSystem.swift b/Sources/NIOFileSystem/FileSystem.swift
index 291102d0..d158c41d 100644
--- a/Sources/NIOFileSystem/FileSystem.swift
+++ b/Sources/NIOFileSystem/FileSystem.swift
@@ -21,10 +21,6 @@ import NIOPosix
#if canImport(Darwin)
import Darwin
-#elseif canImport(Glibc)
-import Glibc
-#elseif canImport(Musl)
-import Musl
#elseif canImport(Bionic)
import Bionic
#endif
diff --git a/Sources/NIOFileSystem/FileSystemError+Syscall.swift b/Sources/NIOFileSystem/FileSystemError+Syscall.swift
index 633b6dc7..377203d3 100644
--- a/Sources/NIOFileSystem/FileSystemError+Syscall.swift
+++ b/Sources/NIOFileSystem/FileSystemError+Syscall.swift
@@ -17,10 +17,6 @@ import SystemPackage
#if canImport(Darwin)
import Darwin
-#elseif canImport(Glibc)
-import Glibc
-#elseif canImport(Musl)
-import Musl
#endif
extension FileSystemError {
diff --git a/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift b/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift
index 9212839c..be4ba011 100644
--- a/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift
+++ b/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift
@@ -18,11 +18,7 @@ import SystemPackage
#if canImport(Darwin)
import Darwin
-#elseif canImport(Glibc)
-import Glibc
-import CNIOLinux
-#elseif canImport(Musl)
-import Musl
+#elseif canImport(Glibc) || canImport(Musl)
import CNIOLinux
#elseif canImport(Bionic)
import Bionic
diff --git a/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift b/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift
index 0a80b9f1..d738d856 100644
--- a/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift
+++ b/Sources/NIOFileSystem/Internal/System Calls/Syscall.swift
@@ -18,11 +18,7 @@ import SystemPackage
#if canImport(Darwin)
import Darwin
import CNIODarwin
-#elseif canImport(Glibc)
-import Glibc
-import CNIOLinux
-#elseif canImport(Musl)
-import Musl
+#elseif canImport(Glibc) || canImport(Musl)
import CNIOLinux
#elseif canImport(Bionic)
import Bionic
diff --git a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift
index 41406573..83cc013b 100644
--- a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift
+++ b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift
@@ -20,11 +20,7 @@ import NIOPosix
#if canImport(Darwin)
import Darwin
-#elseif canImport(Glibc)
-import Glibc
-import CNIOLinux
-#elseif canImport(Musl)
-import Musl
+#elseif canImport(Glibc) || canImport(Musl)
import CNIOLinux
#elseif canImport(Bionic)
import Bionic
diff --git a/Sources/NIOPosix/VsockAddress.swift b/Sources/NIOPosix/VsockAddress.swift
index 94d0f797..e08725c3 100644
--- a/Sources/NIOPosix/VsockAddress.swift
+++ b/Sources/NIOPosix/VsockAddress.swift
@@ -17,11 +17,6 @@ import NIOCore
#if canImport(Darwin)
import CNIODarwin
#elseif os(Linux) || os(Android)
-#if canImport(Glibc)
-import Glibc
-#elseif canImport(Musl)
-import Musl
-#endif
import CNIOLinux
#endif
private let vsockUnimplemented = "VSOCK support is not implemented for this platform"
The bad news is that I don't know why some of these are not needed. The imports in NIOPosix/VsockAddress.swift aren't needed because one of the Glibc imports in that module is @_exported, so that appears to cover all files in that module. I presume the same for NIOFileSystem/Internal/SystemFileHandle.swift and NIOFileSystem/FileSystem.swift, which both explicitly import NIOPosix. The other three are a mystery.
Pinging @ian-twilightcoder, perhaps you know what's going on here.
There was a problem hiding this comment.
Hmm, maybe this is related to the current leaks in the module system, such as for member declarations that is currently being fixed, so given Ian hasn't responded, I will just change this so the compiler won't error, and we can merge and then revisit it later, once we know the Swift module leaks have been fixed.
There was a problem hiding this comment.
I'm guessing this is related to the clang re-export leak that Allan wrote up on the forum.
|
Updated this pull to import the Android overlay instead and rebased, ready for review. |
|
@swift-server-bot add to allowlist |
There was a problem hiding this comment.
Easiest PR ever, thanks @finagolfin, and thanks for your attention to detail here as well, it's much appreciated.
Tested natively on Android AArch64 with a recent trunk Swift toolchain, no problems.