Skip to content

Changes required for swift-4.1-branch port to Android #1518

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 9, 2018
Merged

Changes required for swift-4.1-branch port to Android #1518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25131e8
Changes required for swift-4.1-branch port to Android
johnno1962 Apr 13, 2018
df28468
Remove contentious wokrarounds
johnno1962 Apr 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CoreFoundation/Base.subproj/CFInternal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ extern void __CFGenericValidateType_(CFTypeRef cf, CFTypeID type, const char *fu
#define __CFBitfield64GetValue(V, N1, N2) (((V) & __CFBitfield64Mask(N1, N2)) >> (N2))
#define __CFBitfield64SetValue(V, N1, N2, X) ((V) = ((V) & ~__CFBitfield64Mask(N1, N2)) | ((((uint64_t)X) << (N2)) & __CFBitfield64Mask(N1, N2)))

#if __LP64__
#if __LP64__ || DEPLOYMENT_TARGET_ANDROID
typedef uint64_t __CFInfoType;
#define __CFInfoMask(N1, N2) __CFBitfield64Mask(N1, N2)
#else
Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/Base.subproj/CFKnownLocations.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ CFURLRef _Nullable _CFKnownLocationCreatePreferencesURLForUser(CFKnownLocationUs
}

}
#elif !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#elif !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

