From 7bcddc3279e723d878544cfebcf0258ce0b5ab3e Mon Sep 17 00:00:00 2001
From: "rnystrom@google.com" <rnystrom@google.com>
Date: Thu, 25 Sep 2014 00:45:14 +0000
Subject: [PATCH] Tell the user if the binstub directory is not on their path.

R=nweiz@google.com

Review URL: https://codereview.chromium.org//601933002

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@40662 260f80e4-7a28-3924-810f-c04153c831b5
---
 lib/src/global_packages.dart                  | 46 ++++++++++++++++++-
 .../does_not_warn_if_no_executables_test.dart | 27 +++++++++++
 .../does_not_warn_if_on_path_test.dart        | 37 +++++++++++++++
 .../global/binstubs/warns_if_not_on_path.dart | 28 +++++++++++
 test/test_pub.dart                            | 24 +++++++---
 5 files changed, 153 insertions(+), 9 deletions(-)
 create mode 100644 test/global/binstubs/does_not_warn_if_no_executables_test.dart
 create mode 100644 test/global/binstubs/does_not_warn_if_on_path_test.dart
 create mode 100644 test/global/binstubs/warns_if_not_on_path.dart

diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart
index 479a486c..93acb294 100644
--- a/lib/src/global_packages.dart
+++ b/lib/src/global_packages.dart
@@ -469,8 +469,6 @@ class GlobalPackages {
     if (installed.isNotEmpty) {
       var names = namedSequence("executable", installed.map(log.bold));
       log.message("Installed $names.");
-      // TODO(rnystrom): Show the user how to add the binstub directory to
-      // their PATH if not already on it.
     }
 
     // Show errors for any collisions.
@@ -514,6 +512,8 @@ class GlobalPackages {
             'which was not found in ${log.bold(package.name)}.');
       }
     }
+
+    if (installed.isNotEmpty) _suggestIfNotOnPath(installed);
   }
 
   /// Creates a binstub named [executable] that runs [script] from [package].
@@ -617,4 +617,46 @@ $invocation "\$@"
       }
     }
   }
+
+  /// Checks to see if the binstubs are on the user's PATH and, if not, suggests
+  /// that the user add the directory to their PATH.
+  void _suggestIfNotOnPath(List<String> installed) {
+    if (Platform.operatingSystem == "windows") {
+      // See if the shell can find one of the binstubs.
+      // "\q" means return exit code 0 if found or 1 if not.
+      var result = Process.runSync("where", r"\q", [installed.first]);
+      if (result.exitCode == 0) return;
+
+      var binDir = _binStubDir;
+      if (binDir.startsWith(Platform.environment['APPDATA'])) {
+        binDir = p.join("%APPDATA%", p.relative(binDir,
+            from: Platform.environment['APPDATA']));
+      }
+
+      log.warning(
+          "${log.yellow('Warning:')} Pub installs executables into "
+              "${log.bold(binDir)}, which is not on your path.\n"
+          "You can fix that by adding that directory to your system's "
+              '"Path" environment variable.\n'
+          'A web search for "configure windows path" will show you how.');
+    } else {
+      // See if the shell can find one of the binstubs.
+      var result = Process.runSync("which", [installed.first]);
+      if (result.exitCode == 0) return;
+
+      var binDir = _binStubDir;
+      if (binDir.startsWith(Platform.environment['HOME'])) {
+        binDir = p.join("~", p.relative(binDir,
+            from: Platform.environment['HOME']));
+      }
+
+      log.warning(
+          "${log.yellow('Warning:')} Pub installs executables into "
+              "${log.bold(binDir)}, which is not on your path.\n"
+          "You can fix that by adding this to your shell's config file "
+              "(.bashrc, .bash_profile, etc.):\n"
+          "\n"
+          "\n${log.bold('export PATH="\$PATH":"$binDir"')}\n");
+    }
+  }
 }
diff --git a/test/global/binstubs/does_not_warn_if_no_executables_test.dart b/test/global/binstubs/does_not_warn_if_no_executables_test.dart
new file mode 100644
index 00000000..565e23af
--- /dev/null
+++ b/test/global/binstubs/does_not_warn_if_no_executables_test.dart
@@ -0,0 +1,27 @@
+// Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+import 'package:scheduled_test/scheduled_test.dart';
+
+import '../../descriptor.dart' as d;
+import '../../test_pub.dart';
+
+main() {
+  initConfig();
+  integration("does not warn if the package has no executables", () {
+    servePackages((builder) {
+      builder.serve("foo", "1.0.0", contents: [
+        d.dir("bin", [
+          d.file("script.dart", "main(args) => print('ok \$args');")
+        ])
+      ]);
+    });
+
+    schedulePub(args: ["global", "activate", "foo"],
+        output: isNot(contains("is not on your path")));
+  });
+}
\ No newline at end of file
diff --git a/test/global/binstubs/does_not_warn_if_on_path_test.dart b/test/global/binstubs/does_not_warn_if_on_path_test.dart
new file mode 100644
index 00000000..124d1679
--- /dev/null
+++ b/test/global/binstubs/does_not_warn_if_on_path_test.dart
@@ -0,0 +1,37 @@
+// Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+import 'package:scheduled_test/scheduled_test.dart';
+
+import '../../descriptor.dart' as d;
+import '../../test_pub.dart';
+
+main() {
+  initConfig();
+  integration("does not warn if the binstub directory is on the path", () {
+    servePackages((builder) {
+      builder.serve("foo", "1.0.0", pubspec: {
+        "executables": {
+          "script": null
+        }
+      }, contents: [
+        d.dir("bin", [
+          d.file("script.dart", "main(args) => print('ok \$args');")
+        ])
+      ]);
+    });
+
+    // Add the test's cache bin directory to the path.
+    var binDir = p.dirname(Platform.executable);
+    var separator = Platform.operatingSystem == "windows" ? ";" : ":";
+    var path = "${Platform.environment["PATH"]}$separator$binDir";
+
+    schedulePub(args: ["global", "activate", "foo"],
+        output: isNot(contains("is not on your path")),
+        environment: {"PATH": path});
+  });
+}
\ No newline at end of file
diff --git a/test/global/binstubs/warns_if_not_on_path.dart b/test/global/binstubs/warns_if_not_on_path.dart
new file mode 100644
index 00000000..8ea5d2e3
--- /dev/null
+++ b/test/global/binstubs/warns_if_not_on_path.dart
@@ -0,0 +1,28 @@
+// Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:scheduled_test/scheduled_test.dart';
+
+import '../../descriptor.dart' as d;
+import '../../test_pub.dart';
+
+main() {
+  initConfig();
+  integration("warns if the binstub directory is not on the path", () {
+    servePackages((builder) {
+      builder.serve("foo", "1.0.0", pubspec: {
+        "executables": {
+          "script": null
+        }
+      }, contents: [
+        d.dir("bin", [
+          d.file("script.dart", "main(args) => print('ok \$args');")
+        ])
+      ]);
+    });
+
+    schedulePub(args: ["global", "activate", "foo"],
+        output: contains("is not on your path"));
+  });
+}
diff --git a/test/test_pub.dart b/test/test_pub.dart
index bb3f6a21..c60d1bbe 100644
--- a/test/test_pub.dart
+++ b/test/test_pub.dart
@@ -411,12 +411,15 @@ void scheduleSymlink(String target, String symlink) {
 /// If [outputJson] is given, validates that pub outputs stringified JSON
 /// matching that object, which can be a literal JSON object or any other
 /// [Matcher].
+///
+/// If [environment] is given, any keys in it will override the environment
+/// variables passed to the spawned process.
 void schedulePub({List args, output, error, outputJson,
-    int exitCode: exit_codes.SUCCESS}) {
+    int exitCode: exit_codes.SUCCESS, Map<String, String> environment}) {
   // Cannot pass both output and outputJson.
   assert(output == null || outputJson == null);
 
-  var pub = startPub(args: args);
+  var pub = startPub(args: args, environment: environment);
   pub.shouldExit(exitCode);
 
   var failures = [];
@@ -502,7 +505,11 @@ Map getPubTestEnvironment([String tokenEndpoint]) {
 /// interaction with that process.
 ///
 /// Any futures in [args] will be resolved before the process is started.
-ScheduledProcess startPub({List args, Future<String> tokenEndpoint}) {
+///
+/// If [environment] is given, any keys in it will override the environment
+/// variables passed to the spawned process.
+ScheduledProcess startPub({List args, Future<String> tokenEndpoint,
+    Map<String, String> environment}) {
   ensureDir(_pathInSandbox(appPath));
 
   // Find a Dart executable we can use to spawn. Use the same one that was
@@ -528,18 +535,21 @@ ScheduledProcess startPub({List args, Future<String> tokenEndpoint}) {
 
   if (tokenEndpoint == null) tokenEndpoint = new Future.value();
   var environmentFuture = tokenEndpoint.then((tokenEndpoint) {
-    var environment = getPubTestEnvironment(tokenEndpoint);
+    var pubEnvironment = getPubTestEnvironment(tokenEndpoint);
 
     // If there is a server running, tell pub what its URL is so hosted
     // dependencies will look there.
     if (_hasServer) {
       return port.then((p) {
-        environment['PUB_HOSTED_URL'] = "http://localhost:$p";
-        return environment;
+        pubEnvironment['PUB_HOSTED_URL'] = "http://localhost:$p";
+        return pubEnvironment;
       });
     }
 
-    return environment;
+    return pubEnvironment;
+  }).then((pubEnvironment) {
+    if (environment != null) pubEnvironment.addAll(environment);
+    return pubEnvironment;
   });
 
   return new PubProcess.start(dartBin, dartArgs, environment: environmentFuture,
-- 
GitLab