Port of Foundation to Android #622

johnno1962 · 2016-09-07T10:00:57Z

This PR contains the relatively minor changes to have Foundation compile with the Android NDK. There is an associated pull request with the small change swiftlang/swift#4662 to build-script-impl in the swift project to pass through the required arguments to the ninja scritpt generator. The focus has been on getting the code to compile but preliminary testing of Regular expressions, NSHost, fetching Strings from URLs has been successful so far. There is a known issue with needing to hook thread exit to call JNI method DetachCurrentThread which I’ll have to look at before making a PR on swift-corelibs-libdispatch (resolved, see below).

Issues found and not resolved:
Optimising the creation of a string from zero length data - optimisation patched out for now
Fetching the current Locale language for web fetch request headers - also patched out

Changes tested on Linx and OSX build. Included is a minor change fixing a missing -lcurl when building "util/build-script —foundation" on Linux.

As foundation has binary dependencies on libxml2 and libcurl and it's not been able to automate the libdispatch crosscompile the build is dependent on a pre-built binary & header package downloaded by a series of scripts in the swift-corelibs-foundation/android directory. These binaries were built from the public repos pretty much as-is after some Makefilery. Details are in the README.md in this directory.

modocache · 2016-09-07T12:33:22Z

CoreFoundation/String.subproj/CFString.c

@@ -1264,12 +1264,15 @@ CF_PRIVATE CFStringRef __CFStringCreateImmutableFunnel3(
 	contentsDeallocator = __CFGetDefaultAllocator();
    }

+    #ifndef DEPLOYMENT_TARGET_ANDROID
+    // crashes String.swift, line 51 when creating String from zero length Data


It'd be great to file a https://bugs.swift.org report to track this crash, and include the report number here. That way, we'll know to remove this #ifdef once the report is fixed.

IIRC android's memcpy and memmove are not the same as Darwin. That may be the culprit of this issue.

modocache · 2016-09-07T12:55:03Z

Issues found and not resolved:
Fetching the current Locale language for web fetch request headers - also patched out

I think this might be related?

mekjaer · 2016-09-07T15:24:39Z

Foundation/NSGeometry.swift

+  return floor( Double(value ) )
+}
+private func ceil( value: CGFloat ) -> Double {
+  return floor( Double(value ) )


phausler · 2016-09-07T15:57:14Z

For the Android port does this need to have a JNI_OnLoad or is that entry point higher?

johnno1962 · 2016-09-07T16:06:38Z

The JNI_OnLoad is in https://github.com/SwiftJava/java_swift/blob/master/Sources/JavaJNI.swift#L23 . App itself has second hook to “bind" through a pair of interfaces/protocols https://github.com/SwiftJava/swift-android-samples/blob/master/swifthello/src/main/swift/Sources/main.swift#L22

phausler · 2016-09-07T16:27:36Z

Will that mean we will need a hook in Thread.swift for VM context destruction?

johnno1962 · 2016-09-07T16:33:38Z

Absolutely. Android requires you to detach the thread and won’t allow you to do it with anything on the stack, claiming you are "stilll processing". Some help required here also. I haven’t made much headway understanding libdispatch, kqueues, etc. Threads seem to time out after 15 seconds if that sounds familiar crashing the app. Only way to stop it crashing is to keep making requests on the queue.

parkera · 2016-09-07T20:16:44Z

CoreFoundation/Base.subproj/CoreFoundation_Prefix.h

@@ -203,6 +206,7 @@ strlcat(char * dst, const char * src, size_t maxlen) {
    }
    return dstlen + srclen;
 }
+#endif


Why is fd_mask defined in an else clause with definitions of strlcpy/strlcat? Are the two related in some way that I am missing?

I’m just saving a line. I’ll separate them.

johnno1962 · 2016-09-08T13:37:02Z

