dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.25k stars 1.58k forks source link

Platform.operatingSystemVersion on iOS Simulator returns host machine OS version #47138

Closed alfalcon90 closed 3 years ago

alfalcon90 commented 3 years ago

When running a Flutter app on the iOS Simulator calling Platform.operatingSystemVersion from dart:io returns the OS version of the machine the simulator is running on instead of the device's that's being simulated. Using a package like https://pub.dev/packages/device_info_plus returns the correct OS version.

Screen Shot 2021-09-07 at 2 50 43 PM

a-siva commented 3 years ago

Comment from @chinmaygarde

That call in dart:io will get the host version for simulators since those apps are just mac apps in iOS garb. I think the right way of doing this (that works everywhere Flutter is supported) is [NSProcessInfo oeprationSystemVersion] https://developer.apple.com/documentation/foundation/nsprocessinfo/1410906-operatingsystemversion That will get you simulated iOS version. To check if you are in a simulated environment in the first place, you can use the macros in Availability.h. So, the macros tell you if you are on iOS (simulated or otherwise) and the processinfo tells you which version it is.

a-siva commented 3 years ago

The relevant code on the Dart side is at https://github.com/dart-lang/sdk/blob/master/runtime/bin/platform_macos.cc#L162

brianquinlan commented 3 years ago

device_info_plus uses [UIDevice systemVersion] to fetch the operating system version string.

The issue with using UIDevice or NSProcessInfo is that we'd need to add a dependency on Cocoa, which I don't think that we have right now:

$ otool -L dart
dart:
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)
    /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1775.118.101)
    /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 59754.100.106)
    /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1122.33.0)
$ otool -L testapp/build/ios/iphonesimulator/Runner.app/Runner
testapp/build/ios/iphonesimulator/Runner.app/Runner:
    /System/Library/Frameworks/Foundation.framework/Foundation (compatibility version 300.0.0, current version 1775.118.101)
    /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)
    /System/Library/Frameworks/UIKit.framework/UIKit (compatibility version 1.0.0, current version 4218.1.100)
       ...
    @rpath/libswiftCoreFoundation.dylib (compatibility version 1.0.0, current version 1.6.0, weak)
       ...

I did a quick check to see how Swift deals with this and they have the same issue i.e. not wanting to introduce a UIKit dependency but otherwise not knowing how to get the iOS version in the simulator.

Determining if we are being built for the simulator is trivial - there is a TARGET_OS_SIMULATOR define that we can use - but that doesn't help with the exact version.

Also, Platform.operatingSystem and platform.isIOS return the expected results on the simulator i.e. ios and true.

chinmaygarde commented 3 years ago

UIDevice is in UIKit (Cocoa Touch) which I can totally understand not wanting to add a dependency on. OTOH, NSProcessInfo is in Foundation which should be fine to depend on IMO.

Another alternative is to let the embedder (Flutter Engine in this case) tell the VM the version name via a callback stashed in DartInitialize.

The current implementation is TechnicallyCorrectâ„¢. Just that it doesn't account for simulated environments. It may be possible to reverse engineer how Apple does this in the simulator specific Foundation.framework but I doubt that is worth it.

brianquinlan commented 3 years ago

@chinmaygarde if we use NSProcessInfo, don't we need some sort of C++/Objective-C interop to access the NSProcessInfo methods? There are no .m or .mm files in the Dart SDK.

chinmaygarde commented 3 years ago

You should be able to just toss that one method into a separate TU and make it accessible to platform_macos.cc via a C++ call that converts from an NSString to a std::string. I am not sure if @a-siva has any objections to adding an ObjC TU to Dart or preferences for any one of the alternatives presented.

a-siva commented 3 years ago

I like your other idea of having the Embedder (Flutter Engine) tell the VM via a callback that way the smarts of interoping with Objective-C is in the embedder and not the VM.

brianquinlan commented 3 years ago

@a-siva One thing that I don't like about making the Embedder responsible for this is that it is a very odd separation of concerns i.e. Dart is responsible for getting platform version information except in the exact case of the iOS simulator, in which case it delegates to the embedder.

Of course my intuition could be totally off here.

I have a POC implementation of the NSProcessInfo info approach (sorry for presenting it as a diff - my git configuration is a messed up right now). I can probably tighten it up to only define the NSProcessInfo code when running in the simulator and it might be possible to only link against "Foundation.framework" when doing the simulator build.

But I'm happy to go with the Embedder approach if we prefer that - it should also be straightforward.

