From 2acca4cf4c58a556fe04e9dfb3a90d9f2c2d0311 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 20 Sep 2023 15:08:23 +0000 Subject: [PATCH 1/2] Deprecate the unnamed AnalyzerResolvers constructor, replace it with a static instance getter and custom factory constructor --- build/CHANGELOG.md | 2 + build/pubspec.yaml | 4 +- build/test/builder/build_step_impl_test.dart | 20 ++++--- build_resolvers/CHANGELOG.md | 8 +++ .../lib/src/build_asset_uri_resolver.dart | 14 +++-- build_resolvers/lib/src/resolver.dart | 60 +++++++++++++++---- build_resolvers/pubspec.yaml | 2 +- build_runner_core/CHANGELOG.md | 4 +- .../lib/src/generate/options.dart | 2 +- build_runner_core/pubspec.yaml | 4 +- build_test/CHANGELOG.md | 2 + build_test/lib/src/resolve_source.dart | 6 +- build_test/lib/src/test_builder.dart | 5 +- build_test/pubspec.yaml | 2 +- 14 files changed, 97 insertions(+), 38 deletions(-) diff --git a/build/CHANGELOG.md b/build/CHANGELOG.md index 9f9b3e207..fc78c2d1b 100644 --- a/build/CHANGELOG.md +++ b/build/CHANGELOG.md @@ -1,3 +1,5 @@ +## 2.4.2-wip + ## 2.4.1 - Allow the latest analyzer (6.x.x). diff --git a/build/pubspec.yaml b/build/pubspec.yaml index c60894d36..1d3b47b95 100644 --- a/build/pubspec.yaml +++ b/build/pubspec.yaml @@ -1,5 +1,5 @@ name: build -version: 2.4.1 +version: 2.4.2-wip description: A package for authoring build_runner compatible code generators. repository: https://github.com/dart-lang/build/tree/master/build @@ -18,7 +18,7 @@ dependencies: path: ^1.8.0 dev_dependencies: - build_resolvers: ^2.0.0 + build_resolvers: ^2.3.3 build_test: ^2.0.0 dart_flutter_team_lints: ^1.0.0 test: ^1.16.0 diff --git a/build/test/builder/build_step_impl_test.dart b/build/test/builder/build_step_impl_test.dart index 760f17e95..fcdadac9e 100644 --- a/build/test/builder/build_step_impl_test.dart +++ b/build/test/builder/build_step_impl_test.dart @@ -35,7 +35,7 @@ void main() { primary = makeAssetId(); outputs = List.generate(5, (index) => makeAssetId()); buildStep = BuildStepImpl(primary, outputs, reader, writer, - AnalyzerResolvers(), resourceManager, _unsupported); + AnalyzerResolvers.custom(), resourceManager, _unsupported); }); test('doesnt allow non-expected outputs', () { @@ -93,7 +93,7 @@ void main() { [outputId], reader, writer, - AnalyzerResolvers(), + AnalyzerResolvers.custom(), resourceManager, _unsupported, ); @@ -121,7 +121,7 @@ void main() { var primary = makeAssetId('a|web/a.dart'); var buildStep = BuildStepImpl(primary, [], reader, writer, - AnalyzerResolvers(), resourceManager, _unsupported); + AnalyzerResolvers.custom(), resourceManager, _unsupported); var resolver = buildStep.resolver; var aLib = await resolver.libraryFor(primary); @@ -150,8 +150,14 @@ void main() { assetWriter = SlowAssetWriter(); outputId = makeAssetId('a|test.txt'); outputContent = '$outputId'; - buildStep = BuildStepImpl(primary, [outputId], StubAssetReader(), - assetWriter, AnalyzerResolvers(), resourceManager, _unsupported); + buildStep = BuildStepImpl( + primary, + [outputId], + StubAssetReader(), + assetWriter, + AnalyzerResolvers.custom(), + resourceManager, + _unsupported); }); test('Completes only after writes finish', () async { @@ -199,7 +205,7 @@ void main() { primary = makeAssetId(); output = makeAssetId(); buildStep = BuildStepImpl(primary, [output], reader, writer, - AnalyzerResolvers(), resourceManager, _unsupported, + AnalyzerResolvers.custom(), resourceManager, _unsupported, stageTracker: NoOpStageTracker.instance); }); @@ -214,7 +220,7 @@ void main() { var writer = StubAssetWriter(); var unused = {}; var buildStep = BuildStepImpl(makeAssetId(), [], reader, writer, - AnalyzerResolvers(), resourceManager, _unsupported, + AnalyzerResolvers.custom(), resourceManager, _unsupported, reportUnusedAssets: unused.addAll); var reported = [ makeAssetId(), diff --git a/build_resolvers/CHANGELOG.md b/build_resolvers/CHANGELOG.md index 8e02102b1..4e07cf07c 100644 --- a/build_resolvers/CHANGELOG.md +++ b/build_resolvers/CHANGELOG.md @@ -1,3 +1,11 @@ +## 2.3.3-wip + +- Deprecate the unnamed `AnalyzerResolvers` constructor, an replace it with the + `AnalyzerResolvers.sharedInstance` getter and `AnalyzerResolvers.custom` + constructor. These new apis enforce a 1:1 relationship between the + `BuildAssetUriResolver` instances and `AnalyzerResolvers` instances, which is + necessary to ensure correct builds in the presence of multiple resolvers. + ## 2.3.2 - Skip file delete for SDK summary and deps file. This will only impact behavior diff --git a/build_resolvers/lib/src/build_asset_uri_resolver.dart b/build_resolvers/lib/src/build_asset_uri_resolver.dart index c25a71aba..4f6e8e402 100644 --- a/build_resolvers/lib/src/build_asset_uri_resolver.dart +++ b/build_resolvers/lib/src/build_asset_uri_resolver.dart @@ -54,10 +54,16 @@ class BuildAssetUriResolver extends UriResolver { /// input, subsequent calls to a resolver, or a transitive import thereof. final _buildStepTransitivelyResolvedAssets = >{}; - /// Use the [instance] getter to get an instance. - BuildAssetUriResolver._(); + /// Use the [sharedInstance] getter to get the shared instance, which is what + /// real builds should do. There should always be a 1:1 relationship between + /// [BuildAssetUriResolver]s and `AnalyzerResolvers`. + BuildAssetUriResolver(); - static final BuildAssetUriResolver instance = BuildAssetUriResolver._(); + /// The instance used by the shared `AnalyzerResolvers` instance, which is + /// what should be used by normal builds. + /// + /// This is not used within testing contexts or similar custom contexts. + static final BuildAssetUriResolver sharedInstance = BuildAssetUriResolver(); /// Updates [resourceProvider] and the analysis driver given by /// `withDriverResource` with updated versions of [entryPoints]. @@ -261,7 +267,7 @@ Set _parseDependencies(String content, AssetId from) => HashSet.of( /// directives, and cache the results if they weren't already cached. Future?> dependenciesOf( AssetId id, BuildStep buildStep) async => - (await BuildAssetUriResolver.instance + (await BuildAssetUriResolver.sharedInstance ._updateCachedAssetState(id, buildStep)) ?.dependencies; diff --git a/build_resolvers/lib/src/resolver.dart b/build_resolvers/lib/src/resolver.dart index 0fc532946..e24dcc3f1 100644 --- a/build_resolvers/lib/src/resolver.dart +++ b/build_resolvers/lib/src/resolver.dart @@ -381,7 +381,8 @@ class AnalyzerResolvers implements Resolvers { // Lazy, all access must be preceded by a call to `_ensureInitialized`. late final AnalyzerResolver _resolver; - BuildAssetUriResolver? _uriResolver; + + final BuildAssetUriResolver _uriResolver; /// Nullable, should not be accessed outside of [_ensureInitialized]. Future>? _initialized; @@ -402,30 +403,65 @@ class AnalyzerResolvers implements Resolvers { /// **NOTE**: The [_packageConfig] is not used for path resolution, it is /// primarily used to get the language versions. Any other data (including /// extra data), may be passed to the analyzer on an as needed basis. - AnalyzerResolvers( - [AnalysisOptions? analysisOptions, - Future Function()? sdkSummaryGenerator, - this._packageConfig]) - : _analysisOptions = analysisOptions ?? + factory AnalyzerResolvers.custom({ + AnalysisOptions? analysisOptions, + Future Function()? sdkSummaryGenerator, + PackageConfig? packageConfig, + }) => + AnalyzerResolvers._( + analysisOptions: analysisOptions, + sdkSummaryGenerator: sdkSummaryGenerator, + packageConfig: packageConfig, + // Custom resolvers get their own asset uri resolver, as there should + // always be a 1:1 relationship between them. + uriResolver: BuildAssetUriResolver()); + + /// See [AnalyzerResolvers.custom] for docs. + @Deprecated('Use either the AnalyzerResolvers.custom constructor or the ' + 'AnalyzerResolvers.sharedInstance static getter to get an instance.') + factory AnalyzerResolvers([ + AnalysisOptions? analysisOptions, + Future Function()? sdkSummaryGenerator, + PackageConfig? packageConfig, + ]) => + AnalyzerResolvers._( + analysisOptions: analysisOptions, + sdkSummaryGenerator: sdkSummaryGenerator, + packageConfig: packageConfig, + // For backwards compatibility we use the shared instance here. + uriResolver: BuildAssetUriResolver.sharedInstance); + + /// See [AnalyzerResolvers.custom] for docs. + AnalyzerResolvers._({ + AnalysisOptions? analysisOptions, + PackageConfig? packageConfig, + Future Function()? sdkSummaryGenerator, + required BuildAssetUriResolver uriResolver, + }) : _analysisOptions = analysisOptions ?? (AnalysisOptionsImpl() ..contextFeatures = _featureSet(enableExperiments: enabledExperiments)), + _packageConfig = packageConfig, _sdkSummaryGenerator = - sdkSummaryGenerator ?? defaultSdkSummaryGenerator; + sdkSummaryGenerator ?? defaultSdkSummaryGenerator, + _uriResolver = uriResolver; + + /// The instance that most real build systems should use. + static final AnalyzerResolvers sharedInstance = + AnalyzerResolvers._(uriResolver: BuildAssetUriResolver.sharedInstance); /// Create a Resolvers backed by an `AnalysisContext` using options /// [_analysisOptions]. Future _ensureInitialized() { return Result.release(_initialized ??= Result.capture(() async { _warnOnLanguageVersionMismatch(); - final uriResolver = _uriResolver = BuildAssetUriResolver.instance; final loadedConfig = _packageConfig ??= await loadPackageConfigUri((await Isolate.packageConfig)!); - var driver = await analysisDriver(uriResolver, _analysisOptions, + var driver = await analysisDriver(_uriResolver, _analysisOptions, await _sdkSummaryGenerator(), loadedConfig); - _resolver = - AnalyzerResolver(driver, _driverPool, _readAndWritePool, uriResolver); + _resolver = AnalyzerResolver( + driver, _driverPool, _readAndWritePool, _uriResolver); }())); } @@ -439,7 +475,7 @@ class AnalyzerResolvers implements Resolvers { /// Must be called between each build. @override void reset() { - _uriResolver?.reset(); + _uriResolver.reset(); } } diff --git a/build_resolvers/pubspec.yaml b/build_resolvers/pubspec.yaml index e3a280112..84b08d97a 100644 --- a/build_resolvers/pubspec.yaml +++ b/build_resolvers/pubspec.yaml @@ -1,5 +1,5 @@ name: build_resolvers -version: 2.3.2 +version: 2.3.3-wip description: Resolve Dart code in a Builder repository: https://github.com/dart-lang/build/tree/master/build_resolvers diff --git a/build_runner_core/CHANGELOG.md b/build_runner_core/CHANGELOG.md index fae430c04..341842b4e 100644 --- a/build_runner_core/CHANGELOG.md +++ b/build_runner_core/CHANGELOG.md @@ -1,7 +1,9 @@ -## 7.2.11-dev +## 7.2.11-wip - Add better error messages for incomplete package configs when creating a package graph. +- Update to build_resolvers 2.3.3 and use the shared analyzer resolvers + instance. ## 7.2.10 diff --git a/build_runner_core/lib/src/generate/options.dart b/build_runner_core/lib/src/generate/options.dart index b49c8ef52..71bfe5a64 100644 --- a/build_runner_core/lib/src/generate/options.dart +++ b/build_runner_core/lib/src/generate/options.dart @@ -208,7 +208,7 @@ feature, you may need to run `dart run build_runner clean` and then rebuild. } trackPerformance = true; } - resolvers ??= AnalyzerResolvers(); + resolvers ??= AnalyzerResolvers.sharedInstance; return BuildOptions._( debounceDelay: debounceDelay, diff --git a/build_runner_core/pubspec.yaml b/build_runner_core/pubspec.yaml index 0ee3e5d42..742e340e4 100644 --- a/build_runner_core/pubspec.yaml +++ b/build_runner_core/pubspec.yaml @@ -1,5 +1,5 @@ name: build_runner_core -version: 7.2.11-dev +version: 7.2.11-wip description: Core tools to organize the structure of a build and run Builders. repository: https://github.com/dart-lang/build/tree/master/build_runner_core @@ -15,7 +15,7 @@ dependencies: async: ^2.5.0 build: ^2.4.0 build_config: ^1.0.0 - build_resolvers: ^2.0.0 + build_resolvers: ^2.3.3 collection: ^1.15.0 convert: ^3.0.0 crypto: ^3.0.0 diff --git a/build_test/CHANGELOG.md b/build_test/CHANGELOG.md index 2ce9d9074..1baa5646b 100644 --- a/build_test/CHANGELOG.md +++ b/build_test/CHANGELOG.md @@ -1,6 +1,8 @@ ## 2.2.1-dev - Avoid passing a nullable value to `Future.value`. +- Update to build_resolvers 2.3.3, which resolves some race conditions when + using multiple resolvers instances. ## 2.2.0 diff --git a/build_test/lib/src/resolve_source.dart b/build_test/lib/src/resolve_source.dart index dbce4f301..a2dcab8a2 100644 --- a/build_test/lib/src/resolve_source.dart +++ b/build_test/lib/src/resolve_source.dart @@ -15,8 +15,6 @@ import 'in_memory_writer.dart'; import 'multi_asset_reader.dart'; import 'package_reader.dart'; -final defaultResolvers = AnalyzerResolvers(); - /// Marker constant that may be used in combination with [resolveSources]. /// /// Use of this string means instead of using the contents of the string as the @@ -199,8 +197,8 @@ Future _resolveAssets( // Use the default resolver if no experiments are enabled. This is much // faster. resolvers ??= packageConfig == null && enabledExperiments.isEmpty - ? defaultResolvers - : AnalyzerResolvers(null, null, resolvedConfig); + ? AnalyzerResolvers.sharedInstance + : AnalyzerResolvers.custom(packageConfig: resolvedConfig); // We don't care about the results of this build, but we also can't await // it because that would block on the `tearDown` of the `resolveBuilder`. diff --git a/build_test/lib/src/test_builder.dart b/build_test/lib/src/test_builder.dart index b4363c21e..916f48f6b 100644 --- a/build_test/lib/src/test_builder.dart +++ b/build_test/lib/src/test_builder.dart @@ -14,7 +14,6 @@ import 'assets.dart'; import 'in_memory_reader.dart'; import 'in_memory_writer.dart'; import 'multi_asset_reader.dart'; -import 'resolve_source.dart'; import 'written_asset_reader.dart'; AssetId _passThrough(AssetId id) => id; @@ -171,8 +170,8 @@ Future testBuilder( var logger = Logger('testBuilder'); var logSubscription = logger.onRecord.listen(onLog); var resolvers = packageConfig == null && enabledExperiments.isEmpty - ? defaultResolvers - : AnalyzerResolvers(null, null, packageConfig); + ? AnalyzerResolvers.sharedInstance + : AnalyzerResolvers.custom(packageConfig: packageConfig); for (var input in inputIds) { // create another writer spy and reader for each input. This prevents writes diff --git a/build_test/pubspec.yaml b/build_test/pubspec.yaml index 29c3cb446..b06b9cd30 100644 --- a/build_test/pubspec.yaml +++ b/build_test/pubspec.yaml @@ -10,7 +10,7 @@ dependencies: async: ^2.5.0 build: ^2.1.0 build_config: ^1.0.0 - build_resolvers: ^2.0.0 + build_resolvers: ^2.3.3 crypto: ^3.0.0 glob: ^2.0.0 html: ^0.15.0 From 48172a786ca1a81d5505a862e9c8e042ce7cd893 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 20 Sep 2023 16:21:15 +0000 Subject: [PATCH 2/2] set version to 2.4.0 --- build/pubspec.yaml | 2 +- build_resolvers/CHANGELOG.md | 2 +- build_resolvers/pubspec.yaml | 2 +- build_runner_core/CHANGELOG.md | 2 +- build_runner_core/pubspec.yaml | 2 +- build_test/CHANGELOG.md | 2 +- build_test/pubspec.yaml | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/build/pubspec.yaml b/build/pubspec.yaml index 1d3b47b95..8879e7c9a 100644 --- a/build/pubspec.yaml +++ b/build/pubspec.yaml @@ -18,7 +18,7 @@ dependencies: path: ^1.8.0 dev_dependencies: - build_resolvers: ^2.3.3 + build_resolvers: ^2.4.0 build_test: ^2.0.0 dart_flutter_team_lints: ^1.0.0 test: ^1.16.0 diff --git a/build_resolvers/CHANGELOG.md b/build_resolvers/CHANGELOG.md index 4e07cf07c..5dce1de75 100644 --- a/build_resolvers/CHANGELOG.md +++ b/build_resolvers/CHANGELOG.md @@ -1,4 +1,4 @@ -## 2.3.3-wip +## 2.4.0-wip - Deprecate the unnamed `AnalyzerResolvers` constructor, an replace it with the `AnalyzerResolvers.sharedInstance` getter and `AnalyzerResolvers.custom` diff --git a/build_resolvers/pubspec.yaml b/build_resolvers/pubspec.yaml index 84b08d97a..dc5c06166 100644 --- a/build_resolvers/pubspec.yaml +++ b/build_resolvers/pubspec.yaml @@ -1,5 +1,5 @@ name: build_resolvers -version: 2.3.3-wip +version: 2.4.0-wip description: Resolve Dart code in a Builder repository: https://github.com/dart-lang/build/tree/master/build_resolvers diff --git a/build_runner_core/CHANGELOG.md b/build_runner_core/CHANGELOG.md index 341842b4e..2ae514aea 100644 --- a/build_runner_core/CHANGELOG.md +++ b/build_runner_core/CHANGELOG.md @@ -2,7 +2,7 @@ - Add better error messages for incomplete package configs when creating a package graph. -- Update to build_resolvers 2.3.3 and use the shared analyzer resolvers +- Update to build_resolvers 2.4.0 and use the shared analyzer resolvers instance. ## 7.2.10 diff --git a/build_runner_core/pubspec.yaml b/build_runner_core/pubspec.yaml index 742e340e4..3fa8c7044 100644 --- a/build_runner_core/pubspec.yaml +++ b/build_runner_core/pubspec.yaml @@ -15,7 +15,7 @@ dependencies: async: ^2.5.0 build: ^2.4.0 build_config: ^1.0.0 - build_resolvers: ^2.3.3 + build_resolvers: ^2.4.0 collection: ^1.15.0 convert: ^3.0.0 crypto: ^3.0.0 diff --git a/build_test/CHANGELOG.md b/build_test/CHANGELOG.md index 1baa5646b..4c1d10573 100644 --- a/build_test/CHANGELOG.md +++ b/build_test/CHANGELOG.md @@ -1,7 +1,7 @@ ## 2.2.1-dev - Avoid passing a nullable value to `Future.value`. -- Update to build_resolvers 2.3.3, which resolves some race conditions when +- Update to build_resolvers 2.4.0, which resolves some race conditions when using multiple resolvers instances. ## 2.2.0 diff --git a/build_test/pubspec.yaml b/build_test/pubspec.yaml index b06b9cd30..fe624c1ae 100644 --- a/build_test/pubspec.yaml +++ b/build_test/pubspec.yaml @@ -10,7 +10,7 @@ dependencies: async: ^2.5.0 build: ^2.1.0 build_config: ^1.0.0 - build_resolvers: ^2.3.3 + build_resolvers: ^2.4.0 crypto: ^3.0.0 glob: ^2.0.0 html: ^0.15.0