New commit with responses to comments pushed. The battle with NSGeometry is won and Operation queues work if you make sure -DDEPLOYMENT_ENABLE_LIBDISPATCH is defined when you build. Less common code path does not seem to work. I’ll raise a ticket. Focus turns to the problem of detaching threads if they have interacted with JNI before they exit. Any hint about where to look appreciated.

tkremenek · 2016-09-16T15:26:08Z

@modocache Let's dicsuss extending the CI on swift-dev@swift.org, not in a pull request.

johnno1962 · 2016-09-19T12:02:53Z

Ready to go this end. All known issues resolved. @available(Android, unavailable) will have to wait.

parkera · 2016-09-22T22:11:47Z

Foundation/NSObjCRuntime.swift

@@ -196,6 +196,9 @@ internal func NSRequiresConcreteImplementation(_ fn: String = #function, file: S
 }

 internal func NSUnimplemented(_ fn: String = #function, file: StaticString = #file, line: UInt = #line) -> Never {
+    #if os(Android)
+    NSLog("\(fn) is not yet implemented. \(file):\(line)")
+    #endif


Why did we add an NSLog here?

fatalError() doesn’t print anything on Android (asserts do). Just giving people a pointer.

shouldn't fatalError call __android_log_print on android?

It should but this is a Foundation PR. I’ll take a look. Annoyingly, using __android_log_print requires a -llog when you link so it wouldn’t be a trivial change. Looks like a change to https://github.com/apple/swift/blob/master/stdlib/public/runtime/Errors.cpp#L204

Jira https://bugs.swift.org/browse/SR-2734

parkera · 2016-09-22T22:13:16Z

@swift-ci please test

parkera · 2016-09-22T22:13:27Z

Let's give this a shot.

parkera · 2016-09-22T23:17:21Z

We can follow up with separate changes for the rest. Thanks for tackling this @johnno1962 !

johnno1962 · 2016-09-22T23:17:51Z

Thanks Tony 👍

johnno1962 · 2016-09-23T00:18:27Z

Reminder: Example program https://github.com/SwiftJava/swift-android-samples,
Pre-compiled toolchain here http://johnholdsworth.com/android_toolchain.tgz

lhoward · 2016-09-23T01:03:36Z

CoreFoundation/Base.subproj/CFUtilities.c

+    CFIndex maxLength = CFStringGetMaximumSizeForEncoding(CFStringGetLength(message), encoding) + 1;
+
+    if (maxLength > sizeof(stack_buffer) / sizeof(stack_buffer[0])) {
+        buffer = calloc(sizeof(char), maxLength);


should check calloc() return value

Too late now...

Not too late - open up another PR. =)

Alright already, new PR #655

zayass · 2017-03-21T10:17:58Z

Hi @johnno1962

it's not been able to automate the libdispatch crosscompile

Can you little explain why it is hard to automate and how you build this libdispatch binary?

johnno1962 · 2017-03-21T15:44:47Z

Things have moved on a little since this PR. Some detail about how to build libdispatch are on this PR swiftlang/swift-corelibs-libdispatch#162

Restore workaround for android initially introduced int swiftlang#622 SwiftJava@6213cba Refix: https://bugs.swift.org/browse/SR-2587 https://bugs.swift.org/browse/SR-2588

johnno1962 mentioned this pull request Sep 7, 2016

Changes for Android build of Foundation swiftlang/swift#4662

Closed

modocache reviewed Sep 7, 2016
View reviewed changes

mekjaer reviewed Sep 7, 2016
View reviewed changes

parkera reviewed Sep 7, 2016
View reviewed changes

johnno1962 added 7 commits September 16, 2016 16:46

Jira for CFStringGetCString() crash with zero length strings

bc4cabb

Only use heap if required in logging

5fe1c8c

Reinstate fix for SR-2666

6858ae8

Reinstate fix for SR-2666

e15e240

Ensure NSUnimplemented() functions logged

759eca6

one true brace style

ffe852b

Final tweaks?

a82e390

parkera reviewed Sep 22, 2016

View reviewed changes

parkera merged commit 16f83dd into swiftlang:master Sep 22, 2016

lhoward reviewed Sep 23, 2016

View reviewed changes

johnno1962 mentioned this pull request Sep 23, 2016

Check return from calloc #655

Merged

zayass mentioned this pull request Apr 18, 2017

Restore android workaround for CONST_STRING_DECL #954

Closed

zayass mentioned this pull request Apr 21, 2017

Restore android workaround for CONST_STRING_DECL #957

Merged

zayass mentioned this pull request Apr 21, 2017

Restore android workaround for CONST_STRING_DECL [swift-3.1] #958

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port of Foundation to Android #622

Port of Foundation to Android #622

johnno1962 commented Sep 7, 2016 •

edited

Loading

modocache Sep 7, 2016

phausler Sep 7, 2016

modocache commented Sep 7, 2016

mekjaer Sep 7, 2016

phausler commented Sep 7, 2016

johnno1962 commented Sep 7, 2016 •

edited

Loading

phausler commented Sep 7, 2016

johnno1962 commented Sep 7, 2016 •

edited

Loading

parkera Sep 7, 2016

johnno1962 Sep 7, 2016 •

edited

Loading

johnno1962 commented Sep 8, 2016 •

edited

Loading

tkremenek commented Sep 16, 2016

johnno1962 commented Sep 19, 2016 •

edited

Loading

parkera Sep 22, 2016

johnno1962 Sep 22, 2016 •

edited

Loading

phausler Sep 22, 2016

johnno1962 Sep 22, 2016 •

edited

Loading

johnno1962 Sep 22, 2016

parkera commented Sep 22, 2016

parkera commented Sep 22, 2016

parkera commented Sep 22, 2016

johnno1962 commented Sep 22, 2016

johnno1962 commented Sep 23, 2016 •

edited

Loading

lhoward Sep 23, 2016

johnno1962 Sep 23, 2016 •

edited

Loading

parkera Sep 23, 2016

johnno1962 Sep 23, 2016

zayass commented Mar 21, 2017

johnno1962 commented Mar 21, 2017

Port of Foundation to Android #622

Port of Foundation to Android #622

Conversation

johnno1962 commented Sep 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

modocache commented Sep 7, 2016

Choose a reason for hiding this comment

phausler commented Sep 7, 2016

johnno1962 commented Sep 7, 2016 • edited Loading

phausler commented Sep 7, 2016

johnno1962 commented Sep 7, 2016 • edited Loading

Choose a reason for hiding this comment

johnno1962 Sep 7, 2016 • edited Loading

Choose a reason for hiding this comment

johnno1962 commented Sep 8, 2016 • edited Loading

tkremenek commented Sep 16, 2016

johnno1962 commented Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

johnno1962 Sep 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnno1962 Sep 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parkera commented Sep 22, 2016

parkera commented Sep 22, 2016

parkera commented Sep 22, 2016

johnno1962 commented Sep 22, 2016

johnno1962 commented Sep 23, 2016 • edited Loading

Choose a reason for hiding this comment

johnno1962 Sep 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zayass commented Mar 21, 2017

johnno1962 commented Mar 21, 2017

johnno1962 commented Sep 7, 2016 •

edited

Loading

johnno1962 commented Sep 7, 2016 •

edited

Loading

johnno1962 commented Sep 7, 2016 •

edited

Loading

johnno1962 Sep 7, 2016 •

edited

Loading

johnno1962 commented Sep 8, 2016 •

edited

Loading

johnno1962 commented Sep 19, 2016 •

edited

Loading

johnno1962 Sep 22, 2016 •

edited

Loading

johnno1962 Sep 22, 2016 •

edited

Loading

johnno1962 commented Sep 23, 2016 •

edited

Loading

johnno1962 Sep 23, 2016 •

edited

Loading