Check for workspaces consistency after analysis contexts creation.
Bug: https://github.com/dart-lang/linter/issues/2694
Change-Id: I13cae115e48ae794e0751b6099f864ca9d202cdc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203821
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/context_manager.dart b/pkg/analysis_server/lib/src/context_manager.dart
index 047e8b1b..721f37c 100644
--- a/pkg/analysis_server/lib/src/context_manager.dart
+++ b/pkg/analysis_server/lib/src/context_manager.dart
@@ -175,6 +175,10 @@
final Map<Folder, AnalysisDriver> driverMap =
HashMap<Folder, AnalysisDriver>();
+ /// The timer that is started after creating analysis contexts, to check
+ /// for in-between changes to configuration files.
+ Timer? _collectionConsistencyCheckTimer;
+
/// Stream subscription we are using to watch each analysis root directory for
/// changes.
final Map<Folder, StreamSubscription<WatchEvent>> changeSubscriptions =
@@ -461,6 +465,7 @@
}
callbacks.afterContextsCreated();
+ _scheduleCollectionConsistencyCheck(collection);
}
/// Clean up and destroy the context associated with the given folder.
@@ -482,6 +487,7 @@
void _destroyAnalysisContexts() {
var collection = _collection;
if (collection != null) {
+ _collectionConsistencyCheckTimer?.cancel();
for (var analysisContext in collection.contexts) {
_destroyAnalysisContext(analysisContext);
}
@@ -632,6 +638,28 @@
existingExcludedSet.containsAll(excludedPaths);
}
+ /// We create analysis contexts, and then start watching the file system
+ /// for modifications to Dart files, and to configuration files, e.g.
+ /// `pubspec.yaml` file Pub workspaces.
+ ///
+ /// So, it is possible that one of these files will be changed between the
+ /// moment when we read it, and the moment when we started watching for
+ /// changes. Using `package:watcher` before creating analysis contexts
+ /// was still not reliable enough.
+ ///
+ /// To work around this we check after a short timeout, and hope that
+ /// any subsequent changes will be noticed by `package:watcher`.
+ void _scheduleCollectionConsistencyCheck(
+ AnalysisContextCollectionImpl collection,
+ ) {
+ _collectionConsistencyCheckTimer = Timer(Duration(seconds: 1), () {
+ _collectionConsistencyCheckTimer = null;
+ if (!collection.areWorkspacesConsistent) {
+ _createAnalysisContexts();
+ }
+ });
+ }
+
/// Starts watching for the `bazel-bin` and `blaze-bin` symlinks.
///
/// This is important since these symlinks might not be present when the
diff --git a/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart b/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart
index 2335ded..103c1a6 100644
--- a/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart
+++ b/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart
@@ -79,6 +79,20 @@
}
}
+ /// Return `true` if the read state of configuration files is consistent
+ /// with their current state on the file system. We use this as a work
+ /// around an issue with watching for file system changes.
+ bool get areWorkspacesConsistent {
+ for (var analysisContext in contexts) {
+ var contextRoot = analysisContext.contextRoot;
+ var workspace = contextRoot.workspace;
+ if (!workspace.isConsistentWithFileSystem) {
+ return false;
+ }
+ }
+ return true;
+ }
+
@override
DriverBasedAnalysisContext contextFor(String path) {
_throwIfNotAbsoluteNormalizedPath(path);
diff --git a/pkg/analyzer/lib/src/workspace/package_build.dart b/pkg/analyzer/lib/src/workspace/package_build.dart
index 5d2ee1e..7e0c179 100644
--- a/pkg/analyzer/lib/src/workspace/package_build.dart
+++ b/pkg/analyzer/lib/src/workspace/package_build.dart
@@ -145,6 +145,9 @@
/// package:build does it.
static const String _pubspecName = 'pubspec.yaml';
+ /// The associated pubspec file.
+ final File _pubspecFile;
+
/// The content of the `pubspec.yaml` file.
/// We read it once, so that all usages return consistent results.
final String? _pubspecContent;
@@ -185,11 +188,17 @@
this.generatedRootPath,
this.generatedThisPath,
File pubspecFile,
- ) : _pubspecContent = _fileContentOrNull(pubspecFile) {
+ ) : _pubspecFile = pubspecFile,
+ _pubspecContent = _fileContentOrNull(pubspecFile) {
_theOnlyPackage = PackageBuildWorkspacePackage(root, this);
}
@override
+ bool get isConsistentWithFileSystem {
+ return _fileContentOrNull(_pubspecFile) == _pubspecContent;
+ }
+
+ @override
UriResolver get packageUriResolver => PackageBuildPackageUriResolver(
this, PackageMapUriResolver(provider, packageMap));
diff --git a/pkg/analyzer/lib/src/workspace/pub.dart b/pkg/analyzer/lib/src/workspace/pub.dart
index ef69e23..e5059d7 100644
--- a/pkg/analyzer/lib/src/workspace/pub.dart
+++ b/pkg/analyzer/lib/src/workspace/pub.dart
@@ -21,6 +21,9 @@
/// Each Pub workspace is itself one package.
late final PubWorkspacePackage _theOnlyPackage;
+ /// The associated pubspec file.
+ final File _pubspecFile;
+
/// The content of the `pubspec.yaml` file.
/// We read it once, so that all usages return consistent results.
final String? _pubspecContent;
@@ -30,11 +33,17 @@
Map<String, List<Folder>> packageMap,
String root,
File pubspecFile,
- ) : _pubspecContent = _fileContentOrNull(pubspecFile),
+ ) : _pubspecFile = pubspecFile,
+ _pubspecContent = _fileContentOrNull(pubspecFile),
super(provider, packageMap, root) {
_theOnlyPackage = PubWorkspacePackage(root, this);
}
+ @override
+ bool get isConsistentWithFileSystem {
+ return _fileContentOrNull(_pubspecFile) == _pubspecContent;
+ }
+
@internal
@override
void contributeToResolutionSalt(ApiSignature buffer) {
diff --git a/pkg/analyzer/lib/src/workspace/workspace.dart b/pkg/analyzer/lib/src/workspace/workspace.dart
index c4cb125..34ab9c1 100644
--- a/pkg/analyzer/lib/src/workspace/workspace.dart
+++ b/pkg/analyzer/lib/src/workspace/workspace.dart
@@ -17,6 +17,11 @@
/// Return true iff this [Workspace] is a [BazelWorkspace].
bool get isBazel => false;
+ /// Return `true` if the read state of configuration files is consistent
+ /// with their current state on the file system.
+ @internal
+ bool get isConsistentWithFileSystem => true;
+
/// The [UriResolver] that can resolve `package` URIs.
UriResolver get packageUriResolver;
diff --git a/pkg/analyzer/test/src/workspace/package_build_test.dart b/pkg/analyzer/test/src/workspace/package_build_test.dart
index 8ce842f..b7a9650 100644
--- a/pkg/analyzer/test/src/workspace/package_build_test.dart
+++ b/pkg/analyzer/test/src/workspace/package_build_test.dart
@@ -640,6 +640,17 @@
workspace.findFile(convertPath('/workspace/web/file.dart')), webFile);
}
+ void test_isConsistentWithFileSystem() {
+ newFolder('/workspace/.dart_tool/build/generated/project/bin');
+ newPubspecYamlFile('/workspace', 'name: project');
+ PackageBuildWorkspace workspace =
+ _createWorkspace('/workspace', ['project']);
+ expect(workspace.isConsistentWithFileSystem, isTrue);
+
+ newPubspecYamlFile('/workspace', 'name: my2');
+ expect(workspace.isConsistentWithFileSystem, isFalse);
+ }
+
PackageBuildWorkspace _createWorkspace(
String root, List<String> packageNames) {
return PackageBuildWorkspace.find(
diff --git a/pkg/analyzer/test/src/workspace/pub_test.dart b/pkg/analyzer/test/src/workspace/pub_test.dart
index f69f1c2..5f68d58 100644
--- a/pkg/analyzer/test/src/workspace/pub_test.dart
+++ b/pkg/analyzer/test/src/workspace/pub_test.dart
@@ -111,4 +111,14 @@
resourceProvider, {}, convertPath('/workspace/lib/lib1.dart'));
expect(workspace, isNull);
}
+
+ void test_isConsistentWithFileSystem() {
+ newPubspecYamlFile('/workspace', 'name: my');
+ var workspace =
+ PubWorkspace.find(resourceProvider, {}, convertPath('/workspace'))!;
+ expect(workspace.isConsistentWithFileSystem, isTrue);
+
+ newPubspecYamlFile('/workspace', 'name: my2');
+ expect(workspace.isConsistentWithFileSystem, isFalse);
+ }
}