diff --git a/fix_version_in_dart.diff b/fix_version_in_dart.diff
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn
index 167ad395937..b39f30e9e5a 100644
--- a/runtime/bin/BUILD.gn
+++ b/runtime/bin/BUILD.gn
@@ -201,6 +201,7 @@ template("build_gen_snapshot") {

     if (is_mac) {
       frameworks = [
+        "Foundation.framework",
         "CoreFoundation.framework",
         "CoreServices.framework",
       ]
@@ -336,6 +337,9 @@ template("build_gen_snapshot_dart_io") {
       "io_natives.cc",
       "io_natives.h",
     ]
+    if (is_ios || is_mac) {
+      sources += ["platform_helper.h", "platform_helper.mm"]
+    }

     include_dirs = [
       "..",
@@ -410,6 +414,7 @@ template("dart_io") {
     deps = [ "//third_party/zlib" ] + extra_deps
     if (is_mac || is_ios) {
       frameworks = [
+        "Foundation.framework",
         "CoreFoundation.framework",
         "Security.framework",
       ]
@@ -437,6 +442,9 @@ template("dart_io") {
                  "io_natives.cc",
                  "io_natives.h",
                ] + extra_sources
+    if (is_ios || is_mac) {
+      sources += ["platform_helper.h", "platform_helper.mm"]
+    }

     if (is_linux || is_win || is_fuchsia) {
       if (dart_use_fallback_root_certificates) {
diff --git a/runtime/bin/platform_helper.h b/runtime/bin/platform_helper.h
new file mode 100644
index 00000000000..d70cde6146e
--- /dev/null
+++ b/runtime/bin/platform_helper.h
@@ -0,0 +1,14 @@
+#ifndef RUNTIME_BIN_PLATFORM_HELPER_H_
+#define RUNTIME_BIN_PLATFORM_HELPER_H_
+
+#include <string>
+
+namespace dart {
+namespace bin {
+
+std::string OSVersion();
+
+}
+}
+
+#endif
diff --git a/runtime/bin/platform_helper.mm b/runtime/bin/platform_helper.mm
new file mode 100644
index 00000000000..ce48e0c9cf6
--- /dev/null
+++ b/runtime/bin/platform_helper.mm
@@ -0,0 +1,28 @@
+#include "bin/platform_helper.h"
+
+#import <Foundation/NSProcessInfo.h>
+#import <Foundation/NSString.h>
+#include <sstream>
+
+namespace dart {
+namespace bin {
+
+std::string OSVersion() {
+    /*
+    NSString *OSVersionString = [[NSProcessInfo processInfo] ];
+
+    return std::string([OSVersionString UTF8String]);
+    */
+    NSOperatingSystemVersion version = [[NSProcessInfo processInfo] operatingSystemVersion];
+
+    std::stringstream s;
+    s << version.majorVersion
+      << "."
+      << version.minorVersion
+      << "."
+      << version.patchVersion;
+    return s.str();
+}
+
+}
+}
diff --git a/runtime/bin/platform_macos.cc b/runtime/bin/platform_macos.cc
index 26f40a4de2f..b356c26f6cc 100644
--- a/runtime/bin/platform_macos.cc
+++ b/runtime/bin/platform_macos.cc
@@ -9,10 +9,16 @@
 #include "bin/platform_macos.h"

 #include <CoreFoundation/CoreFoundation.h>
+#include <TargetConditionals.h>
+
+#ifdef TARGET_OS_SIMULATOR
+#include "bin/platform_helper.h"
+#endif

 #if !DART_HOST_OS_IOS
 #include <crt_externs.h>  // NOLINT
 #endif                    // !DART_HOST_OS_IOS
+
 #include <errno.h>        // NOLINT
 #include <mach-o/dyld.h>
 #include <signal.h>        // NOLINT
@@ -137,6 +143,7 @@ char* ExtractsOSVersionFromString(char* str) {
   return result;
 }

+#ifndef TARGET_OS_SIMULATOR
 static char* GetOSVersionFromPlist() {
   const char* path = "/System/Library/CoreServices/SystemVersion.plist";
   File* file = File::Open(NULL, path, File::kRead);
@@ -158,8 +165,16 @@ static char* GetOSVersionFromPlist() {
   }
   return ExtractsOSVersionFromString(buffer);
 }
+#endif

 const char* Platform::OperatingSystemVersion() {
+  // https://github.com/dart-lang/sdk/issues/47138
+  // [NSProcessInfo oeprationSystemVersion]
+  // https://github.com/apple/swift/blob/131a08169983e1d2fcabf045683489afc15de1bd/stdlib/private/StdlibUnittest/StdlibUnittest.swift#L2224
+
+#ifdef TARGET_OS_SIMULATOR
+  return DartUtils::ScopedCopyCString(OSVersion().c_str());
+#else
   char str[64];
   size_t size = sizeof(str);
   // This is only available to some versions later than 10.13.*. If it failed,
@@ -196,6 +211,7 @@ const char* Platform::OperatingSystemVersion() {
     return NULL;
   }
   return result;
+#endif
 }

 const char* Platform::LibraryPrefix() {
a-siva commented 3 years ago

Agree with your assesement @brianquinlan I forgot that the platform code that platform_macos.cc was already in the embedder side and would not have access to the VM callback