Skip to content

Commit

Permalink
Replace more Bazel with Blaze, some simplifications.
Browse files Browse the repository at this point in the history
Change-Id: I096860e17ce56c2f602718f11706f24989c1f65b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255141
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
  • Loading branch information
scheglov authored and Commit Bot committed Aug 15, 2022
1 parent c66faf9 commit 4eb511d
Show file tree
Hide file tree
Showing 27 changed files with 235 additions and 338 deletions.
4 changes: 2 additions & 2 deletions pkg/analysis_server/lib/src/analysis_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ abstract class AnalysisServer {
ProcessRunner? processRunner,
this.notificationManager, {
this.requestStatistics,
bool enableBazelWatcher = false,
bool enableBlazeWatcher = false,
}) : resourceProvider = OverlayResourceProvider(baseResourceProvider),
pubApi = PubApi(instrumentationService, httpClient,
Platform.environment['PUB_HOSTED_URL']) {
Expand Down Expand Up @@ -228,7 +228,7 @@ abstract class AnalysisServer {
analysisPerformanceLogger,
analysisDriverScheduler,
instrumentationService,
enableBazelWatcher: enableBazelWatcher,
enableBlazeWatcher: enableBlazeWatcher,
);
searchEngine = SearchEngineImpl(driverMap.values);
}
Expand Down
66 changes: 13 additions & 53 deletions pkg/analysis_server/lib/src/context_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ import 'package:path/path.dart' as path;
import 'package:watcher/watcher.dart';
import 'package:yaml/yaml.dart';

/// Enables watching of files generated by Bazel.
/// Enables watching of files generated by Blaze.
///
/// TODO(michalt): This is a temporary flag that we use to disable this
/// functionality due its performance issues. We plan to benchmark and optimize
/// it and re-enable it everywhere.
/// Not private to enable testing.
/// NB: If you set this to `false` remember to disable the
/// `test/integration/serve/bazel_changes_test.dart`.
/// `test/integration/serve/blaze_changes_test.dart`.
var experimentalEnableBlazeWatching = true;

