From e01bcbaa0d7f456f02ca42dcff9495ff64f3b83f Mon Sep 17 00:00:00 2001 From: "ajohnsen@google.com" Date: Fri, 21 Jun 2013 06:44:55 +0000 Subject: [PATCH] Include parent environment by default, add option to not, for Process. BUG=https://code.google.com/p/dart/issues/detail?id=9294,https://code.google.com/p/dart/issues/detail?id=9295 R=sgjesse@google.com Review URL: https://codereview.chromium.org//17261026 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@24262 260f80e4-7a28-3924-810f-c04153c831b5 --- runtime/bin/process_patch.dart | 40 ++++++++++++------- sdk/lib/_internal/pub/lib/src/io.dart | 9 +---- sdk/lib/_internal/pub/test/test_pub.dart | 3 +- sdk/lib/io/process.dart | 10 +++++ .../io/process_environment_test.dart | 18 +++++++-- 5 files changed, 51 insertions(+), 29 deletions(-) diff --git a/runtime/bin/process_patch.dart b/runtime/bin/process_patch.dart index b30f449c7d0..65d271f0a1f 100644 --- a/runtime/bin/process_patch.dart +++ b/runtime/bin/process_patch.dart @@ -20,11 +20,13 @@ patch class Process { List arguments, {String workingDirectory, Map environment, - bool runInShell}) { + bool includeParentEnvironment: true, + bool runInShell: false}) { _ProcessImpl process = new _ProcessImpl(executable, arguments, workingDirectory, environment, + includeParentEnvironment, runInShell); return process._start(); } @@ -34,13 +36,15 @@ patch class Process { List arguments, {String workingDirectory, Map environment, - bool runInShell, + bool includeParentEnvironment: true, + bool runInShell: false, Encoding stdoutEncoding: Encoding.SYSTEM, Encoding stderrEncoding: Encoding.SYSTEM}) { return _runNonInteractiveProcess(executable, arguments, workingDirectory, environment, + includeParentEnvironment, runInShell, stdoutEncoding, stderrEncoding); @@ -68,6 +72,7 @@ class _ProcessImpl extends NativeFieldWrapperClass1 implements Process { List arguments, String this._workingDirectory, Map environment, + bool includeParentEnvironment, bool runInShell) { runInShell = identical(runInShell, true); if (runInShell) { @@ -101,20 +106,23 @@ class _ProcessImpl extends NativeFieldWrapperClass1 implements Process { "WorkingDirectory is not a String: $_workingDirectory"); } - if (environment != null) { - var env = environment; - if (env is !Map) { - throw new ArgumentError("Environment is not a map: $env"); - } - _environment = []; - env.forEach((key, value) { - if (key is !String || value is !String) { - throw new ArgumentError( - "Environment key or value is not a string: ($key, $value)"); - } - _environment.add('$key=$value'); - }); + _environment = []; + if (environment == null) { + environment = {}; } + if (environment is !Map) { + throw new ArgumentError("Environment is not a map: $environment"); + } + if (identical(true, includeParentEnvironment)) { + environment = Platform.environment..addAll(environment); + } + environment.forEach((key, value) { + if (key is !String || value is !String) { + throw new ArgumentError( + "Environment key or value is not a string: ($key, $value)"); + } + _environment.add('$key=$value'); + }); // stdin going to process. _stdin = new _StdSink(new _Socket._writePipe()); @@ -331,6 +339,7 @@ Future _runNonInteractiveProcess(String path, List arguments, String workingDirectory, Map environment, + bool includeParentEnvironment, bool runInShell, Encoding stdoutEncoding, Encoding stderrEncoding) { @@ -339,6 +348,7 @@ Future _runNonInteractiveProcess(String path, arguments, workingDirectory: workingDirectory, environment: environment, + includeParentEnvironment: includeParentEnvironment, runInShell: runInShell).then((Process p) { int pid = p.pid; diff --git a/sdk/lib/_internal/pub/lib/src/io.dart b/sdk/lib/_internal/pub/lib/src/io.dart index 193bc4a8f28..f61f47e5f7f 100644 --- a/sdk/lib/_internal/pub/lib/src/io.dart +++ b/sdk/lib/_internal/pub/lib/src/io.dart @@ -591,19 +591,12 @@ Future _doProcess(Function fn, String executable, List args, executable = "cmd"; } - var env = null; - if (environment != null) { - env = new Map.from(Platform.environment); - environment.forEach((key, value) => env[key] = value); - } - - log.process(executable, args); return fn(executable, args, workingDirectory: workingDir, - environment: env); + environment: environment); } /// Wraps [input] to provide a timeout. If [input] completes before diff --git a/sdk/lib/_internal/pub/test/test_pub.dart b/sdk/lib/_internal/pub/test/test_pub.dart index 4813ee48d0a..a5fcd168826 100644 --- a/sdk/lib/_internal/pub/test/test_pub.dart +++ b/sdk/lib/_internal/pub/test/test_pub.dart @@ -453,8 +453,7 @@ ScheduledProcess startPub({List args, Future tokenEndpoint}) { if (tokenEndpoint == null) tokenEndpoint = new Future.value(); var environmentFuture = tokenEndpoint.then((tokenEndpoint) { - // TODO(nweiz): remove this when issue 9294 is fixed. - var environment = new Map.from(Platform.environment); + var environment = {}; environment['_PUB_TESTING'] = 'true'; environment['PUB_CACHE'] = pathInSandbox(cachePath); environment['DART_SDK'] = pathInSandbox(sdkPath); diff --git a/sdk/lib/io/process.dart b/sdk/lib/io/process.dart index 6c79c12d226..456d215941a 100644 --- a/sdk/lib/io/process.dart +++ b/sdk/lib/io/process.dart @@ -83,6 +83,10 @@ abstract class Process { * if an environment variable with code-points outside the US-ASCII range is * passed in. * + * If [includeParentEnvironment] is `true`, the process's environment will + * include the parent process's environment, with [environment] taking + * precedence. Default is `true`. + * * If [runInShell] is true, the process will be spawned through a system * shell. On Linux and Mac OS, [:/bin/sh:] is used, while * [:%WINDIR%\system32\cmd.exe:] is used on Windows. @@ -97,6 +101,7 @@ abstract class Process { List arguments, {String workingDirectory, Map environment, + bool includeParentEnvironment: true, bool runInShell: false}); /** @@ -114,6 +119,10 @@ abstract class Process { * if an environment variable with code-points outside the US-ASCII range is * passed in. * + * If [includeParentEnvironment] is `true`, the process's environment will + * include the parent process's environment, with [environment] taking + * precedence. Default is `true`. + * * If [runInShell] is true, the process will be spawned through a system * shell. On Linux and Mac OS, `/bin/sh` is used, while * `%WINDIR%\system32\cmd.exe` is used on Windows. @@ -133,6 +142,7 @@ abstract class Process { List arguments, {String workingDirectory, Map environment, + bool includeParentEnvironment: true, bool runInShell: false, Encoding stdoutEncoding: Encoding.SYSTEM, Encoding stderrEncoding: Encoding.SYSTEM}); diff --git a/tests/standalone/io/process_environment_test.dart b/tests/standalone/io/process_environment_test.dart index 24cc73d3265..5291751988a 100644 --- a/tests/standalone/io/process_environment_test.dart +++ b/tests/standalone/io/process_environment_test.dart @@ -7,7 +7,7 @@ import "dart:io"; import "dart:isolate"; import "process_test_util.dart"; -runEnvironmentProcess(Map environment, name, callback) { +runEnvironmentProcess(Map environment, name, includeParent, callback) { var dartExecutable = new Options().executable; var printEnv = 'tests/standalone/io/print_env.dart'; if (!new File(printEnv).existsSync()) { @@ -15,7 +15,8 @@ runEnvironmentProcess(Map environment, name, callback) { } Process.run(dartExecutable, [printEnv, name], - environment: environment) + environment: environment, + includeParentEnvironment: includeParent) .then((result) { Expect.equals(0, result.exitCode); callback(result.stdout); @@ -29,7 +30,7 @@ testEnvironment() { // Check that some value in the environment stays the same when passed // to another process. for (var k in env.keys) { - runEnvironmentProcess(env, k, (output) { + runEnvironmentProcess({}, k, true, (output) { // Only check startsWith. The print statements will add // newlines at the end. Expect.isTrue(output.startsWith(env[k])); @@ -39,7 +40,7 @@ testEnvironment() { var name = 'MYENVVAR'; while (env.containsKey(name)) name = '${name}_'; copy[name] = 'value'; - runEnvironmentProcess(copy, name, (output) { + runEnvironmentProcess(copy, name, true, (output) { Expect.isTrue(output.startsWith('value')); donePort.close(); }); @@ -50,6 +51,15 @@ testEnvironment() { } } +testNoIncludeEnvironment() { + var donePort = new ReceivePort(); + runEnvironmentProcess({}, "PATH", false, (output) { + donePort.close(); + Expect.isTrue(output.startsWith("null")); + }); +} + main() { testEnvironment(); + testNoIncludeEnvironment(); }