Skip to content

Commit dfa3203

Browse files
jcollins-gcommit-bot@chromium.org
jcollins-g
authored and
commit-bot@chromium.org
committed
Add scaffolding to trial_migration for more package sources.
Imports a subprocess helper with JSON parsing borrowed from Dartdoc, which will come in handy both for launching pub and for building the trial migration script out for comparing output between versions. Also puts some basic structure in to allow for fetching packages for multiple sources. Change-Id: I5262b93aa1d60005ee262b7ed00bf534b1c51447 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127446 Commit-Queue: Janice Collins <jcollins@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Reviewed-by: Paul Berry <paulberry@google.com>
1 parent d85eccb commit dfa3203

File tree

3 files changed

+301
-30
lines changed

3 files changed

+301
-30
lines changed

‎pkg/nnbd_migration/tool/src/package.dart‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
/// Abstractions for the different sources of truth for different packages.
6+
7+
import 'dart:convert';
8+
import 'dart:io';
9+
10+
import 'package:path/path.dart' as path;
11+
12+
/// Returns the path to the SDK repository this script is a part of.
13+
final String thisSdkRepo = () {
14+
var maybeSdkRepoDir = Platform.script.toFilePath();
15+
while (maybeSdkRepoDir != path.dirname(maybeSdkRepoDir)) {
16+
maybeSdkRepoDir = path.dirname(maybeSdkRepoDir);
17+
if (File(path.join(maybeSdkRepoDir, 'README.dart-sdk')).existsSync()) {
18+
return maybeSdkRepoDir;
19+
}
20+
}
21+
throw UnsupportedError(
22+
'Script ${Platform.script} using this library must be within the SDK repository');
23+
}();
24+
25+
Uri get thisSdkUri => Uri.file(thisSdkRepo);
26+
27+
/// Abstraction for a package fetched via Github.
28+
class GitHubPackage extends Package {
29+
GitHubPackage(String name, [String label]) : super(name) {
30+
throw UnimplementedError();
31+
}
32+
33+
@override
34+
// TODO: implement packagePath
35+
String get packagePath => null;
36+
}
37+
38+
/// Abstraction for a package fetched via pub.
39+
class PubPackage extends Package {
40+
PubPackage(String name, [String version]) : super(name) {
41+
throw UnimplementedError();
42+
}
43+
44+
@override
45+
// TODO: implement packagePath
46+
String get packagePath => null;
47+
}
48+
49+
/// Abstraction for a package located within pkg or third_party/pkg.
50+
class SdkPackage extends Package {
51+
/// Where to find packages. Constructor searches in-order.
52+
static List<String> _searchPaths = [
53+
'pkg',
54+
path.join('third_party', 'pkg'),
55+
];
56+
57+
SdkPackage(String name) : super(name) {
58+
for (String potentialPath
59+
in _searchPaths.map((p) => path.join(thisSdkRepo, p, name))) {
60+
if (Directory(potentialPath).existsSync()) {
61+
_packagePath = potentialPath;
62+
}
63+
}
64+
if (_packagePath == null)
65+
throw ArgumentError('Package $name not found in SDK');
66+
}
67+
68+
/* late final */ String _packagePath;
69+
@override
70+
String get packagePath => _packagePath;
71+
72+
@override
73+
String toString() => path.relative(packagePath, from: thisSdkRepo);
74+
}
75+
76+
/// Base class for pub, github, SDK, or possibly other package sources.
77+
abstract class Package {
78+
final String name;
79+
80+
Package(this.name);
81+
82+
/// Returns the root directory of the package.
83+
String get packagePath;
84+
85+
@override
86+
String toString() => name;
87+
}
88+
89+
/// Abstraction for compiled Dart SDKs (not this repository).
90+
class Sdk {
91+
/// The root of the compiled SDK.
92+
/* late final */ String sdkPath;
93+
94+
Sdk(String sdkPath) {
95+
this.sdkPath = path.canonicalize(sdkPath);
96+
}
97+
98+
/// Returns true if the SDK was built with --nnbd.
99+
///
100+
/// May throw if [sdkPath] is invalid, or there is an error parsing
101+
/// the libraries.json file.
102+
bool get isNnbdSdk {
103+
// TODO(jcollins-g): contact eng-prod for a more foolproof detection method
104+
String libraries = path.join(sdkPath, 'lib', 'libraries.json');
105+
var decodedJson = JsonDecoder().convert(File(libraries).readAsStringSync());
106+
return ((decodedJson['comment:1'] as String).contains('sdk_nnbd'));
107+
}
108+
}

