Skip to content

Commit

Permalink
stop reading .packages files, dont require them to exist at all (#3306)
Browse files Browse the repository at this point in the history
  • Loading branch information
jakemac53 authored May 16, 2022
1 parent de4f74f commit 274ab56
Show file tree
Hide file tree
Showing 17 changed files with 18 additions and 83 deletions.
2 changes: 1 addition & 1 deletion _test/test/common/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Future<void> startServer({bool? ensureCleanBuild, List<String>? buildArgs}) =>
_startServer(
'dart',
[
'--packages=.packages',
'--packages=.dart_tool/package_config.json',
p.join('..', 'build_runner', 'bin', 'build_runner.dart'),
'serve',
'--verbose',
Expand Down
3 changes: 2 additions & 1 deletion build_runner/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## 2.1.11-dev
## 2.1.11

- Stop reading or requiring `.packages` files.
- Use an explicit `dynamic` generic type for collections in default builder
options to reduce behavior differences between reading default options and
user provided options.
Expand Down
3 changes: 2 additions & 1 deletion build_runner/lib/src/entrypoint/run_script.dart
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ class RunCommand extends BuildRunnerCommand {

// Find the path of the script to run.
var scriptPath = p.join(tempPath, scriptName);
var packageConfigPath = p.join(tempPath, '.packages');
var packageConfigPath =
p.join(tempPath, '.dart_tool/package_config.json');

onExit = ReceivePort();
onError = ReceivePort();
Expand Down
2 changes: 1 addition & 1 deletion build_runner/lib/src/generate/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import 'watch_impl.dart' as watch_impl;
/// A [packageGraph] may be supplied, otherwise one will be constructed using
/// [PackageGraph.forThisPackage]. The default functionality assumes you are
/// running in the root directory of a package, with both a `pubspec.yaml` and
/// `.packages` file present.
/// `.dart_tool/package_config.json` file present.
///
/// A [reader] and [writer] may also be supplied, which can read/write assets
/// to arbitrary locations or file systems. By default they will write directly
Expand Down
11 changes: 2 additions & 9 deletions build_runner/lib/src/generate/watch_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,7 @@ class WatchImpl implements BuildState {
_logger.info('Terminating. No further builds will be scheduled\n');
});

Digest? originalRootPackagesDigest;
Digest? originalRootPackageConfigDigest;
final rootPackagesId = AssetId(packageGraph.root.name, '.packages');
final rootPackageConfigId =
AssetId(packageGraph.root.name, '.dart_tool/package_config.json');

Expand All @@ -275,11 +273,8 @@ class WatchImpl implements BuildState {
})
.asyncMap<AssetChange>((change) {
var id = change.id;
if (id == rootPackagesId || id == rootPackageConfigId) {
var digest = id == rootPackagesId
? originalRootPackagesDigest
: originalRootPackageConfigDigest;
assert(digest != null);
if (id == rootPackageConfigId) {
var digest = originalRootPackageConfigDigest!;
// Kill future builds if the root packages file changes.
//
// We retry the reads for a little bit to handle the case where a
Expand Down Expand Up @@ -337,8 +332,6 @@ class WatchImpl implements BuildState {
() async {
await logTimedAsync(_logger, 'Waiting for all file watchers to be ready',
() => graphWatcher.ready);
originalRootPackagesDigest = md5
.convert(await watcherEnvironment.reader.readAsBytes(rootPackagesId));
originalRootPackageConfigDigest = md5.convert(
await watcherEnvironment.reader.readAsBytes(rootPackageConfigId));

Expand Down
2 changes: 1 addition & 1 deletion build_runner/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: build_runner
version: 2.1.11-dev
version: 2.1.11
description: A build system for Dart code generation and modular compilation.
repository: https://github.com/dart-lang/build/tree/master/build_runner

Expand Down
4 changes: 0 additions & 4 deletions build_runner/test/generate/build_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ void main() {

setUp(() async {
writer = InMemoryRunnerAssetWriter();
await writer.writeAsString(makeAssetId('a|.packages'), '''
# Fake packages file
a:file://fake/pkg/path
''');
await writer.writeAsString(packageConfigId, jsonEncode(_packageConfig));
});

Expand Down
4 changes: 0 additions & 4 deletions build_runner/test/generate/serve_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ void main() {
setUp(() async {
_terminateServeController = StreamController();
writer = InMemoryRunnerAssetWriter();
await writer.writeAsString(makeAssetId('a|.packages'), '''
# Fake packages file
a:file://fake/pkg/path
''');
await writer.writeAsString(
makeAssetId('a|.dart_tool/package_config.json'),
jsonEncode({
Expand Down
28 changes: 0 additions & 28 deletions build_runner/test/generate/watch_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ void main() {

setUp(() async {
writer = InMemoryRunnerAssetWriter();
await writer.writeAsString(makeAssetId('a|.packages'), '''
# Fake packages file
a:file://fake/pkg/path
''');
await writer.writeAsString(packageConfigId, jsonEncode(_packageConfig));
});

Expand Down Expand Up @@ -364,30 +360,6 @@ a:file://fake/pkg/path
checkBuild(result, outputs: {'a|web/a.txt.copy': 'b'}, writer: writer);
});

test('edits to .packages prevent future builds and ask you to restart',
() async {
var logs = <LogRecord>[];
var buildState = await startWatch(
[copyABuildApplication], {'a|web/a.txt': 'a'}, writer,
logLevel: Level.SEVERE, onLog: logs.add);
var results = StreamQueue(buildState.buildResults);

var result = await results.next;
checkBuild(result, outputs: {'a|web/a.txt.copy': 'a'}, writer: writer);

await writer.writeAsString(AssetId('a', '.packages'), '''
# Fake packages file
a:file://different/fake/pkg/path
''');

expect(await results.hasNext, isFalse);
expect(logs, hasLength(1));
expect(
logs.first.message,
contains('Terminating builds due to package graph update, '
'please restart the build.'));
});

test(
'edits to .dart_tool/package_config.json prevent future builds '
'and ask you to restart', () async {
Expand Down
4 changes: 0 additions & 4 deletions build_runner/test/server/serve_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ void main() {
..cacheStringAsset(AssetId('example', 'web/initial.txt'), 'initial')
..cacheStringAsset(AssetId('example', 'web/large.txt'),
List.filled(10000, 'large').join(''))
..cacheStringAsset(
AssetId('example', '.packages'),
'# Fake packages file\n'
'example:file://fake/pkg/path')
..cacheStringAsset(
makeAssetId('example|.dart_tool/package_config.json'),
jsonEncode({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ void main() {
'packages/a/a.txt.copy': 'a',
'packages/b/c.txt': 'c',
'packages/b/c.txt.copy': 'c',
'.packages': 'a:packages/a/\r\nb:packages/b/\r\n\$sdk:packages/\$sdk/',
'.dart_tool/package_config.json':
_expectedPackageConfig('a', ['a', 'b']),
};
Expand Down Expand Up @@ -290,7 +289,6 @@ void main() {
'packages/a/a.txt': 'a',
'packages/b/c.txt': 'c',
'web/b.txt': 'b',
'.packages': 'a:packages/a/\r\nb:packages/b/\r\n\$sdk:packages/\$sdk/',
'.dart_tool/package_config.json':
_expectedPackageConfig('a', ['a', 'b'])
};
Expand Down Expand Up @@ -477,7 +475,6 @@ void _expectAllFiles(Directory dir) {
'packages/b/c.txt.copy': 'c',
'web/b.txt': 'b',
'web/b.txt.copy': 'b',
'.packages': 'a:packages/a/\r\nb:packages/b/\r\n\$sdk:packages/\$sdk/',
'.dart_tool/package_config.json': _expectedPackageConfig('a', ['a', 'b'])
};
_expectFiles(expectedFiles, dir);
Expand Down
3 changes: 0 additions & 3 deletions build_runner_core/test/generate/build_definition_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ void main() {
'pkg_a',
[
await pubspec('a'),
d.file('.packages', '\na:./lib/\nb:../pkg_b/lib/'),
d.file('pubspec.lock', 'packages: {}'),
d.dir('.dart_tool', [
d.dir('build', [
Expand Down Expand Up @@ -594,8 +593,6 @@ targets:
'pubspec.yaml',
(await readFile('pubspec.yaml'))
.replaceFirst('name: a', 'name: c'));
await modifyFile('.packages',
(await readFile('.packages')).replaceFirst('a:', 'c:'));
await modifyFile(
'.dart_tool/package_config.json',
(await readFile('.dart_tool/package_config.json'))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void main() {
throwsA(anything));
});

test('missing .packages file throws on create', () {
test('missing .dart_tool/package_config.json file throws on create', () {
expect(
() => PackageGraph.forPath(
p.join('test', 'fixtures', 'no_packages_file')),
Expand Down
2 changes: 2 additions & 0 deletions build_test/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
## 2.1.6-dev

## 2.1.5

- Allow the latest analyzer.
Expand Down
10 changes: 5 additions & 5 deletions build_test/lib/src/package_reader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import 'package:stream_transform/stream_transform.dart';
/// Resolves using a [PackageConfig] before reading from the file system.
///
/// For a simple implementation that uses the current isolate's package
/// resolution logic (i.e. whatever you have generated in `.packages` in most
/// cases), use [currentIsolate]:
/// resolution logic (i.e. whatever you have generated in
/// `.dart_tool/package_config.json` in most cases), use [currentIsolate]:
/// ```dart
/// var assetReader = await PackageAssetReader.currentIsolate();
/// ```
Expand Down Expand Up @@ -74,12 +74,12 @@ class PackageAssetReader extends AssetReader
/// A [rootPackage] should be provided for full API compatibility.
static Future<PackageAssetReader> currentIsolate(
{String? rootPackage}) async {
final packageConfig = await Isolate.packageConfig ??
final packageConfigUri = await Isolate.packageConfig ??
(throw UnsupportedError('No package config found'));
final configUri = await findPackageConfigUri(packageConfig) ??
final packageConfig = await findPackageConfigUri(packageConfigUri) ??
(throw UnsupportedError('Package configuration file not found'));

return PackageAssetReader(configUri, rootPackage);
return PackageAssetReader(packageConfig, rootPackage);
}

File? _resolve(AssetId id) {
Expand Down
2 changes: 1 addition & 1 deletion build_test/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: build_test
description: Utilities for writing unit tests of Builders.
version: 2.1.5
version: 2.1.6-dev
repository: https://github.com/dart-lang/build/tree/master/build_test

environment:
Expand Down
16 changes: 0 additions & 16 deletions build_web_compilers/lib/src/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@
// 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:async';
import 'dart:io';

import 'package:build/build.dart';
import 'package:build_modules/build_modules.dart';
import 'package:path/path.dart' as p;
import 'package:scratch_space/scratch_space.dart';

Expand All @@ -24,20 +22,6 @@ String sdkDdcKernelPath(bool soundNullSafety) => p.url.join('lib', '_internal',
String soundnessExt(bool soundNullSafety) =>
soundNullSafety ? '.sound' : '.unsound';

// TODO: better solution for a .packages file, today we just create a new one
// for every kernel build action.
Future<File> createPackagesFile(Iterable<AssetId> allAssets) async {
var allPackages = allAssets.map((id) => id.package).toSet();
var packagesFileDir =
await Directory.systemTemp.createTemp('kernel_builder_');
var packagesFile = File(p.join(packagesFileDir.path, '.packages'));
await packagesFile.create();
await packagesFile.writeAsString(allPackages
.map((pkg) => '$pkg:$multiRootScheme:///packages/$pkg')
.join('\r\n'));
return packagesFile;
}

/// Validates that [config] only has the top level keys [supportedOptions].
///
/// Throws an [ArgumentError] if not.
Expand Down

0 comments on commit 274ab56

Please sign in to comment.