/// Class that maintains a mapping from included/excluded paths to a set of
Expand Down Expand Up @@ -193,20 +193,20 @@ class ContextManagerImpl implements ContextManager {
/// Subscriptions to watch included resources for changes.
final List<StreamSubscription<WatchEvent>> watcherSubscriptions = [];

/// For each folder, stores the subscription to the Bazel workspace so that we
/// For each folder, stores the subscription to the Blaze workspace so that we
/// can establish watches for the generated files.
final blazeSearchSubscriptions =
<Folder, StreamSubscription<BlazeSearchInfo>>{};

/// The watcher service running in a separate isolate to watch for changes
/// to files generated by Bazel.
/// to files generated by Blaze.
///
/// Might be `null` if watching Bazel files is not enabled.
/// Might be `null` if watching Blaze files is not enabled.
BlazeFileWatcherService? blazeWatcherService;

/// The subscription to changes in the files watched by [blazeWatcherService].
///
/// Might be `null` if watching Bazel files is not enabled.
/// Might be `null` if watching Blaze files is not enabled.
StreamSubscription<List<WatchEvent>>? blazeWatcherSubscription;

/// For each [Folder] store which files are being watched. This allows us to
Expand All @@ -233,9 +233,9 @@ class ContextManagerImpl implements ContextManager {
this._performanceLog,
this._scheduler,
this._instrumentationService,
{required bool enableBazelWatcher})
{required bool enableBlazeWatcher})
: pathContext = resourceProvider.pathContext {
if (enableBazelWatcher) {
if (enableBlazeWatcher) {
blazeWatcherService = BlazeFileWatcherService(_instrumentationService);
blazeWatcherSubscription = blazeWatcherService!.events
.listen((events) => _handleBlazeWatchEvents(events));
Expand Down Expand Up @@ -630,7 +630,6 @@ class ContextManagerImpl implements ContextManager {
for (var path in watched.paths) {
blazeWatcherService!.stopWatching(watched.workspace, path);
}
_stopWatchingBlazeBinPaths(watched);
}
blazeSearchSubscriptions.remove(rootFolder)?.cancel();
driverMap.remove(rootFolder);
Expand All @@ -650,16 +649,11 @@ class ContextManagerImpl implements ContextManager {
}
}

List<String> _getPossibleBlazeBinPaths(_BlazeWatchedFiles watched) => [
pathContext.join(watched.workspace, 'bazel-bin'),
pathContext.join(watched.workspace, 'blaze-bin'),
];

/// Establishes watch(es) for the Bazel generated files provided in
/// Establishes watch(es) for the Blaze generated files provided in
/// [notification].
///
/// Whenever the files change, we trigger re-analysis. This allows us to react
/// to creation/modification of files that were generated by Bazel.
/// to creation/modification of files that were generated by Blaze.
void _handleBlazeSearchInfo(
Folder folder, String workspace, BlazeSearchInfo info) {
final blazeWatcherService = this.blazeWatcherService;
Expand All @@ -673,19 +667,8 @@ class ContextManagerImpl implements ContextManager {
if (added) blazeWatcherService.startWatching(workspace, info);
}

/// Notifies the drivers that a generated Bazel file has changed.
/// Notifies the drivers that a generated Blaze file has changed.
void _handleBlazeWatchEvents(List<WatchEvent> events) {
// 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 = blazeWatchedPathsPerFolder.values
.expand((watched) => _getPossibleBlazeBinPaths(watched))
.toSet();
if (events.any((event) => bazelSymlinkPaths.contains(event.path))) {
refresh();
return;
}

// If a file was created or removed, the URI resolution is likely wrong.
// Do as for `package_config.json` changes - recreate all contexts.
if (events
Expand Down Expand Up @@ -807,34 +790,12 @@ class ContextManagerImpl implements ContextManager {
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 _startWatchingBlazeBinPaths(_BlazeWatchedFiles watched) {
var watcherService = blazeWatcherService;
if (watcherService == null) return;
var paths = _getPossibleBlazeBinPaths(watched);
watcherService.startWatching(
watched.workspace, BlazeSearchInfo(paths[0], paths));
}

/// Stops watching for the `bazel-bin` and `blaze-bin` symlinks.
void _stopWatchingBlazeBinPaths(_BlazeWatchedFiles watched) {
var watcherService = blazeWatcherService;
if (watcherService == null) return;
var paths = _getPossibleBlazeBinPaths(watched);
watcherService.stopWatching(watched.workspace, paths[0]);
}

/// Listens to files generated by Bazel that were found or searched for.
/// Listens to files generated by Blaze that were found or searched for.
///
/// This is handled specially because the files are outside the package
/// folder, but we still want to watch for changes to them.
///
/// Does nothing if the [driver] is not in a Bazel workspace.
/// Does nothing if the [analysisDriver] is not in a Blaze workspace.
void _watchBlazeFilesIfNeeded(Folder folder, AnalysisDriver analysisDriver) {
if (!experimentalEnableBlazeWatching) return;
var watcherService = blazeWatcherService;
Expand All @@ -849,7 +810,6 @@ class ContextManagerImpl implements ContextManager {

var watched = _BlazeWatchedFiles(workspace.root);
blazeWatchedPathsPerFolder[folder] = watched;
_startWatchingBlazeBinPaths(watched);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/analysis_server/lib/src/legacy_analysis_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ class LegacyAnalysisServer extends AnalysisServer {
DiagnosticServer? diagnosticServer,
this.detachableFileSystemManager,
// Disable to avoid using this in unit tests.
bool enableBazelWatcher = false,
bool enableBlazeWatcher = false,
}) : super(
options,
sdkManager,
Expand All @@ -394,7 +394,7 @@ class LegacyAnalysisServer extends AnalysisServer {
processRunner,
NotificationManager(channel, baseResourceProvider.pathContext),
requestStatistics: requestStatistics,
enableBazelWatcher: enableBazelWatcher,
enableBlazeWatcher: enableBlazeWatcher,
) {
var contextManagerCallbacks =
ServerContextManagerCallbacks(this, resourceProvider);
Expand Down
4 changes: 2 additions & 2 deletions pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class LspAnalysisServer extends AnalysisServer {
DiagnosticServer? diagnosticServer,
this.detachableFileSystemManager,
// Disable to avoid using this in unit tests.
bool enableBazelWatcher = false,
bool enableBlazeWatcher = false,
}) : super(
options,
sdkManager,
Expand All @@ -152,7 +152,7 @@ class LspAnalysisServer extends AnalysisServer {
httpClient,
processRunner,
LspNotificationManager(channel, baseResourceProvider.pathContext),
enableBazelWatcher: enableBazelWatcher,
enableBlazeWatcher: enableBlazeWatcher,
) {
notificationManager.server = this;
messageHandler = UninitializedStateMessageHandler(this);
Expand Down
2 changes: 1 addition & 1 deletion pkg/analysis_server/lib/src/lsp/lsp_socket_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class LspSocketServer implements AbstractSocketServer {
instrumentationService,
diagnosticServer: diagnosticServer,
detachableFileSystemManager: detachableFileSystemManager,
enableBazelWatcher: true,
enableBlazeWatcher: true,
);
detachableFileSystemManager?.setAnalysisServer(server);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ String _getPath(ResourceProvider provider, Element? e,
}

var path = source.fullName;
var bazelWorkspace = BlazeWorkspace.find(provider, path);
if (bazelWorkspace != null) {
return provider.pathContext.relative(path, from: bazelWorkspace.root);
var blazeWorkspace = BlazeWorkspace.find(provider, path);
if (blazeWorkspace != null) {
return provider.pathContext.relative(path, from: blazeWorkspace.root);
}
var gnWorkspace = GnWorkspace.find(provider, path);
if (gnWorkspace != null) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/analysis_server/lib/src/socket_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class SocketServer implements AbstractSocketServer {
requestStatistics: requestStatistics,
diagnosticServer: diagnosticServer,
detachableFileSystemManager: detachableFileSystemManager,
enableBazelWatcher: true,
enableBlazeWatcher: true,
);
detachableFileSystemManager?.setAnalysisServer(server);
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/analysis_server/test/analysis/get_hover_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ void main() {
class AnalysisHoverBlazeTest extends BlazeWorkspaceAnalysisServerTest {
Future<void> test_blaze_notOwnedUri() async {
newFile(
'$workspaceRootPath/bazel-genfiles/dart/my/lib/test.dart',
'$workspaceRootPath/blaze-genfiles/dart/my/lib/test.dart',
'// generated',
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,13 @@ class BlazeChangesTest extends AbstractAnalysisServerIntegrationTest {
sourceDirectory = Directory(inWorkspace('third_party/dart/project'));
sourceDirectory.createSync(recursive: true);

blazeRoot = inTmpDir('bazel_or_blaze_root');
blazeRoot = inTmpDir('blaze_root');
Directory(blazeRoot).createSync(recursive: true);

blazeOutPath =
'$blazeRoot/execroot/bazel_or_blaze_workspace/bazel_or_blaze-out';
blazeBinPath =
'$blazeRoot/execroot/bazel_or_blaze_workspace/bazel_or_blaze-out/bin';
blazeOutPath = '$blazeRoot/execroot/blaze_workspace/blaze-out';
blazeBinPath = '$blazeRoot/execroot/blaze_workspace/blaze-out/bin';
blazeGenfilesPath =
'$blazeRoot/execroot/bazel_or_blaze_workspace/bazel_or_blaze-out/genfiles';
'$blazeRoot/execroot/blaze_workspace/blaze-out/genfiles';

Directory(inTmpDir(blazeOutPath)).createSync(recursive: true);
Directory(inTmpDir(blazeBinPath)).createSync(recursive: true);
Expand Down
4 changes: 2 additions & 2 deletions pkg/analysis_server/test/lsp/initialization_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class InitializationTest extends AbstractLspAnalysisServerTest {

Future<void> test_blazeWorkspace() async {
var workspacePath = '/home/user/ws';
// Make it a Bazel workspace.
// Make it a Blaze workspace.
newFile('$workspacePath/${file_paths.blazeWorkspaceMarker}', '');

var packagePath = '$workspacePath/team/project1';
Expand Down Expand Up @@ -816,7 +816,7 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
final file1 = convertPath('/home/nonProject/file1.dart');
newFile(file1, '');

// Make /home a bazel workspace.
// Make /home a Blaze workspace.
newFile('/home/${file_paths.blazeWorkspaceMarker}', '');

await initialize(allowEmptyRootUri: true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ import 'package:test/222/new_name.dart';

@failingTest
Future<void> test_file_imported_with_package_uri_lib_change() async {
// The current testing stack does not support creating such bazel roots
// The current testing stack does not support creating such Blaze roots
var file = newFile('/home/test0/test1/test2/lib/111/name.dart', '');
addTestSource(r'''
import 'package:test0.test1.test2/111/name.dart';
Expand All @@ -101,7 +101,7 @@ import 'package:test0.test1.test3/111/name.dart';

@failingTest
Future<void> test_file_imported_with_package_uri_lib_change_down() async {
// The current testing stack does not support creating such bazel roots
// The current testing stack does not support creating such Blaze roots
var file = newFile('/home/test0/test1/test2/lib/111/name.dart', '');
addTestSource(r'''
import 'package:test0.test1.test2/111/name.dart';
Expand All @@ -124,7 +124,7 @@ import 'package:test0.test1.test2.test3/111/name.dart';

@failingTest
Future<void> test_file_imported_with_package_uri_lib_change_up() async {
// The current testing stack does not support creating such bazel roots
// The current testing stack does not support creating such Blaze roots
var file = newFile('/home/test0/test1/test2/lib/111/name.dart', '');
addTestSource(r'''
import 'package:test0.test1.test2/111/name.dart';
Expand Down
6 changes: 3 additions & 3 deletions pkg/analysis_server/test/src/plugin/plugin_manager_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -493,11 +493,11 @@ class PluginManagerTest with ResourceProviderMixin, _ContextRoot {

void test_pathsFor_withPubspec_inBlazeWorkspace() {
//
// Build a Bazel workspace containing four packages, including the plugin.
// Build a Blaze workspace containing four packages, including the plugin.
//
newFile('/workspaceRoot/${file_paths.blazeWorkspaceMarker}', '');
newFolder('/workspaceRoot/bazel-bin');
newFolder('/workspaceRoot/bazel-genfiles');
newFolder('/workspaceRoot/blaze-bin');
newFolder('/workspaceRoot/blaze-genfiles');

String newPackage(String packageName, [List<String>? dependencies]) {
var packageRoot =
Expand Down
2 changes: 1 addition & 1 deletion pkg/analysis_server/test/src/utilities/mock_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void _cacheFiles(Map<String, String> cachedFiles) {
}

/// Helper for copying files from "tests/mock_packages" to memory file system
/// for Bazel.
/// for Blaze.
class BlazeMockPackages {
static final BlazeMockPackages instance = BlazeMockPackages._();

Expand Down
2 changes: 1 addition & 1 deletion pkg/analyzer/lib/dart/analysis/results.dart
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class NotLibraryButPartResult
/// The type of [InvalidResult] returned when the given file path does not
/// represent the corresponding URI.
///
/// This usually happens in Bazel workspaces, when a URI is resolved to
/// This usually happens in Blaze workspaces, when a URI is resolved to
/// a generated file, but there is also a writable file to which this URI
/// would be resolved, if there were no generated file.
///
Expand Down
2 changes: 1 addition & 1 deletion pkg/analyzer/lib/src/dart/analysis/file_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1352,7 +1352,7 @@ class FileSystemState {
/// Return `true` if there is a URI that can be resolved to the [path].
///
/// When a file exists, but for the URI that corresponds to the file is
/// resolved to another file, e.g. a generated one in Bazel, Gn, etc, we
/// resolved to another file, e.g. a generated one in Blaze, Gn, etc, we
/// cannot analyze the original file.
bool hasUri(String path) {
bool? flag = _hasUriForPath[path];
Expand Down
Loading

3 comments on commit 4eb511d

@bernaferrari
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this make it harder for people outside Google to compile it? Because standard people have no access to blaze.

@mraleph
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bernaferrari this change is just a renaming, see #49629 for context. We don't build any part of the open source Dart SDK with Bazel/Blaze anyway.

@bernaferrari
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thanks. Missed the issue.

Please sign in to comment.