‎pkg/nnbd_migration/tool/src/subprocess_launcher.dart‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// TODO(jcollins-g): Merge this with similar utilities in dartdoc
6+
// and extract into a separate package, generate testing and mirrors, and
7+
// reimport that into the SDK, before cut and paste gets out of hand.
8+
9+
/// This is a modified version of dartdoc's
10+
/// SubprocessLauncher from test/src/utils.dart, for use with the
11+
/// nnbd_migration script.
12+
13+
import 'dart:convert';
14+
import 'dart:io';
15+
16+
final RegExp quotables = RegExp(r'[ "\r\n\$]');
17+
18+
/// SubprocessLauncher manages one or more launched, non-interactive
19+
/// subprocesses. It handles I/O streams, parses JSON output if
20+
/// available, and logs debugging information so the user can see exactly
21+
/// what was run.
22+
class SubprocessLauncher {
23+
final String context;
24+
final Map<String, String> environmentDefaults;
25+
26+
String get prefix => context.isNotEmpty ? '$context: ' : '';
27+
28+
/// From flutter:dev/tools/dartdoc.dart, modified.
29+
static Future<void> _printStream(Stream<List<int>> stream, Stdout output,
30+
{String prefix = '', Iterable<String> Function(String line) filter}) {
31+
assert(prefix != null);
32+
if (filter == null) filter = (line) => [line];
33+
return stream
34+
.transform(utf8.decoder)
35+
.transform(const LineSplitter())
36+
.expand(filter)
37+
.listen((String line) {
38+
if (line != null) {
39+
output.write('$prefix$line'.trim());
40+
output.write('\n');
41+
}
42+
}).asFuture();
43+
}
44+
45+
SubprocessLauncher(this.context, [Map<String, String> environment])
46+
: this.environmentDefaults = environment ?? <String, String>{};
47+
48+
/// A wrapper around start/await process.exitCode that will display the
49+
/// output of the executable continuously and fail on non-zero exit codes.
50+
/// It will also parse any valid JSON objects (one per line) it encounters
51+
/// on stdout/stderr, and return them. Returns null if no JSON objects
52+
/// were encountered, or if DRY_RUN is set to 1 in the execution environment.
53+
///
54+
/// Makes running programs in grinder similar to set -ex for bash, even on
55+
/// Windows (though some of the bashisms will no longer make sense).
56+
/// TODO(jcollins-g): refactor to return a stream of stderr/stdout lines
57+
/// and their associated JSON objects.
58+
Future<Iterable<Map>> runStreamed(String executable, List<String> arguments,
59+
{String workingDirectory,
60+
Map<String, String> environment,
61+
bool includeParentEnvironment = true,
62+
void Function(String) perLine}) async {
63+
environment ??= {};
64+
environment.addAll(environmentDefaults);
65+
List<Map> jsonObjects;
66+
67+
/// Parses json objects generated by the subprocess. If a json object
68+
/// contains the key 'message' or the keys 'data' and 'text', return that
69+
/// value as a collection of lines suitable for printing.
70+
Iterable<String> jsonCallback(String line) {
71+
if (perLine != null) perLine(line);
72+
Map result;
73+
try {
74+
result = json.decoder.convert(line) as Map;
75+
} catch (FormatException) {
76+
// ignore
77+
}
78+
if (result != null) {
79+
jsonObjects ??= [];
80+
jsonObjects.add(result);
81+
if (result.containsKey('message')) {
82+
line = result['message'] as String;
83+
} else if (result.containsKey('data') &&
84+
result['data'] is Map &&
85+
(result['data'] as Map).containsKey('key')) {
86+
line = result['data']['text'] as String;
87+
}
88+
}
89+
return line.split('\n');
90+
}
91+
92+
stderr.write('$prefix+ ');
93+
if (workingDirectory != null) stderr.write('(cd "$workingDirectory" && ');
94+
if (environment != null) {
95+
stderr.write(environment.keys.map((String key) {
96+
if (environment[key].contains(quotables)) {
97+
return "$key='${environment[key]}'";
98+
} else {
99+
return "$key=${environment[key]}";
100+
}
101+
}).join(' '));
102+
stderr.write(' ');
103+
}
104+
stderr.write('$executable');
105+
if (arguments.isNotEmpty) {
106+
for (String arg in arguments) {
107+
if (arg.contains(quotables)) {
108+
stderr.write(" '$arg'");
109+
} else {
110+
stderr.write(" $arg");
111+
}
112+
}
113+
}
114+
if (workingDirectory != null) stderr.write(')');
115+
stderr.write('\n');
116+
117+
if (Platform.environment.containsKey('DRY_RUN')) return null;
118+
119+
String realExecutable = executable;
120+
final List<String> realArguments = [];
121+
if (Platform.isLinux) {
122+
// Use GNU coreutils to force line buffering. This makes sure that
123+
// subprocesses that die due to fatal signals do not chop off the
124+
// last few lines of their output.
125+
//
126+
// Dart does not actually do this (seems to flush manually) unless
127+
// the VM crashes.
128+
realExecutable = 'stdbuf';
129+
realArguments.addAll(['-o', 'L', '-e', 'L']);
130+
realArguments.add(executable);
131+
}
132+
realArguments.addAll(arguments);
133+
134+
Process process = await Process.start(realExecutable, realArguments,
135+
workingDirectory: workingDirectory,
136+
environment: environment,
137+
includeParentEnvironment: includeParentEnvironment);
138+
Future<void> stdoutFuture = _printStream(process.stdout, stdout,
139+
prefix: prefix, filter: jsonCallback);
140+
Future<void> stderrFuture = _printStream(process.stderr, stderr,
141+
prefix: prefix, filter: jsonCallback);
142+
await Future.wait([stderrFuture, stdoutFuture, process.exitCode]);
143+
144+
int exitCode = await process.exitCode;
145+
if (exitCode != 0) {
146+
throw ProcessException(executable, arguments,
147+
"SubprocessLauncher got non-zero exitCode: $exitCode", exitCode);
148+
}
149+
return jsonObjects;
150+
}
151+
}

