Skip to content

Commit c839ac2

Browse files
authored
Fix package root resolution after build_runner 2.14.0 AOT changes (#750)
## What does this change? This fixes package root resolution in `flutter_gen_runner` after the `build_runner 2.14.0` change that runs `build` / `watch` through AOT by default. Previously, `FlutterGenBuilder` relied on `Isolate.packageConfigSync!` as the only source for package roots. With the newer AOT build-script flow, the runtime isolate package config can be null or no longer represent the active build graph, which may crash the builder or fail to resolve the target package root, especially on Windows. The package root is now resolved in this order: 1. Use `BuildStep.packageConfig` as the primary source for the active build graph. 2. Fall back to the runtime isolate package config when available. 3. Fall back to discovering the package from `.dart_tool/package_config.json` or a matching `pubspec.yaml` under the current working directory. This also removes the eager static runtime package config load, avoiding the null assertion crash from `Isolate.packageConfigSync!`. Fixes #749 🎯
1 parent 59626d9 commit c839ac2

3 files changed

Lines changed: 147 additions & 36 deletions

File tree

packages/runner/lib/flutter_gen_runner.dart

Lines changed: 137 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import 'dart:convert';
2-
import 'dart:io' show File;
2+
import 'dart:io' show Directory, File;
33
import 'dart:isolate';
44

55
import 'package:build/build.dart';
@@ -41,17 +41,6 @@ class FlutterGenBuilder extends Builder {
4141

4242
final BuilderOptions _options;
4343

44-
/// We resolve package roots from the runtime package configuration of the
45-
/// build script isolate.
46-
///
47-
/// `buildStep.packageConfig` is package-aware, but the URIs exposed there are
48-
/// asset-style URIs. The legacy generator code still needs a real file-system
49-
/// root so it can reuse the existing `dart:io` based generation pipeline.
50-
/// Loading the isolate package config once gives us stable `file:` package
51-
/// roots for all packages in the build graph, including workspace members.
52-
static final Future<PackageConfig> _runtimePackageConfig =
53-
loadPackageConfigUri(Isolate.packageConfigSync!);
54-
5544
@override
5645
Future<void> build(BuildStep buildStep) async {
5746
// Resolve the package being built from the current BuildStep instead of the
@@ -90,16 +79,16 @@ class FlutterGenBuilder extends Builder {
9079
);
9180

9281
// `Config` still carries a `File pubspecFile` because the lower-level core
93-
// generator APIs remain file-system based. We construct a package-local file
94-
// path here after resolving the correct package root above.
82+
// generator APIs remain file-system based. We construct a package-local
83+
// file path here after resolving the correct package root above.
9584
final pubspecFile = File(join(packageRoot, 'pubspec.yaml'));
9685
final config = loadPubspecConfigFromInputOrNull(
9786
ConfigLoadInput(
9887
pubspecFile: pubspecFile,
9988
pubspecContent: pubspecContent,
100-
// BuilderOptions are now the supported way to pass build.yaml options in
101-
// the build_runner path. This keeps config target-local and workspace
102-
// aware without relying on process cwd.
89+
// BuilderOptions are now the supported way to pass build.yaml options
90+
// in the build_runner path. This keeps config target-local and
91+
// workspace aware without relying on process cwd.
10392
buildOptions: _options.config,
10493
pubspecLockContent: pubspecLockContent,
10594
analysisOptionsContent: analysisOptionsContent,
@@ -143,7 +132,7 @@ class FlutterGenBuilder extends Builder {
143132
writer: (contents, path) {
144133
outputs.add(
145134
FlutterGenManifestOutput(
146-
path: relative(path, from: packageRoot),
135+
path: _packageRelativePath(path, from: packageRoot),
147136
contents: contents,
148137
),
149138
);
@@ -214,14 +203,125 @@ class FlutterGenBuilder extends Builder {
214203
}
215204

216205
Future<String> _packageRoot(BuildStep buildStep) async {
217-
final packageConfig = await _runtimePackageConfig;
218-
final package = packageConfig[buildStep.inputId.package];
219-
if (package == null) {
220-
throw StateError(
221-
'Unable to resolve package root for ${buildStep.inputId.package}.',
206+
final packageName = buildStep.inputId.package;
207+
208+
// Prefer build_runner's package config because it describes the active
209+
// build graph, even when the build script runs in an AOT isolate.
210+
final buildStepPackageConfig = await buildStep.packageConfig;
211+
final buildStepPackage = buildStepPackageConfig[packageName];
212+
final buildStepRoot = _tryFilePath(buildStepPackage?.root);
213+
if (buildStepRoot != null) {
214+
return normalize(buildStepRoot);
215+
}
216+
217+
// Fall back to the runtime isolate config when it still exposes a usable
218+
// `file:` root for the target package.
219+
final runtimePackageConfig = await _loadRuntimePackageConfig();
220+
final runtimePackage = runtimePackageConfig?[packageName];
221+
final runtimeRoot = _tryFilePath(runtimePackage?.root);
222+
if (runtimeRoot != null) {
223+
return normalize(runtimeRoot);
224+
}
225+
226+
// Last resort for AOT/Windows edge cases: discover the package from the
227+
// current working directory and nearby package config files.
228+
final discoveredRoot = await _findPackageRootFromCwd(packageName);
229+
if (discoveredRoot != null) {
230+
return normalize(discoveredRoot);
231+
}
232+
233+
throw StateError(
234+
'Unable to resolve package root for $packageName. '
235+
'buildStep.packageConfig, runtime package config, and current working '
236+
'directory discovery did not provide a usable file: root.',
237+
);
238+
}
239+
240+
Future<PackageConfig?> _loadRuntimePackageConfig() async {
241+
final packageConfigUri = Isolate.packageConfigSync;
242+
if (packageConfigUri == null) {
243+
return null;
244+
}
245+
return loadPackageConfigUri(packageConfigUri);
246+
}
247+
248+
String? _tryFilePath(Uri? uri) {
249+
if (uri == null || uri.scheme != 'file') {
250+
return null;
251+
}
252+
253+
try {
254+
return uri.toFilePath();
255+
} on UnsupportedError {
256+
return null;
257+
}
258+
}
259+
260+
String _packageRelativePath(String path, {required String from}) {
261+
return split(relative(path, from: from)).join('/');
262+
}
263+
264+
Future<String?> _findPackageRootFromCwd(String packageName) async {
265+
var dir = Directory.current.absolute;
266+
267+
while (true) {
268+
final packageConfigRoot = await _findPackageRootFromPackageConfig(
269+
packageName,
270+
File(join(dir.path, '.dart_tool', 'package_config.json')),
222271
);
272+
if (packageConfigRoot != null) {
273+
return packageConfigRoot;
274+
}
275+
276+
final pubspecRoot = _findPackageRootFromPubspec(
277+
packageName,
278+
File(join(dir.path, 'pubspec.yaml')),
279+
);
280+
if (pubspecRoot != null) {
281+
return pubspecRoot;
282+
}
283+
284+
final parent = dir.parent;
285+
if (parent.path == dir.path) {
286+
return null;
287+
}
288+
dir = parent;
223289
}
224-
return normalize(package.root.toFilePath());
290+
}
291+
292+
Future<String?> _findPackageRootFromPackageConfig(
293+
String packageName,
294+
File packageConfigFile,
295+
) async {
296+
if (!packageConfigFile.existsSync()) {
297+
return null;
298+
}
299+
300+
try {
301+
final packageConfig = await loadPackageConfigUri(packageConfigFile.uri);
302+
return _tryFilePath(packageConfig[packageName]?.root);
303+
} on Object {
304+
return null;
305+
}
306+
}
307+
308+
String? _findPackageRootFromPubspec(
309+
String packageName,
310+
File pubspecFile,
311+
) {
312+
if (!pubspecFile.existsSync()) {
313+
return null;
314+
}
315+
316+
try {
317+
final pubspec = loadYaml(pubspecFile.readAsStringSync());
318+
if (pubspec is YamlMap && pubspec['name'] == packageName) {
319+
return pubspecFile.parent.path;
320+
}
321+
} on Object {
322+
return null;
323+
}
324+
return null;
225325
}
226326

227327
/// Reads an optional package-local asset if it exists.
@@ -292,8 +392,8 @@ class FlutterGenPostProcessBuilder extends PostProcessBuilder {
292392
final nextOutputs = manifest.outputs.map((output) => output.path).toSet();
293393

294394
// Explicit stale cleanup is required because these source outputs are not
295-
// regular declared outputs of the original builder. build_runner manages the
296-
// manifest lifecycle, but FlutterGen owns the lifecycle of the final
395+
// regular declared outputs of the original builder. build_runner manages
396+
// the manifest lifecycle, but FlutterGen owns the lifecycle of the final
297397
// materialized files.
298398
for (final output in previousOutputs.difference(nextOutputs)) {
299399
final absolutePath = normalize(join(manifest.packageRoot, output));
@@ -309,8 +409,8 @@ class FlutterGenPostProcessBuilder extends PostProcessBuilder {
309409
// Materialize the exact output set described by the manifest.
310410
//
311411
// These files are intentionally managed outside build_runner's declared
312-
// output model because their paths are configuration-dependent. Writing them
313-
// directly avoids `InvalidOutputException` when the same files already
412+
// output model because their paths are configuration-dependent. Writing
413+
// them directly avoids `InvalidOutputException` when the same files already
314414
// exist, for example after a previous `fluttergen` command run or a stale
315415
// checked-out generated file.
316416
for (final output in manifest.outputs) {
@@ -344,7 +444,11 @@ class FlutterGenPostProcessBuilder extends PostProcessBuilder {
344444
if (paths is! List) {
345445
return <String>{};
346446
}
347-
return paths.whereType<String>().toSet();
447+
return paths.whereType<String>().map(_packageRelativePath).toSet();
448+
}
449+
450+
String _packageRelativePath(String path) {
451+
return split(path).join('/');
348452
}
349453

350454
/// Guards cleanup against deleting files outside the active package.
@@ -356,10 +460,11 @@ class FlutterGenPostProcessBuilder extends PostProcessBuilder {
356460
}
357461
}
358462

359-
/// Self-contained description of the desired FlutterGen outputs for one package.
463+
/// Self-contained description of the desired FlutterGen outputs for one
464+
/// package.
360465
///
361-
/// The manifest is designed to be replayable by the post-process builder with no
362-
/// additional context: package root, package name, and final rendered file
466+
/// The manifest is designed to be replayable by the post-process builder with
467+
/// no additional context: package root, package name, and final rendered file
363468
/// contents are all embedded here.
364469
class FlutterGenManifest {
365470
const FlutterGenManifest({

packages/runner/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ environment:
1111

1212
dependencies:
1313
flutter_gen_core: 5.14.0
14-
build: '>=2.0.0 <5.0.0'
14+
build: ^4.0.0
1515
build_config: ^1.1.2
1616
collection: ^1.17.0
1717
crypto: ^3.0.0

packages/runner/test/workspace_build_test.dart

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ void main() {
125125
'lib/alt_gen/fonts.gen.dart',
126126
]),
127127
);
128-
});
128+
},
129+
timeout: const Timeout(Duration(minutes: 5)),
130+
);
129131

130132
test('applies package build.yaml options in workspace mode', () async {
131133
final workspaceDir = await _createWorkspaceFixture();
@@ -184,7 +186,9 @@ targets:
184186
File(p.join(appDir.path, 'lib', 'gen', 'assets.gen.dart')).existsSync(),
185187
isFalse,
186188
);
187-
});
189+
},
190+
timeout: const Timeout(Duration(minutes: 5)),
191+
);
188192

189193
test('overwrites existing generated files in workspace mode', () async {
190194
final workspaceDir = await _createWorkspaceFixture();
@@ -224,7 +228,9 @@ targets:
224228
generatedFile.readAsStringSync(),
225229
isNot(contains('// stale contents')),
226230
);
227-
});
231+
},
232+
timeout: const Timeout(Duration(minutes: 5)),
233+
);
228234
}
229235

230236
Future<Directory> _createWorkspaceFixture() async {

0 commit comments

Comments
 (0)