Revert change to Platform.excutable and add Platform.resolvedExecutable

sgjesse · sgjesse · commit c05c8c66069d · 2015-06-15T10:13:21.000+02:00
The change to Platform.excutable in e03ab17 was a breaking change and it has been reverted. A new getter Platform.resolvedExecutable has been added to provide the the fully qualified path of the executable. BUG=#16994 R=lrn@google.com, kustermann@google.com, len@google.com Review URL: https://codereview.chromium.org//1180623006.
diff --git a/runtime/bin/io_natives.cc b/runtime/bin/io_natives.cc
@@ -40,6 +40,7 @@ namespace bin {
   V(Platform_PathSeparator, 0)                                                 \
   V(Platform_LocalHostname, 0)                                                 \
   V(Platform_ExecutableName, 0)                                                \
+  V(Platform_ResolvedExecutableName, 0)                                        \
   V(Platform_Environment, 0)                                                   \
   V(Platform_ExecutableArguments, 0)                                           \
   V(Platform_PackageRoot, 0)                                                   \
diff --git a/runtime/bin/platform.cc b/runtime/bin/platform.cc
@@ -13,7 +13,7 @@ namespace dart {
 namespace bin {
 
 const char* Platform::executable_name_ = NULL;
-bool Platform::executable_name_resolved_ = false;
+const char* Platform::resolved_executable_name_ = NULL;
 const char* Platform::package_root_ = NULL;
 int Platform::script_index_ = 1;
 char** Platform::argv_ = NULL;
@@ -51,6 +51,16 @@ void FUNCTION_NAME(Platform_ExecutableName)(Dart_NativeArguments args) {
 }
 
 
+void FUNCTION_NAME(Platform_ResolvedExecutableName)(Dart_NativeArguments args) {
+  if (Platform::GetResolvedExecutableName() != NULL) {
+    Dart_SetReturnValue(
+        args, Dart_NewStringFromCString(Platform::GetResolvedExecutableName()));
+  } else {
+    Dart_SetReturnValue(args, Dart_Null());
+  }
+}
+
+
 void FUNCTION_NAME(Platform_ExecutableArguments)(Dart_NativeArguments args) {
   int end = Platform::GetScriptIndex();
   char** argv = Platform::GetArgv();
diff --git a/runtime/bin/platform.h b/runtime/bin/platform.h
@@ -46,15 +46,14 @@ class Platform {
     executable_name_ = executable_name;
   }
   static const char* GetExecutableName() {
-    if (!executable_name_resolved_) {
+    return executable_name_;
+  }
+  static const char* GetResolvedExecutableName() {
+    if (resolved_executable_name_ == NULL) {
       // Try to resolve the executable path using platform specific APIs.
-      char* path = Platform::ResolveExecutablePath();
-      if (path != NULL) {
-        executable_name_ = path;
-      }
-      executable_name_resolved_ = true;
+      resolved_executable_name_ = Platform::ResolveExecutablePath();
     }
-    return executable_name_;
+    return resolved_executable_name_;
   }
 
   // Stores and gets the package root.
@@ -80,8 +79,8 @@ class Platform {
  private:
   // The path to the executable.
   static const char* executable_name_;
-  // State to indicate whether the executable name has been resolved.
-  static bool executable_name_resolved_;
+  // The path to the resolved executable.
+  static const char* resolved_executable_name_;
 
   static const char* package_root_;
   static int script_index_;
diff --git a/runtime/bin/platform_patch.dart b/runtime/bin/platform_patch.dart
@@ -10,6 +10,8 @@ patch class _Platform {
       native "Platform_OperatingSystem";
   /* patch */ static _localHostname() native "Platform_LocalHostname";
   /* patch */ static _executable() native "Platform_ExecutableName";
+  /* patch */ static _resolvedExecutable()
+      native "Platform_ResolvedExecutableName";
   /* patch */ static _environment() native "Platform_Environment";
   /* patch */ static List<String> _executableArguments()
        native "Platform_ExecutableArguments";
diff --git a/sdk/lib/_internal/compiler/js_lib/io_patch.dart b/sdk/lib/_internal/compiler/js_lib/io_patch.dart
@@ -221,6 +221,10 @@ class _Platform {
     throw new UnsupportedError("Platform._executable");
   }
   @patch
+  static _resolvedExecutable() {
+    throw new UnsupportedError("Platform._resolvedExecutable");
+  }
+  @patch
   static List<String> _executableArguments() {
     throw new UnsupportedError("Platform._executableArguments");
   }
diff --git a/sdk/lib/io/platform.dart b/sdk/lib/io/platform.dart
@@ -131,13 +131,24 @@ class Platform {
    * Returns the path of the executable used to run the script in this
    * isolate.
    *
-   * If supported by the platform the returned path will be absolute.
+   * The path returned is the literal path used to run the script. This
+   * path might be relative or just be a name from which the executable
+   * was found by searching the `PATH`.
    *
-   * If the execution environment does not support [executable] an empty
-   * string is returned.
+   * To get the absolute path to the resolved executable use
+   * [resolvedExecutable].
    */
   static String get executable => _Platform.executable;
 
+  /**
+   * Returns the path of the executable used to run the script in this
+   * isolate after it has been resolved by the OS.
+   *
+   * This is the absolute path, with all symlinks resolved, to the
+   * executable used to run the script.
+   */
+  static String get resolvedExecutable => _Platform.resolvedExecutable;
+
   /**
    * Returns the absolute URI of the script being run in this
    * isolate.
diff --git a/sdk/lib/io/platform_impl.dart b/sdk/lib/io/platform_impl.dart
@@ -10,6 +10,7 @@ class _Platform {
   external static String _operatingSystem();
   external static _localHostname();
   external static _executable();
+  external static _resolvedExecutable();
   /**
    * Retrieve the entries of the process environment.
    *
@@ -31,6 +32,7 @@ class _Platform {
   external static String _version();
 
   static String executable = _executable();
+  static String resolvedExecutable = _resolvedExecutable();
   static String packageRoot = _packageRoot();
 
   // Cache the OS environemnt. This can be an OSError instance if
diff --git a/tests/standalone/io/platform_resolved_executable_test.dart b/tests/standalone/io/platform_resolved_executable_test.dart
@@ -35,7 +35,7 @@ void verify(String exePath, {String altPath}) {
   }
 
   var result = processResult.stdout.trim();
-  expectEquals(Platform.executable, result);
+  expectEquals(Platform.resolvedExecutable, result);
 }
 
 void testDartExecShouldNotBeInCurrentDir() {
@@ -51,20 +51,20 @@ void testShouldSucceedWithEmptyPathEnvironment() {
 }
 
 void testShouldSucceedWithSourcePlatformExecutable() {
-  verify(Platform.executable);
+  verify(Platform.resolvedExecutable);
 }
 
 void testExeSymLinked(Directory dir) {
   var dirUri = new Uri.directory(dir.path);
   var link = new Link.fromUri(dirUri.resolve('dart_exe_link'));
-  link.createSync(Platform.executable);
+  link.createSync(Platform.resolvedExecutable);
   verify(link.path);
 }
 
 void testPathToDirWithExeSymLinked(Directory dir) {
   var dirUri = new Uri.directory(dir.path);
   var link = new Link.fromUri(dirUri.resolve('dart_exe_link'));
-  link.createSync(Platform.executable);
+  link.createSync(Platform.resolvedExecutable);
   verify('dart_exe_link', altPath: dir.path);
 }
 
@@ -75,7 +75,7 @@ void testExeDirSymLinked(Directory dir) {
   var linkDirUri = dirUri.resolve('dart_bin_dir_link');
   var link = new Link.fromUri(linkDirUri);
 
-  var exeFile = new File(Platform.executable);
+  var exeFile = new File(Platform.resolvedExecutable);
 
   link.createSync(exeFile.parent.path);
 
@@ -91,15 +91,15 @@ void testPathPointsToSymLinkedSDKPath(Directory dir) {
   var linkDirUri = dirUri.resolve('dart_bin_dir_link');
   var link = new Link.fromUri(linkDirUri);
 
-  var exeFile = new File(Platform.executable);
+  var exeFile = new File(Platform.resolvedExecutable);
 
   link.createSync(exeFile.parent.path);
 
   verify(platformExeName, altPath: link.path);
 }
 
 void testPathToSDKDir() {
-  var exeFile = new File(Platform.executable);
+  var exeFile = new File(Platform.resolvedExecutable);
   var binDirPath = exeFile.parent.path;
 
   verify(platformExeName, altPath: binDirPath);
@@ -115,15 +115,19 @@ void withTempDir(void test(Directory dir)) {
 }
 
 String get platformExeName {
-  var raw = new Uri.file(Platform.executable);
+  var raw = new Uri.file(Platform.resolvedExecutable);
   return raw.pathSegments.last;
 }
 
 String get scriptPath => Platform.script.toFilePath();
 
 void main() {
+  // The same script is used for both running the tests and as for starting
+  // child verifying the value of Platform.resolvedExecutable. If the
+  // environment variable _SCRIPT_KEY is set this is a child process which
+  // should print the value of Platform.resolvedExecutable.
   if (Platform.environment.containsKey(_SCRIPT_KEY)) {
-    print(Platform.executable);
+    print(Platform.resolvedExecutable);
     return;
   }
 
diff --git a/tests/standalone/io/platform_test.dart b/tests/standalone/io/platform_test.dart
@@ -24,9 +24,15 @@ test() {
   Expect.isTrue(hostname is String && hostname != "");
   var environment = Platform.environment;
   Expect.isTrue(environment is Map<String, String>);
-  Expect.isTrue(Platform.executable.contains('dart'));
   if (!Platform.isWindows) {
-    Expect.isTrue(Platform.executable.startsWith('/'));
+    Expect.isTrue(Platform.executable.endsWith('dart'));
+    Expect.isTrue(Platform.resolvedExecutable.endsWith('dart'));
+  } else {
+    Expect.isTrue(Platform.executable.endsWith('dart.exe'));
+    Expect.isTrue(Platform.resolvedExecutable.endsWith('dart.exe'));
+  }
+  if (!Platform.isWindows) {
+    Expect.isTrue(Platform.resolvedExecutable.startsWith('/'));
   } else {
     // This assumes that tests (both locally and on the bots) are
     // running off a location referred to by a drive letter. If a UNC
diff --git a/tests/standalone/standalone.status b/tests/standalone/standalone.status
@@ -35,7 +35,7 @@ io/directory_fuzz_test: Skip
 # This is expected as MacOS by default runs with a very low number
 # of allowed open files ('ulimit -n' says something like 256).
 io/socket_many_connections_test: Skip
-io/platform_executable_test/06: RuntimeError
+io/platform_resolved_executable_test/06: RuntimeError
 
 [ $compiler == none && ($runtime == drt || $runtime == dartium || $runtime == ContentShellOnAndroid) ]
 typed_array_test: Fail # Issue 13921
@@ -211,10 +211,10 @@ io/issue_22637_test: Crash # (test()async{server=...  cannot handle async/sync*/
 io/link_test: Crash # Instance of 'TypeOperator': type check unimplemented for _Nullary.
 io/link_uri_test: Crash # Instance of 'TypeOperator': type check unimplemented for _Nullary.
 io/observatory_test: Crash # Instance of 'TypeOperator': type check unimplemented for _Nullary.
-io/platform_executable_test/01: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
-io/platform_executable_test/02: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
-io/platform_executable_test/04: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
-io/platform_executable_test/05: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
+io/platform_resolved_executable_test/01: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
+io/platform_resolved_executable_test/02: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
+io/platform_resolved_executable_test/04: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
+io/platform_resolved_executable_test/05: Crash # (try {test(tempDir);}finally {tempDir.deleteSync(recursive:true);}): try/finally
 io/platform_test: Crash # Instance of 'TypeOperator': type check unimplemented for _Nullary.
 io/process_invalid_arguments_test: Crash # Instance of 'TypeOperator': type check unimplemented for _Nullary.
 io/raw_datagram_socket_test: Crash # (switch (event){case...  Unhandled node

Original file line number	Diff line number	Diff line change
`@@ -221,6 +221,10 @@ class _Platform {`
`221`	`221`	`throw new UnsupportedError("Platform._executable");`
`222`	`222`	`}`
`223`	`223`	`@patch`
	`224`	`+ static _resolvedExecutable() {`
	`225`	`+ throw new UnsupportedError("Platform._resolvedExecutable");`
	`226`	`+ }`
	`227`	`+ @patch`
`224`	`228`	`static List<String> _executableArguments() {`
`225`	`229`	`throw new UnsupportedError("Platform._executableArguments");`
`226`	`230`	`}`