-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add android support, that uses new swift-foundation for Android #5024
Changes from 5 commits
9b2dde9
7f4bf4b
5f40cce
e505ccf
90c4f5c
70ce513
1da58eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,8 @@ | |
|
||
#if canImport(Glibc) | ||
import Glibc | ||
#elseif canImport(Android) | ||
import Android | ||
#endif | ||
|
||
#if os(Windows) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,8 @@ import struct WinSDK.HANDLE | |
|
||
#if canImport(Darwin) | ||
import Darwin | ||
#elseif canImport(Android) | ||
import Android | ||
#endif | ||
|
||
internal import Synchronization | ||
|
@@ -940,6 +942,13 @@ open class Process: NSObject, @unchecked Sendable { | |
var spawnAttrs: posix_spawnattr_t? = nil | ||
#else | ||
var spawnAttrs: posix_spawnattr_t = posix_spawnattr_t() | ||
#endif | ||
#if os(Android) | ||
guard var spawnAttrs else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't make any sense, as it is set to Maybe you needed this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, you're correct! It looks like I can't get rid of it yet, but the nullability on the NDK is broken for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, let me know when you get that annotation change up, as I will need it too. I will follow this pull up with my own changes later today, mostly minor stuff like importing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hyp, any progress with that annotation change? I'd like to delete this block in this repo, like I have to do on my Android CI. |
||
throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno), userInfo: [ | ||
NSURLErrorKey:self.executableURL! | ||
]) | ||
} | ||
#endif | ||
try _throwIfPosixError(posix_spawnattr_init(&spawnAttrs)) | ||
try _throwIfPosixError(posix_spawnattr_setflags(&spawnAttrs, .init(POSIX_SPAWN_SETPGROUP))) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,8 @@ import FoundationNetworking | |
#endif | ||
#if os(Windows) | ||
import WinSDK | ||
#elseif os(Android) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind, I modified this in my pull hyp#1 instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change can still be made. |
||
import Android | ||
#endif | ||
|
||
enum HelperCheckStatus : Int32 { | ||
|
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.
I think that we should consider doing this via the build configuration.
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.
I think we've been doing this in the headers because build configuration is spread across several systems (cmake, SPM), and they do not all support the same features.
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.
Instead of setting this separately here, can you just add
TARGET_OS_ANDROID
to the list below? That's how I do it on my CI.