/*
Building for an OS that uses the FHS, BSD's hier(7), and/or the XDG specification for paths:
Expand Down
3 changes: 3 additions & 0 deletions CoreFoundation/Base.subproj/CFPlatform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,9 @@ CF_EXPORT CFStringRef CFCopyFullUserName(void) {
uid_t euid;
__CFGetUGIDs(&euid, NULL);
struct passwd *upwd = getpwuid(euid ? euid : getuid());
#if DEPLOYMENT_TARGET_ANDROID
#define pw_gecos pw_name
#endif
if (upwd && upwd->pw_gecos) {
result = CFStringCreateWithCString(kCFAllocatorSystemDefault, upwd->pw_gecos, kCFPlatformInterfaceStringEncoding);
}
Expand Down
1 change: 1 addition & 0 deletions CoreFoundation/Base.subproj/CFUtilities.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@ void CFLog1(CFLogLevel lev, CFStringRef message) {
CFStringGetCString(message, buffer, maxLength, encoding);

__android_log_print(priority, tag, "%s", buffer);
fprintf(stderr, "%s\n", buffer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for? Isn't __android_log_print enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is helpful when you are running an Android binary (such as the tests) from the command line using an “adb shell". Otherwise NSLogs are only visible using an “adb logcat” by which time the messages have lost their context. This doesn’t affect normal running of an actual application and the logging to stderr goes into the void.


if (buffer != &stack_buffer[0]) free(buffer);
#else
Expand Down
2 changes: 2 additions & 0 deletions CoreFoundation/Base.subproj/ForSwiftFoundationOnly.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,8 @@ static inline _Bool _withStackOrHeapBuffer(size_t amount, void (__attribute__((n
static inline int _direntNameLength(struct dirent *entry) {
#ifdef _D_EXACT_NAMLEN // defined on Linux
return _D_EXACT_NAMLEN(entry);
#elif DEPLOYMENT_TARGET_ANDROID
return strlen(entry->d_name);
#else
return entry->d_namlen;
#endif
Expand Down
9 changes: 9 additions & 0 deletions CoreFoundation/NumberDate.subproj/CFTimeZone.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,15 @@ static CFTimeZoneRef __CFTimeZoneCreateSystem(void) {
CFRelease(name);
if (result) return result;
}
#if DEPLOYMENT_TARGET_ANDROID
// Timezone database by name not available on Android.
// Approximate with gmtoff - could be general default.
struct tm info;
time_t now = time(NULL);
if (NULL != localtime_r(&now, &info)) {
return CFTimeZoneCreateWithTimeIntervalFromGMT(kCFAllocatorSystemDefault, info.tm_gmtoff);
}
#endif
return CFTimeZoneCreateWithTimeIntervalFromGMT(kCFAllocatorSystemDefault, 0.0);
}

Expand Down
8 changes: 4 additions & 4 deletions CoreFoundation/PlugIn.subproj/CFBundle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ static void _CFBundleEnsureBundlesExistForImagePaths(CFArrayRef imagePaths);

#pragma mark -

#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

// Functions and constants for FHS bundles:
#define _CFBundleFHSDirectory_share CFSTR("share")
Expand All @@ -162,10 +162,10 @@ static Boolean _CFBundleURLIsForFHSInstalledBundle(CFURLRef bundleURL) {

return isFHSBundle;
}
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

Boolean _CFBundleSupportsFHSBundles() {
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID
return true;
#else
return false;
Expand Down Expand Up @@ -714,7 +714,7 @@ static CFBundleRef _CFBundleCreate(CFAllocatorRef allocator, CFURLRef bundleURL,

bundle->_url = newURL;

#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous comment on finding the other FHS declarations and disabling them still applies, I believe. It will break CFBundle if not so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve gone through the code again and gated anything I could find about FHS out for Android.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a bug in github where I can’t reply directly to your next comment about path(toResource:ofType:) so I’ll add a note here. One of the goals of the Android port is that code for iOS should run on Android and it’s likely that code will use the path(toResource:ofType:) pattern somewhere. On Android It’s up to the Java side of the app to copy resources required into the cache/tmp directory then the Swift/Foundation side can find these resources inside this directory as if it was a bundle so the code runs unmodified.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the use case, but again, it breaks our contract that Bundle.main be where the resources for a specific bundle actually are (not potentially will be if the author is careful), and that this be a directory that's either specific to the application or at worst contains existing application executable and resource data for that one application.

I outlined a workaround above — since apps need to write custom code to copy resources to some filesystem directory, it is no stretch, if they want Bundle API to fetch their own resources, to ask that they construct a Bundle instance to the copied data.

I will also reiterate that any library code using Bundle(for:) that builds with the Swift Package Manager and then asserts the returned bundle is exclusive to that library is essentially in undefined behavior territory, and should be fixed upstream.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Also — that breaks if multiple components on Android are using Swift, have equally-named resources or resource directories {which isn't that unlikely: I assume careless copying code could easily clobber an en.lproj folder}, and are running at the same time.)

Copy link
Contributor Author

@johnno1962 johnno1962 Apr 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps there just is a misunderstanding here. This is not a system shared directory. It is the app's home, combination Documents and tmp directory on iOS with a path something like: /data/data/com.example.user.myapplication/cache and is the only directory an app has read write access to I am aware of. There's no candidate for a bundle directory other than the approach I suggest of copying resources into this directory on Android as resources are unpacked from their .apk (zip file) on demand. This is what the zip file for an Android app looks like:

Users-iMac:swift-corelibs-foundation user$ unzip -l ~/swift-android-kotlin/app/build/outputs/apk/app-debug.apk
Archive:  /Users/user/swift-android-kotlin/app/build/outputs/apk/app-debug.apk
  Length      Date    Time    Name
---------  ---------- -----   ----
     2788  00-00-1980 00:00   AndroidManifest.xml
     1211  00-00-1980 00:00   META-INF/CERT.RSA
    31429  00-00-1980 00:00   META-INF/CERT.SF
    31367  00-00-1980 00:00   META-INF/MANIFEST.MF
      246  01-01-1970 01:00   META-INF/kotlin-runtime.kotlin_module
       43  01-01-1970 01:00   META-INF/kotlin-stdlib-jre7.kotlin_module
     2090  01-01-1970 01:00   META-INF/kotlin-stdlib.kotlin_module
    46044  00-00-1980 00:00   classes.dex
      752  00-00-1980 00:00   classes2.dex
      926  01-01-1970 01:00   kotlin/annotation/annotation.kotlin_builtins
     3689  01-01-1970 01:00   kotlin/collections/collections.kotlin_builtins
      726  01-01-1970 01:00   kotlin/internal/internal.kotlin_builtins
    14202  01-01-1970 01:00   kotlin/kotlin.kotlin_builtins
     2296  01-01-1970 01:00   kotlin/ranges/ranges.kotlin_builtins
     4866  01-01-1970 01:00   kotlin/reflect/reflect.kotlin_builtins
 10933048  00-00-1980 00:00   lib/armeabi-v7a/libFoundation.so
   359812  00-00-1980 00:00   lib/armeabi-v7a/libXCTest.so
   661056  00-00-1980 00:00   lib/armeabi-v7a/libc++_shared.so
  2009280  00-00-1980 00:00   lib/armeabi-v7a/libcurl.so
   668356  00-00-1980 00:00   lib/armeabi-v7a/libdispatch.so
 25911876  00-00-1980 00:00   lib/armeabi-v7a/libscudata.so
  2045828  00-00-1980 00:00   lib/armeabi-v7a/libscui18n.so
  1418788  00-00-1980 00:00   lib/armeabi-v7a/libscuuc.so
  1079184  00-00-1980 00:00   lib/armeabi-v7a/libsqlite3.so
  5259060  00-00-1980 00:00   lib/armeabi-v7a/libswiftCore.so
    31020  00-00-1980 00:00   lib/armeabi-v7a/libswiftGlibc.so
   506816  00-00-1980 00:00   lib/armeabi-v7a/libswiftRemoteMirror.so
   383140  00-00-1980 00:00   lib/armeabi-v7a/libswiftSwiftOnoneSupport.so
  2810068  00-00-1980 00:00   lib/armeabi-v7a/libswifthello.so
  1783196  00-00-1980 00:00   lib/armeabi-v7a/libxml2.so
      620  00-00-1980 00:00   res/anim-v21/design_bottom_sheet_slide_in.xml
      620  00-00-1980 00:00   res/anim-v21/design_bottom_sheet_slide_out.xml
...

Should I mark the whole Bundle api unavailable? There is also no prospect of a OSS fix upstream to Alamofire as the code I’m using is a fork of a fork. If it were me I would make the fix I suggest apply to all Linux systems in the interests of compatibility. If I were to fix Alamofire there just be some other library which falls at this hurdle.

But we’ve discussed these issues a few times and we’re going around in circles. What are the minimum changes I have proposed you would have me remove to get this PR merged? I’ll have to maintain a fork (undesirable as that may be) to get what I need to get done done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following changes are, I believe, at issue:

  • Bundle(for:) should not return an arbitrary bundle. It should stay callable. (My personal intent is to implement it on Linux as soon as resources allow.)

  • Bundle.main should not pick an arbitrary path whose forming substantially differs from other platforms. It should go on the existing fallback path unless that breaks existing well-formed code.

  • CFPreferences… should not assume that any CFBundle path is writable or a correct location for user data. All paths returned by CFBundle must be treated as read-only.

Copy link
Contributor

@millenomi millenomi Apr 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, someone should send Alamofire a patch, I believe. (For the reasons I outlined, Alamofire's code is not well-formed.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.. how does it look now?

bundle->_isFHSInstalledBundle = _CFBundleURLIsForFHSInstalledBundle(newURL);
#endif

Expand Down
14 changes: 7 additions & 7 deletions CoreFoundation/PlugIn.subproj/CFBundle_Executable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include <dlfcn.h>
#endif

#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

#if DEPLOYMENT_TARGET_LINUX
#if __LP64__
Expand Down Expand Up @@ -48,7 +48,7 @@
_kCFBundleFHSDirectory_lib
#endif // DEPLOYMENT_TARGET_LINUX

#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

// This is here because on iPhoneOS with the dyld shared cache, we remove binaries from their
// original locations on disk, so checking whether a binary's path exists is no longer sufficient.
Expand All @@ -73,7 +73,7 @@ static CFURLRef _CFBundleCopyExecutableURLRaw(CFURLRef urlPath, CFStringRef exeN
CFURLRef executableURL = NULL;
if (!urlPath || !exeName) return NULL;

#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID
if (!executableURL) {
executableURL = CFURLCreateWithFileSystemPathRelativeToBase(kCFAllocatorSystemDefault, exeName, kCFURLPOSIXPathStyle, false, urlPath);
if (!_binaryLoadable(executableURL)) {
Expand Down Expand Up @@ -203,7 +203,7 @@ static CFURLRef _CFBundleCopyExecutableURLInDirectory2(CFBundleRef bundle, CFURL
Boolean doExecSearch = true;
#endif

#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID
if (lookupMainExe && bundle && bundle->_isFHSInstalledBundle) {
// For a FHS installed bundle, the URL points to share/Bundle.resources, and the binary is in:

Expand All @@ -227,13 +227,13 @@ static CFURLRef _CFBundleCopyExecutableURLInDirectory2(CFBundleRef bundle, CFURL

CFRelease(prefixPath);
}
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

// Now, look for the executable inside the bundle.
if (!foundIt && doExecSearch && 0 != version) {
CFURLRef exeDirURL = NULL;

#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID
if (bundle && bundle->_isFHSInstalledBundle) {
CFURLRef withoutExtension = CFURLCreateCopyDeletingPathExtension(kCFAllocatorSystemDefault, url);
CFStringRef lastPathComponent = CFURLCopyLastPathComponent(withoutExtension);
Expand All @@ -248,7 +248,7 @@ static CFURLRef _CFBundleCopyExecutableURLInDirectory2(CFBundleRef bundle, CFURL
CFRelease(libexec);
CFRelease(exeDirName);
} else
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID
if (1 == version) {
exeDirURL = CFURLCreateWithString(kCFAllocatorSystemDefault, _CFBundleExecutablesURLFromBase1, url);
} else if (2 == version) {
Expand Down
4 changes: 2 additions & 2 deletions CoreFoundation/PlugIn.subproj/CFBundle_Internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ CF_EXTERN_C_BEGIN
#endif

// FHS bundles are supported on the Swift and C runtimes, except on Windows.
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#if !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

#if DEPLOYMENT_TARGET_LINUX || DEPLOYMENT_TARGET_FREEBSD
#define _CFBundleFHSSharedLibraryFilenamePrefix CFSTR("lib")
Expand All @@ -43,7 +43,7 @@ CF_EXTERN_C_BEGIN
#error Disable FHS bundles or specify shared library prefixes and suffixes for this platform.
#endif // DEPLOYMENT_TARGET_…

#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS
#endif // !DEPLOYMENT_RUNTIME_OBJC && !DEPLOYMENT_TARGET_WINDOWS && !DEPLOYMENT_TARGET_ANDROID

#define CFBundleExecutableNotFoundError 4
#define CFBundleExecutableNotLoadableError 3584
Expand Down
6 changes: 3 additions & 3 deletions CoreFoundation/URL.subproj/CFURL.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#if __has_include(<sys/syslog.h>)
#include <sys/syslog.h>
#elif __has_include(<syslog.h>)
#if __has_include(<syslog.h>)
#include <syslog.h>
#else
#include <sys/syslog.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this from __has_include style?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a minor portability fix. We need a syslog.h but probably not both and if you include sys/syslog.h on android which has syslog.h you can end up including the host os one.

#endif
#include <CoreFoundation/CFURLPriv.h>
#endif
Expand Down
6 changes: 4 additions & 2 deletions Foundation/Bundle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ open class Bundle: NSObject {
self.init(path: url.path)
}

public init(for aClass: AnyClass) { NSUnimplemented() }

public init(for aClass: AnyClass) {
NSUnimplemented()
}

public init?(identifier: String) {
super.init()

Expand Down
11 changes: 7 additions & 4 deletions Foundation/FileManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,13 @@ open class FileManager : NSObject {

This method replaces fileSystemAttributesAtPath:.
*/
#if os(Android)
@available(*, unavailable, message: "Unsuppported on this platform")
open func attributesOfFileSystem(forPath path: String) throws -> [FileAttributeKey : Any] {
NSUnsupported()
}
#else
open func attributesOfFileSystem(forPath path: String) throws -> [FileAttributeKey : Any] {
#if os(Android)
NSUnimplemented()
#else
// statvfs(2) doesn't support 64bit inode on Darwin (apfs), fallback to statfs(2)
#if os(macOS) || os(iOS)
var s = statfs()
Expand Down Expand Up @@ -407,8 +410,8 @@ open class FileManager : NSObject {
result[.systemFreeNodes] = NSNumber(value: UInt64(s.f_ffree))

return result
#endif
}
#endif

/* createSymbolicLinkAtPath:withDestination:error: returns YES if the symbolic link that point at 'destPath' was able to be created at the location specified by 'path'. If this method returns NO, the link was unable to be created and an NSError will be returned by reference in the 'error' parameter. This method does not traverse a terminal symlink.

Expand Down
4 changes: 2 additions & 2 deletions Foundation/NSCFString.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ internal final class _NSCFConstantString : _NSCFString {
// FIXME: Split expression as a work-around for slow type
// checking (tracked by SR-5322).
let offTemp1 = MemoryLayout<OpaquePointer>.size + MemoryLayout<uintptr_t>.size
let offTemp2 = MemoryLayout<_CFInfo>.size
return offTemp1 + offTemp2 + MemoryLayout<UnsafePointer<UInt8>>.size
let offset = offTemp1 + MemoryLayout<_CFInfo>.size
return offset + MemoryLayout<UnsafePointer<UInt8>>.size
}

private var _lenPtr : UnsafeMutableRawPointer {
Expand Down
8 changes: 0 additions & 8 deletions Foundation/NSTimeZone.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,6 @@ open class NSTimeZone : NSObject, NSCopying, NSSecureCoding, NSCoding {
extension NSTimeZone {

open class var system: TimeZone {
#if os(Android)
var now = time(nil), info = tm()
if localtime_r(&now, &info) != nil {
// NOTE: this is not a real time zone but a fixed offset from GMT.
// It will be incorrect outside the current daylight saving period.
return TimeZone(reference: NSTimeZone(forSecondsFromGMT: info.tm_gmtoff))
}
#endif
return CFTimeZoneCopySystem()._swiftObject
}

Expand Down
2 changes: 2 additions & 0 deletions TestFoundation/TestFileManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ class TestFileManager : XCTestCase {
}

func test_fileSystemAttributes() {
#if !os(Android)
let fm = FileManager.default
let path = NSTemporaryDirectory()

Expand Down Expand Up @@ -306,6 +307,7 @@ class TestFileManager : XCTestCase {
} catch let err {
XCTFail("\(err)")
}
#endif
}

func test_setFileAttributes() {
Expand Down
5 changes: 0 additions & 5 deletions TestFoundation/TestNSAttributedString.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,6 @@ class TestNSAttributedString : XCTestCase {
}

func test_enumerateAttributes() {
#if os(Android)
// Invalid dictionary returned by CFAttributedStringGetAttributesAndLongestEffectiveRange
XCTFail("Intermittent failures on Android")
#else
let string = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus consectetur et sem vitae consectetur. Nam venenatis lectus a laoreet blandit."

let attrKey1 = NSAttributedStringKey("attribute.placeholder.key1")
Expand Down Expand Up @@ -254,7 +250,6 @@ class TestNSAttributedString : XCTestCase {
}
XCTAssertEqual(rangeDescriptionString, "(0,10)")
XCTAssertEqual(attrsDescriptionString, "[attribute.placeholder.key1:attribute.placeholder.value1]")
#endif
}

func test_copy() {
Expand Down
15 changes: 9 additions & 6 deletions TestFoundation/TestNSNumber.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1083,37 +1083,40 @@ class TestNSNumber : XCTestCase {

func test_stringValue() {

// The following casts on subtraction are required for an Android compile
// https://bugs.swift.org/browse/SR-7469

if UInt.max == UInt32.max {
XCTAssertEqual(NSNumber(value: UInt.min).stringValue, "0")
XCTAssertEqual(NSNumber(value: UInt.min + 1).stringValue, "1")
XCTAssertEqual(NSNumber(value: UInt.max).stringValue, "4294967295")
XCTAssertEqual(NSNumber(value: UInt.max - 1).stringValue, "4294967294")
XCTAssertEqual(NSNumber(value: UInt.max - 1 as UInt).stringValue, "4294967294")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this and similar a 32-bit guarantee violation? Why doesn't it work on Android as written? If it is incorrect, it should be diagnosed as incorrect on all platforms to avoid forking the language.

Copy link
Contributor Author

@johnno1962 johnno1962 Apr 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a strange problem indeed which I don’t know the solution for other than the change you see. It doesn’t seem to be 32 bit specific but something not quite right about the compiler and subtraction. I’m seeing warnings I don’t expect and the errors I had to fix with this change.

Sources/TestNSNumber.swift:1148:50: warning: '-' is deprecated: Please use explicit type conversions or Strideable methods for mixed-type arithmetics.
XCTAssertEqual(NSNumber(value: Int64.max - 1).stringValue, "9223372036854775806")
^
Sources/TestNSNumber.swift:1090:53: error: arithmetic operation '4294967295 + 4294967295' (on unsigned 32-bit integer type) results in an overflow
XCTAssertEqual(NSNumber(value: UInt.max - 1).stringValue, "4294967294")
~~~~~~~~ ^ ~

The compiler is basically built right out of the swift-4.1-branch with only three minor unrelated patches.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something is deeply weird with that error, but the fix makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to add a comment for this line (or jira if there is one) as otherwise it may get reverted by someone 'cleaning up' the tests in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://bugs.swift.org/browse/SR-7469

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need @spevans' suggested comment here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will include in next push

} else if UInt.max == UInt64.max {
XCTAssertEqual(NSNumber(value: UInt.min).stringValue, "0")
XCTAssertEqual(NSNumber(value: UInt.min + 1).stringValue, "1")
XCTAssertEqual(NSNumber(value: UInt.max).stringValue, "18446744073709551615")
XCTAssertEqual(NSNumber(value: UInt.max - 1).stringValue, "18446744073709551614")
XCTAssertEqual(NSNumber(value: UInt.max - 1 as UInt).stringValue, "18446744073709551614")
}

XCTAssertEqual(NSNumber(value: UInt8.min).stringValue, "0")
XCTAssertEqual(NSNumber(value: UInt8.min + 1).stringValue, "1")
XCTAssertEqual(NSNumber(value: UInt8.max).stringValue, "255")
XCTAssertEqual(NSNumber(value: UInt8.max - 1).stringValue, "254")
XCTAssertEqual(NSNumber(value: UInt8.max - 1 as UInt8).stringValue, "254")

XCTAssertEqual(NSNumber(value: UInt16.min).stringValue, "0")
XCTAssertEqual(NSNumber(value: UInt16.min + 1).stringValue, "1")
XCTAssertEqual(NSNumber(value: UInt16.max).stringValue, "65535")
XCTAssertEqual(NSNumber(value: UInt16.max - 1).stringValue, "65534")
XCTAssertEqual(NSNumber(value: UInt16.max - 1 as UInt16).stringValue, "65534")

XCTAssertEqual(NSNumber(value: UInt32.min).stringValue, "0")
XCTAssertEqual(NSNumber(value: UInt32.min + 1).stringValue, "1")
XCTAssertEqual(NSNumber(value: UInt32.max).stringValue, "4294967295")
XCTAssertEqual(NSNumber(value: UInt32.max - 1).stringValue, "4294967294")
XCTAssertEqual(NSNumber(value: UInt32.max - 1 as UInt32).stringValue, "4294967294")

XCTAssertEqual(NSNumber(value: UInt64.min).stringValue, "0")
XCTAssertEqual(NSNumber(value: UInt64.min + 1).stringValue, "1")
XCTAssertEqual(NSNumber(value: UInt64.max).stringValue, "18446744073709551615")
XCTAssertEqual(NSNumber(value: UInt64.max - 1).stringValue, "18446744073709551614")
XCTAssertEqual(NSNumber(value: UInt64.max - 1 as UInt64).stringValue, "18446744073709551614")

if Int.max == Int32.max {
XCTAssertEqual(NSNumber(value: Int.min).stringValue, "-2147483648")
Expand Down