‎pkg/nnbd_migration/tool/trial_migration.dart‎

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
// result of migration, as well as categories (and counts) of exceptions that
99
// occurred.
1010

11-
import 'dart:convert';
1211
import 'dart:io';
1312

1413
import 'package:analyzer/src/dart/analysis/analysis_context_collection.dart';
@@ -19,6 +18,8 @@ import 'package:args/args.dart';
1918
import 'package:nnbd_migration/nnbd_migration.dart';
2019
import 'package:path/path.dart' as path;
2120

21+
import 'src/package.dart';
22+
2223
main(List<String> args) async {
2324
ArgParser argParser = ArgParser();
2425
ArgResults parsedArgs;
@@ -31,6 +32,29 @@ main(List<String> args) async {
3132
help: 'Select the root of the SDK to analyze against for this run '
3233
'(compiled with --nnbd). For example: ../../xcodebuild/DebugX64NNBD/dart-sdk');
3334

35+
argParser.addMultiOption(
36+
'packages',
37+
abbr: 'p',
38+
defaultsTo: [
39+
'charcode',
40+
'collection',
41+
'logging',
42+
'meta',
43+
'path',
44+
'term_glyph',
45+
'typed_data',
46+
'async',
47+
'source_span',
48+
'stack_trace',
49+
'matcher',
50+
'stream_channel',
51+
'boolean_selector',
52+
path.join('test', 'pkgs', 'test_api'),
53+
],
54+
help: 'The list of packages to run the migration against.',
55+
splitCommas: true,
56+
);
57+
3458
try {
3559
parsedArgs = argParser.parse(args);
3660
} on ArgParserException {
@@ -46,35 +70,25 @@ main(List<String> args) async {
4670
throw 'invalid args. Specify *one* argument to get exceptions of interest.';
4771
}
4872

49-
String sdkPath = path.canonicalize(parsedArgs['sdk'] as String);
73+
Sdk sdk = Sdk(parsedArgs['sdk'] as String);
5074

5175
warnOnNoAssertions();
52-
warnOnNoSdkNnbd(sdkPath);
76+
warnOnNoSdkNnbd(sdk);
77+
78+
List<Package> packages = (parsedArgs['packages'] as Iterable<String>)
79+
.map((n) => SdkPackage(n))
80+
.toList(growable: false);
5381

5482
String categoryOfInterest =
5583
parsedArgs.rest.isEmpty ? null : parsedArgs.rest.single;
56-
var rootUri = Platform.script.resolve('../../..');
84+
5785
var listener = _Listener(categoryOfInterest);
58-
for (var testPath in [
59-
'third_party/pkg/charcode',
60-
'third_party/pkg/collection',
61-
'third_party/pkg/logging',
62-
'pkg/meta',
63-
'third_party/pkg/path',
64-
'third_party/pkg/term_glyph',
65-
'third_party/pkg/typed_data',
66-
'third_party/pkg/async',
67-
'third_party/pkg/source_span',
68-
'third_party/pkg/stack_trace',
69-
'third_party/pkg/matcher',
70-
'third_party/pkg/stream_channel',
71-
'third_party/pkg/boolean_selector',
72-
'third_party/pkg/test/pkgs/test_api',
73-
]) {
74-
print('Migrating $testPath');
75-
var testUri = rootUri.resolve(testPath);
86+
for (var package in packages) {
87+
print('Migrating $package');
88+
var testUri = thisSdkUri.resolve(package.packagePath);
7689
var contextCollection = AnalysisContextCollectionImpl(
77-
includedPaths: [testUri.toFilePath()], sdkPath: sdkPath);
90+
includedPaths: [testUri.toFilePath()], sdkPath: sdk.sdkPath);
91+
7892
var context = contextCollection.contexts.single;
7993
var files = context.contextRoot
8094
.analyzedFiles()
@@ -133,17 +147,15 @@ void warnOnNoAssertions() {
133147
printWarning("You didn't --enable-asserts!");
134148
}
135149

136-
void warnOnNoSdkNnbd(String sdkPath) {
137-
// TODO(jcollins-g): contact eng-prod for a more foolproof detection method
138-
String libraries = path.join(sdkPath, 'lib', 'libraries.json');
150+
void warnOnNoSdkNnbd(Sdk sdk) {
139151
try {
140-
var decodedJson = JsonDecoder().convert(File(libraries).readAsStringSync());
141-
if ((decodedJson['comment:1'] as String).contains('sdk_nnbd')) return;
142-
} on Exception {
152+
if (sdk.isNnbdSdk) return;
153+
} catch (e) {
143154
printWarning('Unable to determine whether this SDK supports NNBD');
144155
return;
145156
}
146-
printWarning('SDK at $sdkPath not compiled with --nnbd, use --sdk option');
157+
printWarning(
158+
'SDK at ${sdk.sdkPath} not compiled with --nnbd, use --sdk option');
147159
}
148160

149161
class _Listener implements NullabilityMigrationListener {

0 commit comments

Comments
 (0)