Unverified Commit c25608d8 authored by Kevin Moore's avatar Kevin Moore Committed by GitHub

Provide more information in PUB_ENVIRONMENT for `pub get/upgrade` (#13307)

* Provide more information in PUB_ENVIRONMENT for `pub get/upgrade`

Should help diagnose and fix https://github.com/dart-lang/pub-dartlang-dart/issues/636
parent 8ebd45d3
...@@ -162,7 +162,7 @@ class CreateCommand extends FlutterCommand { ...@@ -162,7 +162,7 @@ class CreateCommand extends FlutterCommand {
generatedCount += _renderTemplate('package', dirPath, templateContext); generatedCount += _renderTemplate('package', dirPath, templateContext);
if (argResults['pub']) if (argResults['pub'])
await pubGet(directory: dirPath); await pubGet(context: 'create_pkg', directory: dirPath);
final String relativePath = fs.path.relative(dirPath); final String relativePath = fs.path.relative(dirPath);
printStatus('Wrote $generatedCount files.'); printStatus('Wrote $generatedCount files.');
...@@ -180,7 +180,7 @@ class CreateCommand extends FlutterCommand { ...@@ -180,7 +180,7 @@ class CreateCommand extends FlutterCommand {
generatedCount += _renderTemplate('plugin', dirPath, templateContext); generatedCount += _renderTemplate('plugin', dirPath, templateContext);
if (argResults['pub']) if (argResults['pub'])
await pubGet(directory: dirPath); await pubGet(context: 'create_plugin', directory: dirPath);
if (android_sdk.androidSdk != null) if (android_sdk.androidSdk != null)
gradle.updateLocalProperties(projectPath: dirPath); gradle.updateLocalProperties(projectPath: dirPath);
...@@ -218,7 +218,7 @@ class CreateCommand extends FlutterCommand { ...@@ -218,7 +218,7 @@ class CreateCommand extends FlutterCommand {
); );
if (argResults['pub']) { if (argResults['pub']) {
await pubGet(directory: appPath); await pubGet(context: 'create', directory: appPath);
injectPlugins(directory: appPath); injectPlugins(directory: appPath);
} }
......
...@@ -69,7 +69,7 @@ class PackagesGetCommand extends FlutterCommand { ...@@ -69,7 +69,7 @@ class PackagesGetCommand extends FlutterCommand {
); );
} }
await pubGet( await pubGet(context: 'get',
directory: target, directory: target,
upgrade: upgrade, upgrade: upgrade,
offline: argResults['offline'], offline: argResults['offline'],
...@@ -102,7 +102,7 @@ class PackagesTestCommand extends FlutterCommand { ...@@ -102,7 +102,7 @@ class PackagesTestCommand extends FlutterCommand {
} }
@override @override
Future<Null> runCommand() => pub(<String>['run', 'test']..addAll(argResults.rest), retry: false); Future<Null> runCommand() => pub(<String>['run', 'test']..addAll(argResults.rest), context: 'run_test', retry: false);
} }
class PackagesPassthroughCommand extends FlutterCommand { class PackagesPassthroughCommand extends FlutterCommand {
......
...@@ -146,7 +146,7 @@ class UpdatePackagesCommand extends FlutterCommand { ...@@ -146,7 +146,7 @@ class UpdatePackagesCommand extends FlutterCommand {
fakePackage.createSync(); fakePackage.createSync();
fakePackage.writeAsStringSync(_generateFakePubspec(dependencies.values)); fakePackage.writeAsStringSync(_generateFakePubspec(dependencies.values));
// First we run "pub upgrade" on this generated package: // First we run "pub upgrade" on this generated package:
await pubGet(directory: temporaryDirectory.path, upgrade: true, checkLastModified: false); await pubGet(context: 'update_packages', directory: temporaryDirectory.path, upgrade: true, checkLastModified: false);
// Then we run "pub deps --style=compact" on the result. We pipe all the // Then we run "pub deps --style=compact" on the result. We pipe all the
// output to tree.fill(), which parses it so that it can create a graph // output to tree.fill(), which parses it so that it can create a graph
// of all the dependencies so that we can figure out the transitive // of all the dependencies so that we can figure out the transitive
...@@ -154,6 +154,7 @@ class UpdatePackagesCommand extends FlutterCommand { ...@@ -154,6 +154,7 @@ class UpdatePackagesCommand extends FlutterCommand {
// each package. // each package.
await pub( await pub(
<String>['deps', '--style=compact'], <String>['deps', '--style=compact'],
context: 'update_pkgs',
directory: temporaryDirectory.path, directory: temporaryDirectory.path,
filter: tree.fill, filter: tree.fill,
retry: false, // errors here are usually fatal since we're not hitting the network retry: false, // errors here are usually fatal since we're not hitting the network
...@@ -210,7 +211,7 @@ class UpdatePackagesCommand extends FlutterCommand { ...@@ -210,7 +211,7 @@ class UpdatePackagesCommand extends FlutterCommand {
int count = 0; int count = 0;
for (Directory dir in packages) { for (Directory dir in packages) {
await pubGet(directory: dir.path, checkLastModified: false); await pubGet(context: 'update_packages', directory: dir.path, checkLastModified: false);
count += 1; count += 1;
} }
......
...@@ -68,7 +68,7 @@ class UpgradeCommand extends FlutterCommand { ...@@ -68,7 +68,7 @@ class UpgradeCommand extends FlutterCommand {
final String projRoot = findProjectRoot(); final String projRoot = findProjectRoot();
if (projRoot != null) { if (projRoot != null) {
printStatus(''); printStatus('');
await pubGet(directory: projRoot, upgrade: true, checkLastModified: false); await pubGet(context: 'upgrade', directory: projRoot, upgrade: true, checkLastModified: false);
} }
// Run a doctor check in case system requirements have changed. // Run a doctor check in case system requirements have changed.
......
...@@ -29,7 +29,10 @@ bool _shouldRunPubGet({ File pubSpecYaml, File dotPackages }) { ...@@ -29,7 +29,10 @@ bool _shouldRunPubGet({ File pubSpecYaml, File dotPackages }) {
return false; return false;
} }
/// [context] provides extra information to package server requests to
/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
Future<Null> pubGet({ Future<Null> pubGet({
@required String context,
String directory, String directory,
bool skipIfAbsent: false, bool skipIfAbsent: false,
bool upgrade: false, bool upgrade: false,
...@@ -59,6 +62,7 @@ Future<Null> pubGet({ ...@@ -59,6 +62,7 @@ Future<Null> pubGet({
try { try {
await pub( await pub(
args, args,
context: context,
directory: directory, directory: directory,
filter: _filterOverrideWarnings, filter: _filterOverrideWarnings,
failureMessage: 'pub $command failed', failureMessage: 'pub $command failed',
...@@ -84,7 +88,11 @@ typedef String MessageFilter(String message); ...@@ -84,7 +88,11 @@ typedef String MessageFilter(String message);
/// ///
/// The `--trace` argument is passed to `pub` (by mutating the provided /// The `--trace` argument is passed to `pub` (by mutating the provided
/// `arguments` list) unless `showTraceForErrors` is false. /// `arguments` list) unless `showTraceForErrors` is false.
///
/// [context] provides extra information to package server requests to
/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
Future<Null> pub(List<String> arguments, { Future<Null> pub(List<String> arguments, {
@required String context,
String directory, String directory,
MessageFilter filter, MessageFilter filter,
String failureMessage: 'pub failed', String failureMessage: 'pub failed',
...@@ -102,7 +110,7 @@ Future<Null> pub(List<String> arguments, { ...@@ -102,7 +110,7 @@ Future<Null> pub(List<String> arguments, {
_pubCommand(arguments), _pubCommand(arguments),
workingDirectory: directory, workingDirectory: directory,
mapFunction: filter, mapFunction: filter,
environment: _pubEnvironment, environment: _createPubEnvironment(context),
); );
if (code != 69) // UNAVAILABLE in https://github.com/dart-lang/pub/blob/master/lib/src/exit_codes.dart if (code != 69) // UNAVAILABLE in https://github.com/dart-lang/pub/blob/master/lib/src/exit_codes.dart
break; break;
...@@ -125,7 +133,7 @@ Future<Null> pubInteractively(List<String> arguments, { ...@@ -125,7 +133,7 @@ Future<Null> pubInteractively(List<String> arguments, {
final int code = await runInteractively( final int code = await runInteractively(
_pubCommand(arguments), _pubCommand(arguments),
workingDirectory: directory, workingDirectory: directory,
environment: _pubEnvironment, environment: _createPubEnvironment('interactive'),
); );
if (code != 0) if (code != 0)
throwToolExit('pub finished with exit code $code', exitCode: code); throwToolExit('pub finished with exit code $code', exitCode: code);
...@@ -137,9 +145,12 @@ List<String> _pubCommand(List<String> arguments) { ...@@ -137,9 +145,12 @@ List<String> _pubCommand(List<String> arguments) {
} }
/// The full environment used when running pub. /// The full environment used when running pub.
Map<String, String> get _pubEnvironment => <String, String>{ ///
/// [context] provides extra information to package server requests to
/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
Map<String, String> _createPubEnvironment(String context) => <String, String>{
'FLUTTER_ROOT': Cache.flutterRoot, 'FLUTTER_ROOT': Cache.flutterRoot,
_pubEnvironmentKey: _getPubEnvironmentValue(), _pubEnvironmentKey: _getPubEnvironmentValue(context),
}; };
final RegExp _analyzerWarning = new RegExp(r'^! \w+ [^ ]+ from path \.\./\.\./bin/cache/dart-sdk/lib/\w+$'); final RegExp _analyzerWarning = new RegExp(r'^! \w+ [^ ]+ from path \.\./\.\./bin/cache/dart-sdk/lib/\w+$');
...@@ -147,10 +158,15 @@ final RegExp _analyzerWarning = new RegExp(r'^! \w+ [^ ]+ from path \.\./\.\./bi ...@@ -147,10 +158,15 @@ final RegExp _analyzerWarning = new RegExp(r'^! \w+ [^ ]+ from path \.\./\.\./bi
/// The console environment key used by the pub tool. /// The console environment key used by the pub tool.
const String _pubEnvironmentKey = 'PUB_ENVIRONMENT'; const String _pubEnvironmentKey = 'PUB_ENVIRONMENT';
final RegExp _validContext = new RegExp('[a-z][a-z_]*[a-z]');
/// Returns the environment value that should be used when running pub. /// Returns the environment value that should be used when running pub.
/// ///
/// Includes any existing environment variable, if one exists. /// Includes any existing environment variable, if one exists.
String _getPubEnvironmentValue() { ///
/// [context] provides extra information to package server requests to
/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
String _getPubEnvironmentValue(String pubContext) {
final List<String> values = <String>[]; final List<String> values = <String>[];
final String existing = platform.environment[_pubEnvironmentKey]; final String existing = platform.environment[_pubEnvironmentKey];
...@@ -165,6 +181,11 @@ String _getPubEnvironmentValue() { ...@@ -165,6 +181,11 @@ String _getPubEnvironmentValue() {
values.add('flutter_cli'); values.add('flutter_cli');
if (!_validContext.hasMatch(pubContext)) {
throw new ArgumentError.value(pubContext, 'pubContext', 'Must match RegExp ${_validContext.pattern}');
}
values.add('ctx_$pubContext');
return values.join(':'); return values.join(':');
} }
......
...@@ -242,7 +242,7 @@ abstract class FlutterCommand extends Command<Null> { ...@@ -242,7 +242,7 @@ abstract class FlutterCommand extends Command<Null> {
await cache.updateAll(); await cache.updateAll();
if (shouldRunPub) if (shouldRunPub)
await pubGet(); await pubGet(context: 'verify');
setupApplicationPackages(); setupApplicationPackages();
......
...@@ -32,7 +32,7 @@ void main() { ...@@ -32,7 +32,7 @@ void main() {
testUsingContext('AnalysisServer success', () async { testUsingContext('AnalysisServer success', () async {
_createSampleProject(tempDir); _createSampleProject(tempDir);
await pubGet(directory: tempDir.path); await pubGet(context: 'flutter_tests', directory: tempDir.path);
server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]); server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
...@@ -52,7 +52,7 @@ void main() { ...@@ -52,7 +52,7 @@ void main() {
testUsingContext('AnalysisServer errors', () async { testUsingContext('AnalysisServer errors', () async {
_createSampleProject(tempDir, brokenCode: true); _createSampleProject(tempDir, brokenCode: true);
await pubGet(directory: tempDir.path); await pubGet(context: 'flutter_tests', directory: tempDir.path);
server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]); server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
......
...@@ -6,6 +6,7 @@ import 'dart:async'; ...@@ -6,6 +6,7 @@ import 'dart:async';
import 'package:file/file.dart'; import 'package:file/file.dart';
import 'package:file/memory.dart'; import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/dart/pub.dart'; import 'package:flutter_tools/src/dart/pub.dart';
import 'package:mockito/mockito.dart'; import 'package:mockito/mockito.dart';
...@@ -18,8 +19,12 @@ import '../src/context.dart'; ...@@ -18,8 +19,12 @@ import '../src/context.dart';
void main() { void main() {
testUsingContext('pub get 69', () async { testUsingContext('pub get 69', () async {
String error; String error;
final MockProcessManager processMock = context.getVariable(ProcessManager);
new FakeAsync().run((FakeAsync time) { new FakeAsync().run((FakeAsync time) {
pubGet(checkLastModified: false).then((Null value) { expect(processMock.lastPubEnvironmment, isNull);
pubGet(context: 'flutter_tests', checkLastModified: false).then((Null value) {
error = 'test completed unexpectedly'; error = 'test completed unexpectedly';
}, onError: (dynamic error) { }, onError: (dynamic error) {
error = 'test failed unexpectedly'; error = 'test failed unexpectedly';
...@@ -30,6 +35,7 @@ void main() { ...@@ -30,6 +35,7 @@ void main() {
'Running "flutter packages get" in /...\n' 'Running "flutter packages get" in /...\n'
'pub get failed (69) -- attempting retry 1 in 1 second...\n' 'pub get failed (69) -- attempting retry 1 in 1 second...\n'
); );
expect(processMock.lastPubEnvironmment, contains('flutter_cli:ctx_flutter_tests'));
time.elapse(const Duration(milliseconds: 500)); time.elapse(const Duration(milliseconds: 500));
expect(testLogger.statusText, expect(testLogger.statusText,
'Running "flutter packages get" in /...\n' 'Running "flutter packages get" in /...\n'
...@@ -83,6 +89,8 @@ class MockProcessManager implements ProcessManager { ...@@ -83,6 +89,8 @@ class MockProcessManager implements ProcessManager {
final int fakeExitCode; final int fakeExitCode;
String lastPubEnvironmment;
@override @override
Future<Process> start( Future<Process> start(
List<dynamic> command, { List<dynamic> command, {
...@@ -92,6 +100,7 @@ class MockProcessManager implements ProcessManager { ...@@ -92,6 +100,7 @@ class MockProcessManager implements ProcessManager {
bool runInShell: false, bool runInShell: false,
ProcessStartMode mode: ProcessStartMode.NORMAL, ProcessStartMode mode: ProcessStartMode.NORMAL,
}) { }) {
lastPubEnvironmment = environment['PUB_ENVIRONMENT'];
return new Future<Process>.value(new MockProcess(fakeExitCode)); return new Future<Process>.value(new MockProcess(fakeExitCode));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment