From 9304826679b813e488ad2854e580c63a7c8e67bd Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Thu, 13 Jun 2024 20:27:22 +0000 Subject: [PATCH] [ DDS ] Refactor DDS entrypoint to allow for overriding behavior in google3 This moves the CLI logic into lib/src/dds_cli_entrypoint.dart so it can be invoked by a custom google3 entrypoint (bin/dds.dart can't be imported using a package: URI). Since google3 will be able to perform some custom configuration of DDS, google3 related logic (e.g., `BazelUriConverter`) is also removed. This is technically a breaking change, but should be safe as this functionality isn't currently being used. Change-Id: I54d8a9927ff2df70e013ca5c8bc1d510b0b95f02 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371520 Reviewed-by: Derek Xu Commit-Queue: Derek Xu Auto-Submit: Ben Konyi --- pkg/dds/bin/dds.dart | 146 +------------- pkg/dds/lib/src/arg_parser.dart | 7 - pkg/dds/lib/src/bazel_uri_converter.dart | 240 ----------------------- pkg/dds/lib/src/dds_cli_entrypoint.dart | 149 ++++++++++++++ pkg/dds/test/uri_converter_test.dart | 51 ----- 5 files changed, 154 insertions(+), 439 deletions(-) delete mode 100644 pkg/dds/lib/src/bazel_uri_converter.dart create mode 100644 pkg/dds/lib/src/dds_cli_entrypoint.dart diff --git a/pkg/dds/bin/dds.dart b/pkg/dds/bin/dds.dart index 2741855d316..9a90fc383cb 100644 --- a/pkg/dds/bin/dds.dart +++ b/pkg/dds/bin/dds.dart @@ -2,147 +2,11 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'dart:convert'; -import 'dart:io'; - -import 'package:dds/dds.dart'; -import 'package:dds/src/arg_parser.dart'; -import 'package:dds/src/bazel_uri_converter.dart'; - -import 'package:path/path.dart' as path; - -Uri _getDevToolsAssetPath() { - final dartDir = File(Platform.resolvedExecutable).parent.path; - final fullSdk = dartDir.endsWith('bin'); - return Uri.file( - fullSdk - ? path.absolute( - dartDir, - 'resources', - 'devtools', - ) - : path.absolute( - dartDir, - 'devtools', - ), - ); -} +import 'package:dds/src/dds_cli_entrypoint.dart'; Future main(List args) async { - final argParser = DartDevelopmentServiceOptions.createArgParser( - includeHelp: true, - ); - final argResults = argParser.parse(args); - if (args.isEmpty || argResults.wasParsed('help')) { - print(''' -Starts a Dart Development Service (DDS) instance. - -Usage: -${argParser.usage} - '''); - return; - } - - // This URI is provided by the VM service directly so don't bother doing a - // lookup. - final remoteVmServiceUri = Uri.parse( - argResults[DartDevelopmentServiceOptions.vmServiceUriOption], - ); - - // Resolve the address which is potentially provided by the user. - late InternetAddress address; - final bindAddress = - argResults[DartDevelopmentServiceOptions.bindAddressOption]; - try { - final addresses = await InternetAddress.lookup(bindAddress); - // Prefer IPv4 addresses. - for (int i = 0; i < addresses.length; i++) { - address = addresses[i]; - if (address.type == InternetAddressType.IPv4) break; - } - } on SocketException catch (e, st) { - writeErrorResponse('Invalid bind address: $bindAddress', st); - return; - } - - final portString = argResults[DartDevelopmentServiceOptions.bindPortOption]; - int port; - try { - port = int.parse(portString); - } on FormatException catch (e, st) { - writeErrorResponse('Invalid port: $portString', st); - return; - } - final serviceUri = Uri( - scheme: 'http', - host: address.address, - port: port, - ); - final disableServiceAuthCodes = - argResults[DartDevelopmentServiceOptions.disableServiceAuthCodesFlag]; - - final serveDevTools = - argResults[DartDevelopmentServiceOptions.serveDevToolsFlag]; - final devToolsServerAddressStr = - argResults[DartDevelopmentServiceOptions.devToolsServerAddressOption]; - Uri? devToolsBuildDirectory; - final devToolsServerAddress = devToolsServerAddressStr == null - ? null - : Uri.parse(devToolsServerAddressStr); - if (serveDevTools) { - devToolsBuildDirectory = _getDevToolsAssetPath(); - } - final enableServicePortFallback = - argResults[DartDevelopmentServiceOptions.enableServicePortFallbackFlag]; - final cachedUserTags = - argResults[DartDevelopmentServiceOptions.cachedUserTagsOption]; - final google3WorkspaceRoot = - argResults[DartDevelopmentServiceOptions.google3WorkspaceRootOption]; - - try { - final dds = await DartDevelopmentService.startDartDevelopmentService( - remoteVmServiceUri, - serviceUri: serviceUri, - enableAuthCodes: !disableServiceAuthCodes, - ipv6: address.type == InternetAddressType.IPv6, - devToolsConfiguration: serveDevTools && devToolsBuildDirectory != null - ? DevToolsConfiguration( - enable: serveDevTools, - customBuildDirectoryPath: devToolsBuildDirectory, - devToolsServerAddress: devToolsServerAddress, - ) - : null, - enableServicePortFallback: enableServicePortFallback, - cachedUserTags: cachedUserTags, - uriConverter: google3WorkspaceRoot != null - ? BazelUriConverter(google3WorkspaceRoot).uriToPath - : null, - ); - final dtdInfo = dds.hostedDartToolingDaemon; - stderr.write(json.encode({ - 'state': 'started', - 'ddsUri': dds.uri.toString(), - if (dds.devToolsUri != null) 'devToolsUri': dds.devToolsUri.toString(), - if (dtdInfo != null) - 'dtd': { - 'uri': dtdInfo.uri, - }, - })); - stderr.close(); - } catch (e, st) { - writeErrorResponse(e, st); - } finally { - // Always close stderr to notify tooling that DDS has finished writing - // launch details. - await stderr.close(); - } -} - -void writeErrorResponse(Object e, StackTrace st) { - stderr.write(json.encode({ - 'state': 'error', - 'error': '$e', - 'stacktrace': '$st', - if (e is DartDevelopmentServiceException) 'ddsExceptionDetails': e.toJson(), - })); + // This level of indirection is only here so DDS can be configured for + // google3 specific functionality as it's not possible to import files under + // a package's bin directory to wrap the entrypoint. + await runDartDevelopmentServiceFromCLI(args); } diff --git a/pkg/dds/lib/src/arg_parser.dart b/pkg/dds/lib/src/arg_parser.dart index fd605b0890c..c609edc881f 100644 --- a/pkg/dds/lib/src/arg_parser.dart +++ b/pkg/dds/lib/src/arg_parser.dart @@ -16,7 +16,6 @@ abstract class DartDevelopmentServiceOptions { static const enableServicePortFallbackFlag = 'enable-service-port-fallback'; static const cachedUserTagsOption = 'cached-user-tags'; static const devToolsServerAddressOption = 'devtools-server-address'; - static const google3WorkspaceRootOption = 'google3-workspace-root'; static ArgParser createArgParser({ int? usageLineLength, @@ -78,12 +77,6 @@ abstract class DartDevelopmentServiceOptions { help: 'A set of UserTag names used to determine which CPU samples are ' 'cached by DDS.', defaultsTo: [], - ) - ..addOption( - google3WorkspaceRootOption, - help: 'Sets the Google3 workspace root used for google3:// URI ' - 'resolution.', - hide: !verbose, ); if (includeHelp) { argParser.addFlag('help', negatable: false); diff --git a/pkg/dds/lib/src/bazel_uri_converter.dart b/pkg/dds/lib/src/bazel_uri_converter.dart deleted file mode 100644 index 6710c3475c9..00000000000 --- a/pkg/dds/lib/src/bazel_uri_converter.dart +++ /dev/null @@ -1,240 +0,0 @@ -// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -// TODO(bkonyi): consider moving to lib/ once this package is no longer shipped -// via pub. - -import 'dart:async'; -import 'dart:collection'; -import 'dart:io' as io; - -import 'package:path/path.dart'; - -/// A URI converter able to handle google3 URIs. -class BazelUriConverter { - final _absoluteUriToFileCache = HashMap(); - final Context _context = context; - final _binPaths = []; - final String _root; - String? _genfiles; - final _bazelCandidateFiles = StreamController.broadcast(); - - BazelUriConverter(String originalPath) : _root = originalPath { - _ensureAbsoluteAndNormalized(originalPath); - // Note: The analyzer code this code is based on checked multiple things - // while trying to find a google3 workspace - the presence of a blaze-out - // directory, a READONLY file or a WORKSPACE file. If the blaze-out - // structure changes, then potentially check for one of the others. - final blazeOutDir = - io.Directory(normalize(join(originalPath, 'blaze-out'))); - if (blazeOutDir.existsSync()) { - _binPaths.addAll(_findBinFolderPaths(blazeOutDir)); - _binPaths.add(normalize(join(originalPath, 'blaze-bin'))); - _genfiles = normalize(join(originalPath, 'blaze-genfiles')); - } - } - - String? uriToPath(String uriStr) { - final uri = Uri.parse(uriStr); - final cached = _absoluteUriToFileCache[uri]; - if (cached != null) { - return cached; - } - - if (uri.isScheme('file')) { - final path = fileUriToNormalizedPath(_context, uri); - final pathRelativeToRoot = _relativeToRoot(path); - if (pathRelativeToRoot == null) return null; - final fullFilePath = _context.join(_root, pathRelativeToRoot); - final file = _findFile(fullFilePath); - if (file != null) { - _absoluteUriToFileCache[uri] = file.path; - return file.path; - } - } - // If the URI passed has a google3 scheme, this means we don't need to - // convert from a package URI and we only need to prepend the root path - // (i.e. the path that contains the user's workspace name). - if (uri.isScheme('google3')) { - // Remove the first character since uri.path starts with '/', though we - // know this is not an absolute path. - return _context.join(_root, uri.path.substring(1)); - } - if (!uri.isScheme('package')) { - // TODO(b/261234406): Handle `dart:` URIs. - if (uri.isScheme('dart')) { - // This doesn't return the actual location of the Dart SDK, which is - // more complicated. The purpose of returning something here is to - // avoid having the external version of URI converter resolving a path - // that is incorrect in a way that confuse VS Code's stack trace. - return _context.join(_root, uri.path); - } - return null; - } - final uriPath = Uri.decodeComponent(uri.path); - final slash = uriPath.indexOf('/'); - - // If the path either starts with a slash or has no slash, it is invalid. - if (slash < 1) { - return null; - } - - if (uriPath.contains('//') || uriPath.contains('..')) { - return null; - } - - final packageName = uriPath.substring(0, slash); - - final fileUriPart = uriPath.substring(slash + 1); - if (fileUriPart.isEmpty) { - return null; - } - - final filePath = fileUriPart.replaceAll('/', _context.separator); - - if (!packageName.contains('.')) { - final fullFilePath = _context.join( - _root, 'third_party', 'dart', packageName, 'lib', filePath); - final file = _findFile(fullFilePath); - if (file != null) { - _absoluteUriToFileCache[uri] = file.path; - return file.path; - } - } else { - final packagePath = packageName.replaceAll('.', _context.separator); - final fullFilePath = _context.join(_root, packagePath, 'lib', filePath); - final file = _findFile(fullFilePath); - if (file != null) { - _absoluteUriToFileCache[uri] = file.path; - return file.path; - } - } - return null; - } - - String fileUriToNormalizedPath(Context context, Uri fileUri) { - assert(fileUri.isScheme('file')); - var contextPath = context.fromUri(fileUri); - contextPath = context.normalize(contextPath); - return contextPath; - } - - /// The file system abstraction supports only absolute and normalized paths. - /// This method is used to validate any input paths to prevent errors later. - void _ensureAbsoluteAndNormalized(String path) { - assert(() { - if (!_context.isAbsolute(path)) { - throw ArgumentError('Path must be absolute : $path'); - } - if (_context.normalize(path) != path) { - throw ArgumentError('Path must be normalized : $path'); - } - return true; - }()); - } - - List _findBinFolderPaths(io.Directory blazeOutDir) { - final binPaths = []; - for (final child - in blazeOutDir.listSync(recursive: false).whereType()) { - // Children are folders denoting architectures and build flags, like - // 'k8-opt', 'k8-fastbuild', perhaps 'host'. - - final possibleBin = io.Directory(normalize(join(child.path, 'bin'))); - if (possibleBin.existsSync()) { - binPaths.add(possibleBin.path); - } - } - return binPaths; - } - - String? _relativeToRoot(String p) { - // genfiles - if (_genfiles != null && _context.isWithin(_genfiles!, p)) { - return context.relative(p, from: _genfiles!); - } - // bin - for (final bin in _binPaths) { - if (context.isWithin(bin, p)) { - return context.relative(p, from: bin); - } - } - - // We are no longer checking for READONLY? Or should I add back? - // READONLY - // final readonly = this.readonly; - // if (readonly != null) { - // if (context.isWithin(readonly, p)) { - // return context.relative(p, from: readonly); - // } - // } - // Not generated - if (context.isWithin(_root, p)) { - return context.relative(p, from: _root); - } - // Failed reverse lookup - return null; - } - - /// Return the file with the given [absolutePath], looking first into - /// directories for generated files: `bazel-bin` and `bazel-genfiles`, and - /// then into the workspace originalPath. The file in the workspace originalPath is returned - /// even if it does not exist. Return `null` if the given [absolutePath] is - /// not in the workspace [originalPath]. - io.File? _findFile(String absolutePath) { - try { - final relative = _context.relative(absolutePath, from: _root); - if (relative == '.') { - return null; - } - // First check genfiles and bin directories. Note that we always use the - // symlinks and not the [binPaths] or [genfiles] to make sure we use the - // files corresponding to the most recent build configuration and get - // consistent view of all the generated files. - final generatedCandidates = ['blaze-genfiles', 'blaze-bin'] - .map((prefix) => context.join(_root, context.join(prefix, relative))); - for (final path in generatedCandidates) { - _ensureAbsoluteAndNormalized(path); - final file = io.File(path); - if (file.existsSync()) { - _bazelCandidateFiles - .add(BazelSearchInfo(relative, generatedCandidates.toList())); - return file; - } - } - // Writable - _ensureAbsoluteAndNormalized(absolutePath); - final writableFile = io.File(absolutePath); - if (writableFile.existsSync()) { - return writableFile; - } - - // If we couldn't find the file, assume that it has not yet been - // generated, so send an event with all the paths that we tried. - _bazelCandidateFiles - .add(BazelSearchInfo(relative, generatedCandidates.toList())); - // Not generated, return the default one. - return writableFile; - } catch (_) { - return null; - } - } -} - -/// Notification that we issue when searching for generated files in a Bazel -/// workspace. -/// -/// This allows clients to watch for changes to the generated files. -class BazelSearchInfo { - /// Candidate paths that we searched. - final List candidatePaths; - - /// Absolute path that we tried searching for. - /// - /// This is not necessarily the path of the actual file that will be used. See - /// `BazelWorkspace.findFile` for details. - final String requestedPath; - - BazelSearchInfo(this.requestedPath, this.candidatePaths); -} diff --git a/pkg/dds/lib/src/dds_cli_entrypoint.dart b/pkg/dds/lib/src/dds_cli_entrypoint.dart new file mode 100644 index 00000000000..1680cba55ca --- /dev/null +++ b/pkg/dds/lib/src/dds_cli_entrypoint.dart @@ -0,0 +1,149 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// TODO(bkonyi): move this file to lib/dds_cli_entrypoint.dart once package:dds +// is no longer shipped via pub. +import 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as path; + +import '../dds.dart'; +import 'arg_parser.dart'; + +Uri _getDevToolsAssetPath() { + final dartDir = File(Platform.resolvedExecutable).parent.path; + final fullSdk = dartDir.endsWith('bin'); + return Uri.file( + fullSdk + ? path.absolute( + dartDir, + 'resources', + 'devtools', + ) + : path.absolute( + dartDir, + 'devtools', + ), + ); +} + +// TODO(bkonyi): allow for injection of custom DevTools handlers in google3. +Future runDartDevelopmentServiceFromCLI( + List args, { + String? Function(String)? uriConverter, +}) async { + final argParser = DartDevelopmentServiceOptions.createArgParser( + includeHelp: true, + ); + final argResults = argParser.parse(args); + if (args.isEmpty || argResults.wasParsed('help')) { + print(''' +Starts a Dart Development Service (DDS) instance. + +Usage: +${argParser.usage} + '''); + return; + } + + // This URI is provided by the VM service directly so don't bother doing a + // lookup. + final remoteVmServiceUri = Uri.parse( + argResults[DartDevelopmentServiceOptions.vmServiceUriOption], + ); + + // Resolve the address which is potentially provided by the user. + late InternetAddress address; + final bindAddress = + argResults[DartDevelopmentServiceOptions.bindAddressOption]; + try { + final addresses = await InternetAddress.lookup(bindAddress); + // Prefer IPv4 addresses. + for (int i = 0; i < addresses.length; i++) { + address = addresses[i]; + if (address.type == InternetAddressType.IPv4) break; + } + } on SocketException catch (e, st) { + writeErrorResponse('Invalid bind address: $bindAddress', st); + return; + } + + final portString = argResults[DartDevelopmentServiceOptions.bindPortOption]; + int port; + try { + port = int.parse(portString); + } on FormatException catch (e, st) { + writeErrorResponse('Invalid port: $portString', st); + return; + } + final serviceUri = Uri( + scheme: 'http', + host: address.address, + port: port, + ); + final disableServiceAuthCodes = + argResults[DartDevelopmentServiceOptions.disableServiceAuthCodesFlag]; + + final serveDevTools = + argResults[DartDevelopmentServiceOptions.serveDevToolsFlag]; + final devToolsServerAddressStr = + argResults[DartDevelopmentServiceOptions.devToolsServerAddressOption]; + Uri? devToolsBuildDirectory; + final devToolsServerAddress = devToolsServerAddressStr == null + ? null + : Uri.parse(devToolsServerAddressStr); + if (serveDevTools) { + devToolsBuildDirectory = _getDevToolsAssetPath(); + } + final enableServicePortFallback = + argResults[DartDevelopmentServiceOptions.enableServicePortFallbackFlag]; + final cachedUserTags = + argResults[DartDevelopmentServiceOptions.cachedUserTagsOption]; + + try { + final dds = await DartDevelopmentService.startDartDevelopmentService( + remoteVmServiceUri, + serviceUri: serviceUri, + enableAuthCodes: !disableServiceAuthCodes, + ipv6: address.type == InternetAddressType.IPv6, + devToolsConfiguration: serveDevTools && devToolsBuildDirectory != null + ? DevToolsConfiguration( + enable: serveDevTools, + customBuildDirectoryPath: devToolsBuildDirectory, + devToolsServerAddress: devToolsServerAddress, + ) + : null, + enableServicePortFallback: enableServicePortFallback, + cachedUserTags: cachedUserTags, + uriConverter: uriConverter, + ); + final dtdInfo = dds.hostedDartToolingDaemon; + stderr.write(json.encode({ + 'state': 'started', + 'ddsUri': dds.uri.toString(), + if (dds.devToolsUri != null) 'devToolsUri': dds.devToolsUri.toString(), + if (dtdInfo != null) + 'dtd': { + 'uri': dtdInfo.uri, + }, + })); + stderr.close(); + } catch (e, st) { + writeErrorResponse(e, st); + } finally { + // Always close stderr to notify tooling that DDS has finished writing + // launch details. + await stderr.close(); + } +} + +void writeErrorResponse(Object e, StackTrace st) { + stderr.write(json.encode({ + 'state': 'error', + 'error': '$e', + 'stacktrace': '$st', + if (e is DartDevelopmentServiceException) 'ddsExceptionDetails': e.toJson(), + })); +} diff --git a/pkg/dds/test/uri_converter_test.dart b/pkg/dds/test/uri_converter_test.dart index 656982f51ee..588655cfa20 100644 --- a/pkg/dds/test/uri_converter_test.dart +++ b/pkg/dds/test/uri_converter_test.dart @@ -6,7 +6,6 @@ import 'dart:async'; import 'dart:io'; import 'package:dds/dds.dart'; -import 'package:dds/src/bazel_uri_converter.dart'; import 'package:test/test.dart'; import 'package:vm_service/vm_service.dart'; import 'package:vm_service/vm_service_io.dart'; @@ -77,54 +76,4 @@ void main() { }, timeout: Timeout.none, ); - - test( - 'DDS can handle basic google3:// paths', - () async { - Uri serviceUri = remoteVmServiceUri; - dds = await DartDevelopmentService.startDartDevelopmentService( - remoteVmServiceUri, - uriConverter: BazelUriConverter('/workspace/root').uriToPath, - ); - serviceUri = dds!.wsUri!; - expect(dds!.isRunning, true); - final service = await vmServiceConnectUri(serviceUri.toString()); - - IsolateRef isolate; - while (true) { - final vm = await service.getVM(); - if (vm.isolates!.isNotEmpty) { - isolate = vm.isolates!.first; - try { - isolate = await service.getIsolate(isolate.id!); - if ((isolate as Isolate).runnable!) { - break; - } - } on SentinelException { - // ignore - } - } - await Future.delayed(const Duration(seconds: 1)); - } - expect(isolate, isNotNull); - - final unresolvedUris = [ - // TODO(b/261234406): Handle `dart:` URIs. - 'dart:io', // dart:io -> file:///workspace/root/io - 'google3://workspace_name/my/script.dart', // google3://workspace_name/my/script.dart -> file:///workspace/root/my/script.dart - 'package:foo/foo.dart', // package:foo/foo.dart -> file:///workspace/root/third_party/dart/foo/lib/foo.dart - ]; - final result = await service.lookupResolvedPackageUris( - isolate.id!, - unresolvedUris, - local: true, - ); - - expect(result.uris?[0], 'file:///workspace/root/io'); - expect(result.uris?[1], 'file:///workspace/root/my/script.dart'); - expect(result.uris?[2], - 'file:///workspace/root/third_party/dart/foo/lib/foo.dart'); - }, - timeout: Timeout.none, - ); }