React when Bazel/Blaze prefix changes
Previously if no Bazel/Blaze symlinks were present, we defaulted to the
`bazel-*` ones and never checked if that assumption was accurate. This
change starts watching the symlinks and recreates the analysis contexts
if a symlink gets added. This way we'll run the `BazelWorkspace` code
again and use the correct paths.
This is quite important since `bazel/blaze clean` does remove the
symlinks so if the analysis server starts at such a point, we would
never find any generated files if the workspaced turned out to be a
Blaze one.
Bug: http://b/175087705
Change-Id: If7bb21c7d69a3092832c18004691d56949e5af54
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197540
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Michal Terepeta <michalt@google.com>
diff --git a/pkg/analysis_server/lib/src/context_manager.dart b/pkg/analysis_server/lib/src/context_manager.dart
index 7b6cb5f..7292d9a 100644
--- a/pkg/analysis_server/lib/src/context_manager.dart
+++ b/pkg/analysis_server/lib/src/context_manager.dart
@@ -460,7 +460,9 @@
for (var path in watched.paths) {
bazelWatcherService!.stopWatching(watched.workspace, path);
}
+ _stopWatchingBazelBinPaths(watched);
}
+ bazelSearchSubscriptions.remove(rootFolder)?.cancel();
driverMap.remove(rootFolder);
}
@@ -474,6 +476,11 @@
}
}
+ List<String> _getPossibelBazelBinPaths(_BazelWatchedFiles watched) => [
+ pathContext.join(watched.workspace, 'bazel-bin'),
+ pathContext.join(watched.workspace, 'blaze-bin'),
+ ];
+
/// Establishes watch(es) for the Bazel generated files provided in
/// [notification].
///
@@ -493,10 +500,23 @@
}
/// Notifies the drivers that a generated Bazel file has changed.
- void _handleBazelWatchEvents(List<WatchEvent> events) {
+ void _handleBazelWatchEvents(List<WatchEvent> allEvents) {
+ // First check if we have any changes to the bazel-*/blaze-* paths. If
+ // we do, we'll simply recreate all contexts to make sure that we follow the
+ // correct paths.
+ var bazelSymlinkPaths = bazelWatchedPathsPerFolder.values
+ .expand((watched) => _getPossibelBazelBinPaths(watched))
+ .toSet();
+ if (allEvents.any((event) => bazelSymlinkPaths.contains(event.path))) {
+ refresh();
+ return;
+ }
+
+ var fileEvents =
+ allEvents.where((event) => !bazelSymlinkPaths.contains(event.path));
for (var driver in driverMap.values) {
var needsUriReset = false;
- for (var event in events) {
+ for (var event in fileEvents) {
if (event.type == ChangeType.ADD) {
driver.addFile(event.path);
needsUriReset = true;
@@ -599,6 +619,28 @@
existingExcludedSet.containsAll(excludedPaths);
}
+ /// Starts watching for the `bazel-bin` and `blaze-bin` symlinks.
+ ///
+ /// This is important since these symlinks might not be present when the
+ /// server starts up, in which case `BazelWorkspace` assumes by default the
+ /// Bazel ones. So we want to detect if the symlinks get created to reset
+ /// everything and repeat the search for the folders.
+ void _startWatchingBazelBinPaths(_BazelWatchedFiles watched) {
+ var watcherService = bazelWatcherService;
+ if (watcherService == null) return;
+ var paths = _getPossibelBazelBinPaths(watched);
+ watcherService.startWatching(
+ watched.workspace, BazelSearchInfo(paths[0], paths));
+ }
+
+ /// Stops watching for the `bazel-bin` and `blaze-bin` symlinks.
+ void _stopWatchingBazelBinPaths(_BazelWatchedFiles watched) {
+ var watcherService = bazelWatcherService;
+ if (watcherService == null) return;
+ var paths = _getPossibelBazelBinPaths(watched);
+ watcherService.stopWatching(watched.workspace, paths[0]);
+ }
+
/// Listens to files generated by Bazel that were found or searched for.
///
/// This is handled specially because the files are outside the package
@@ -607,13 +649,19 @@
/// Does nothing if the [driver] is not in a Bazel workspace.
void _watchBazelFilesIfNeeded(Folder folder, AnalysisDriver analysisDriver) {
if (!experimentalEnableBazelWatching) return;
+ var watcherService = bazelWatcherService;
+ if (watcherService == null) return;
+
var workspace = analysisDriver.analysisContext?.contextRoot.workspace;
if (workspace is BazelWorkspace &&
!bazelSearchSubscriptions.containsKey(folder)) {
- var searchSubscription = workspace.bazelCandidateFiles.listen(
+ bazelSearchSubscriptions[folder] = workspace.bazelCandidateFiles.listen(
(notification) =>
_handleBazelSearchInfo(folder, workspace.root, notification));
- bazelSearchSubscriptions[folder] = searchSubscription;
+
+ var watched = _BazelWatchedFiles(workspace.root);
+ bazelWatchedPathsPerFolder[folder] = watched;
+ _startWatchingBazelBinPaths(watched);
}
}
}
diff --git a/pkg/analysis_server/test/integration/server/bazel_changes_test.dart b/pkg/analysis_server/test/integration/server/bazel_changes_test.dart
index f904df3..da935af 100644
--- a/pkg/analysis_server/test/integration/server/bazel_changes_test.dart
+++ b/pkg/analysis_server/test/integration/server/bazel_changes_test.dart
@@ -35,9 +35,9 @@
late String bazelRoot;
late String tmpPath;
late String workspacePath;
- late String bazelOutPath;
- late String bazelBinPath;
- late String bazelGenfilesPath;
+ late String bazelOrBlazeOutPath;
+ late String bazelOrBlazeBinPath;
+ late String bazelOrBlazeGenfilesPath;
late Directory oldSourceDirectory;
String inTmpDir(String relative) =>
@@ -61,21 +61,19 @@
sourceDirectory = Directory(inWorkspace('third_party/dart/project'));
sourceDirectory.createSync(recursive: true);
- bazelRoot = inTmpDir('bazel_root');
+ bazelRoot = inTmpDir('bazel_or_blaze_root');
Directory(bazelRoot).createSync(recursive: true);
- bazelOutPath = '$bazelRoot/execroot/bazel_workspace/bazel-out';
- bazelBinPath = '$bazelRoot/execroot/bazel_workspace/bazel-out/bin';
- bazelGenfilesPath =
- '$bazelRoot/execroot/bazel_workspace/bazel-out/genfiles';
+ bazelOrBlazeOutPath =
+ '$bazelRoot/execroot/bazel_or_blaze_workspace/bazel_or_blaze-out';
+ bazelOrBlazeBinPath =
+ '$bazelRoot/execroot/bazel_or_blaze_workspace/bazel_or_blaze-out/bin';
+ bazelOrBlazeGenfilesPath =
+ '$bazelRoot/execroot/bazel_or_blaze_workspace/bazel_or_blaze-out/genfiles';
- Directory(inTmpDir(bazelOutPath)).createSync(recursive: true);
- Directory(inTmpDir(bazelBinPath)).createSync(recursive: true);
- Directory(inTmpDir(bazelGenfilesPath)).createSync(recursive: true);
-
- Link(inWorkspace('bazel-out')).createSync(bazelOutPath);
- Link(inWorkspace('bazel-bin')).createSync(bazelBinPath);
- Link(inWorkspace('bazel-genfiles')).createSync(bazelGenfilesPath);
+ Directory(inTmpDir(bazelOrBlazeOutPath)).createSync(recursive: true);
+ Directory(inTmpDir(bazelOrBlazeBinPath)).createSync(recursive: true);
+ Directory(inTmpDir(bazelOrBlazeGenfilesPath)).createSync(recursive: true);
commandLogPath = inTmpDir('$bazelRoot/command.log');
}
@@ -91,6 +89,17 @@
// not run from a snapshot.
@TestTimeout(Timeout.factor(2))
Future<void> test_bazelChanges() async {
+ await testChangesImpl('bazel');
+ }
+
+ // Add a bit more time -- the isolate take a while to start when the test is
+ // not run from a snapshot.
+ @TestTimeout(Timeout.factor(2))
+ Future<void> test_blazeChanges() async {
+ await testChangesImpl('blaze');
+ }
+
+ Future<void> testChangesImpl(String prefix) async {
var testFile = inWorkspace('${sourceDirectory.path}/lib/test.dart');
var errors = <AnalysisError>[];
@@ -126,8 +135,9 @@
await resetCompleterAndErrors();
var generatedFilePath = inWorkspace(
- '$bazelGenfilesPath/third_party/dart/project/lib/generated.dart');
+ '$bazelOrBlazeGenfilesPath/third_party/dart/project/lib/generated.dart');
writeFile(generatedFilePath, 'my_fun() {}');
+ _createSymlinks(prefix);
writeFile(commandLogPath, 'Build completed successfully');
await processedNotification.future;
@@ -145,6 +155,7 @@
// Now delete the file completely.
await resetCompleterAndErrors();
File(generatedFilePath).deleteSync();
+ _deleteSymlinks(prefix);
writeFile(commandLogPath, 'Build did NOT complete successfully');
await processedNotification.future;
@@ -153,9 +164,22 @@
// And finally re-add the correct file -- errors should go away once again.
await resetCompleterAndErrors();
writeFile(generatedFilePath, 'my_fun() {}');
+ _createSymlinks(prefix);
writeFile(commandLogPath, 'Build completed successfully');
await processedNotification.future;
expect(errors, isEmpty);
}
+
+ void _createSymlinks(String prefix) {
+ Link(inWorkspace('$prefix-out')).createSync(bazelOrBlazeOutPath);
+ Link(inWorkspace('$prefix-bin')).createSync(bazelOrBlazeBinPath);
+ Link(inWorkspace('$prefix-genfiles')).createSync(bazelOrBlazeGenfilesPath);
+ }
+
+ void _deleteSymlinks(String prefix) {
+ Link(inWorkspace('$prefix-out')).deleteSync();
+ Link(inWorkspace('$prefix-bin')).deleteSync();
+ Link(inWorkspace('$prefix-genfiles')).deleteSync();
+ }
}
diff --git a/pkg/analyzer/lib/src/workspace/bazel_watcher.dart b/pkg/analyzer/lib/src/workspace/bazel_watcher.dart
index 50ceb0c..05ce4a6 100644
--- a/pkg/analyzer/lib/src/workspace/bazel_watcher.dart
+++ b/pkg/analyzer/lib/src/workspace/bazel_watcher.dart
@@ -110,10 +110,21 @@
/// exist.
_TimestampAndLength? _pollOne(String path) {
try {
- var file = _provider.getFile(path);
- var timestamp = file.modificationStamp;
- var length = file.lengthSync;
- return _TimestampAndLength(timestamp, length);
+ // This might seem a bit convoluted but is necessary to deal with a
+ // symlink to a directory (e.g., `bazel-bin`).
+ var resource = _provider.getResource(
+ _provider.getResource(path).resolveSymbolicLinksSync().path);
+ if (resource is File) {
+ var timestamp = resource.modificationStamp;
+ var length = resource.lengthSync;
+ return _TimestampAndLength(timestamp, length);
+ } else if (resource is Folder) {
+ // `ResourceProvider` doesn't currently support getting timestamps of a
+ // folder, so we use a dummy value here. But it's still useful: this
+ // will correctly generate `ADD` or `REMOVE` events (we'll be just
+ // unable to generate any `CHANGE` events).
+ return _TimestampAndLength(0, 0);
+ }
} on FileSystemException catch (_) {
// File doesn't exist, so return null.
return null;
diff --git a/pkg/analyzer/test/src/workspace/bazel_watcher_test.dart b/pkg/analyzer/test/src/workspace/bazel_watcher_test.dart
index 16df3e4..84faee8 100644
--- a/pkg/analyzer/test/src/workspace/bazel_watcher_test.dart
+++ b/pkg/analyzer/test/src/workspace/bazel_watcher_test.dart
@@ -233,6 +233,47 @@
recPort.close();
}
+ void test_bazelFileWatcherWithFolder() async {
+ _addResources([
+ '/workspace/WORKSPACE',
+ ]);
+
+ // The `_addResources`/`_deleteResources` functions recognize a folder by a
+ // trailing `/`, but everywhere else we need to use normalized paths.
+ var addFolder = (path) => _addResources(['$path/']);
+ var deleteFolder = (path) => _deleteResources(['$path/']);
+
+ var candidates = [
+ convertPath('/workspace/bazel-out'),
+ convertPath('/workspace/blaze-out'),
+ ];
+ var watcher = BazelFilePoller(resourceProvider, candidates);
+
+ // First do some tests with the first candidate path.
+ addFolder(candidates[0]);
+ var event = watcher.poll()!;
+
+ expect(event.type, ChangeType.ADD);
+ expect(event.path, candidates[0]);
+
+ deleteFolder(candidates[0]);
+ event = watcher.poll()!;
+
+ expect(event.type, ChangeType.REMOVE);
+ expect(event.path, candidates[0]);
+
+ // Now check that if we add the *second* candidate, we'll get the
+ // notification for it.
+ addFolder(candidates[1]);
+ event = watcher.poll()!;
+
+ expect(event.type, ChangeType.ADD);
+ expect(event.path, candidates[1]);
+
+ // Next poll should be `null` since there were no changes.
+ expect(watcher.poll(), isNull);
+ }
+
/// Create new files and directories from [paths].
void _addResources(List<String> paths) {
for (String path in paths) {
