From c7ea09e2f63db322e0a7b688f48e258a166b6058 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Thu, 26 Sep 2019 13:19:06 -0700 Subject: [PATCH 1/7] Account for server root --- dwds/CHANGELOG.md | 1 + dwds/lib/src/utilities/dart_uri.dart | 22 +++++++++++++++------- dwds/test/dart_uri_test.dart | 6 ++++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index a96444e58..e1373538c 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -4,6 +4,7 @@ `assetServerPort` and `applicationTarget`. - Expose a `BuildRunnerAssetHandler` which proxies request to the asset server running within build runner. +- Account for root directory path when using `package:` URIs with `DartUri`. - Support the Legacy Module strategy through the injected client. - Support DDK sourcemap URIs. - Update SDK dependency to minimum of 2.5.0. diff --git a/dwds/lib/src/utilities/dart_uri.dart b/dwds/lib/src/utilities/dart_uri.dart index 0e5960b8e..28a245f5d 100644 --- a/dwds/lib/src/utilities/dart_uri.dart +++ b/dwds/lib/src/utilities/dart_uri.dart @@ -4,8 +4,8 @@ import 'dart:io'; -import 'package:path/path.dart' as p; import 'package:package_resolver/package_resolver.dart'; +import 'package:path/path.dart' as p; /// The URI for a particular Dart file, able to canonicalize from various /// different representations. @@ -98,7 +98,9 @@ class DartUri { /// JS script. The dirname of that path should give us the missing prefix. factory DartUri(String uri, [String serverUri]) { // TODO(401): Remove serverUri after D24 is stable. - if (uri.startsWith('package:')) return DartUri._fromPackageUri(uri); + if (uri.startsWith('package:')) { + return DartUri._fromPackageUri(uri, serverUri); + } if (uri.startsWith('org-dartlang-app:')) return DartUri._fromAppUri(uri); if (uri.startsWith('google3:')) return DartUri._fromGoogleUri(uri); if (uri.startsWith('file:')) return DartUri._fromFileUri(uri); @@ -109,16 +111,22 @@ class DartUri { } // Work around short paths if we have been provided the context. if (serverUri != null) { - var path = Uri.parse(serverUri).path; - var dir = p.dirname(path); - return DartUri._fromServerPath(p.normalize(p.join(dir, uri))); + return DartUri._fromServerPath( + p.normalize(p.join(_dirForServerUri(serverUri), uri))); } throw FormatException('Unsupported URI form', uri); } + static String _dirForServerUri(String uri) => p.dirname(Uri.parse(uri).path); + /// Construct from a package: URI - factory DartUri._fromPackageUri(String uri) { - return DartUri._('packages/${uri.substring("package:".length)}'); + factory DartUri._fromPackageUri(String uri, String serverUri) { + var packagePath = 'packages/${uri.substring("package:".length)}'; + if (serverUri != null) { + return DartUri._fromServerPath( + p.normalize(p.join(_dirForServerUri(serverUri), packagePath))); + } + return DartUri._(packagePath); } /// Construct from a file: URI diff --git a/dwds/test/dart_uri_test.dart b/dwds/test/dart_uri_test.dart index bb4c710e2..a2cecb3d4 100644 --- a/dwds/test/dart_uri_test.dart +++ b/dwds/test/dart_uri_test.dart @@ -17,6 +17,12 @@ void main() { expect(uri.serverPath, 'packages/path/path.dart'); }); + test('parses package : paths with root', () { + var uri = DartUri( + 'package:path/path.dart', 'http://localhost:8080/foo/bar/blah'); + expect(uri.serverPath, 'foo/bar/packages/path/path.dart'); + }); + test('parses org-dartlang-app paths', () { var uri = DartUri('org-dartlang-app:////blah/main.dart'); expect(uri.serverPath, 'blah/main.dart'); From c576c9189942347ffe3fffc42ab24d2003a3fb28 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 7 Oct 2019 13:31:05 -0700 Subject: [PATCH 2/7] respond to review --- dwds/CHANGELOG.md | 4 +++- dwds/lib/src/utilities/dart_uri.dart | 21 +++++++++++++++------ dwds/pubspec.yaml | 2 +- webdev/CHANGELOG.md | 4 ++++ webdev/lib/src/version.dart | 2 +- webdev/pubspec.yaml | 6 +++++- 6 files changed, 29 insertions(+), 10 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index cfabb2186..4b9a71628 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,3 +1,6 @@ +## 0.7.2 +- Account for root directory path when using `package:` URIs with `DartUri`. + ## 0.7.1 - Fix a bug where we would try to create a new isolate even for a failed @@ -15,7 +18,6 @@ `assetServerPort` and `applicationTarget`. - Expose a `BuildRunnerAssetHandler` which proxies request to the asset server running within build runner. -- Account for root directory path when using `package:` URIs with `DartUri`. - Support the Legacy Module strategy through the injected client. - Support DDK sourcemap URIs. - Update SDK dependency to minimum of 2.5.0. diff --git a/dwds/lib/src/utilities/dart_uri.dart b/dwds/lib/src/utilities/dart_uri.dart index 28a245f5d..91d968614 100644 --- a/dwds/lib/src/utilities/dart_uri.dart +++ b/dwds/lib/src/utilities/dart_uri.dart @@ -97,11 +97,12 @@ class DartUri { /// packages/path/src/path.dart. The optional [serverUri] is the full URI of the /// JS script. The dirname of that path should give us the missing prefix. factory DartUri(String uri, [String serverUri]) { - // TODO(401): Remove serverUri after D24 is stable. if (uri.startsWith('package:')) { return DartUri._fromPackageUri(uri, serverUri); } - if (uri.startsWith('org-dartlang-app:')) return DartUri._fromAppUri(uri); + if (uri.startsWith('org-dartlang-app:')) { + return DartUri._fromAppUri(uri, serverUri); + } if (uri.startsWith('google3:')) return DartUri._fromGoogleUri(uri); if (uri.startsWith('file:')) return DartUri._fromFileUri(uri); if (uri.startsWith('/packages/')) return DartUri._fromServerPath(uri); @@ -109,6 +110,7 @@ class DartUri { if (uri.startsWith('http:') || uri.startsWith('https:')) { return DartUri(Uri.parse(uri).path); } + // Work around short paths if we have been provided the context. if (serverUri != null) { return DartUri._fromServerPath( @@ -117,14 +119,17 @@ class DartUri { throw FormatException('Unsupported URI form', uri); } - static String _dirForServerUri(String uri) => p.dirname(Uri.parse(uri).path); + /// Returns the dirname for the server URI. + static String _dirForServerUri(String uri) => + // Path will be of the form `/foo/bar` so ignore the leading `/`. + p.dirname(Uri.parse(uri).path.substring(1)); /// Construct from a package: URI factory DartUri._fromPackageUri(String uri, String serverUri) { var packagePath = 'packages/${uri.substring("package:".length)}'; if (serverUri != null) { return DartUri._fromServerPath( - p.normalize(p.join(_dirForServerUri(serverUri), packagePath))); + p.join(_dirForServerUri(serverUri), packagePath)); } return DartUri._(packagePath); } @@ -143,12 +148,16 @@ class DartUri { } /// Construct from an org-dartlang-app: URI. - factory DartUri._fromAppUri(String uri) { + factory DartUri._fromAppUri(String uri, String serverUri) { // We ignore the first segment of the path, which is the root // from which we're serving. + var path = Uri.parse(uri).pathSegments.skip(1).join('/').toString(); // TODO: To be able to convert to an org-dartlang-app: URI we will // need to know the root - possibly keep it as a static? - return DartUri._(Uri.parse(uri).pathSegments.skip(1).join('/').toString()); + if (serverUri != null) { + return DartUri._(p.normalize(p.join(_dirForServerUri(serverUri), path))); + } + return DartUri._(path); } DartUri._(this.serverPath); diff --git a/dwds/pubspec.yaml b/dwds/pubspec.yaml index c413a368f..be4aead0c 100644 --- a/dwds/pubspec.yaml +++ b/dwds/pubspec.yaml @@ -1,5 +1,5 @@ name: dwds -version: 0.7.1 +version: 0.7.2 author: Dart Team homepage: https://github.com/dart-lang/webdev/tree/master/dwds description: >- diff --git a/webdev/CHANGELOG.md b/webdev/CHANGELOG.md index 61ee9fa25..1b9db2ee5 100644 --- a/webdev/CHANGELOG.md +++ b/webdev/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.5.3-dev + +- Depend on the latest `package:dwds`. + ## 2.5.2 - Update SDK dependency to minimum of 2.5.0. diff --git a/webdev/lib/src/version.dart b/webdev/lib/src/version.dart index 22ff88f96..203065220 100644 --- a/webdev/lib/src/version.dart +++ b/webdev/lib/src/version.dart @@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '2.5.2'; +const packageVersion = '2.5.3-dev'; diff --git a/webdev/pubspec.yaml b/webdev/pubspec.yaml index b84239621..9465e7117 100644 --- a/webdev/pubspec.yaml +++ b/webdev/pubspec.yaml @@ -1,6 +1,6 @@ name: webdev # Every time this changes you need to run `pub run build_runner build`. -version: 2.5.2 +version: 2.5.3-dev author: Dart Team homepage: https://github.com/dart-lang/webdev description: >- @@ -46,3 +46,7 @@ dev_dependencies: executables: webdev: + +dependency_overrides: + dwds: + path: ../dwds From 4b5ea0101b4d72a463971b173daac094ba1c2037 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 7 Oct 2019 15:01:21 -0700 Subject: [PATCH 3/7] make source map path clear --- dwds/lib/src/utilities/dart_uri.dart | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dwds/lib/src/utilities/dart_uri.dart b/dwds/lib/src/utilities/dart_uri.dart index 91d968614..0330f9c62 100644 --- a/dwds/lib/src/utilities/dart_uri.dart +++ b/dwds/lib/src/utilities/dart_uri.dart @@ -110,15 +110,17 @@ class DartUri { if (uri.startsWith('http:') || uri.startsWith('https:')) { return DartUri(Uri.parse(uri).path); } + if (serverUri != null) return DartUri._fromSourceMapUri(uri, serverUri); - // Work around short paths if we have been provided the context. - if (serverUri != null) { - return DartUri._fromServerPath( - p.normalize(p.join(_dirForServerUri(serverUri), uri))); - } throw FormatException('Unsupported URI form', uri); } + /// Construct from a sourceMap URI. + factory DartUri._fromSourceMapUri(String uri, String serverUri) { + return DartUri._fromServerPath( + p.normalize(p.join(_dirForServerUri(serverUri), uri))); + } + /// Returns the dirname for the server URI. static String _dirForServerUri(String uri) => // Path will be of the form `/foo/bar` so ignore the leading `/`. From 1b106302a33d7c9e7d3ab979a41d65c1b13ae955 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 7 Oct 2019 15:02:36 -0700 Subject: [PATCH 4/7] optional arg --- dwds/CHANGELOG.md | 1 + dwds/lib/src/utilities/dart_uri.dart | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 4b9a71628..d29d6540d 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,4 +1,5 @@ ## 0.7.2 + - Account for root directory path when using `package:` URIs with `DartUri`. ## 0.7.1 diff --git a/dwds/lib/src/utilities/dart_uri.dart b/dwds/lib/src/utilities/dart_uri.dart index 0330f9c62..958968923 100644 --- a/dwds/lib/src/utilities/dart_uri.dart +++ b/dwds/lib/src/utilities/dart_uri.dart @@ -98,10 +98,10 @@ class DartUri { /// JS script. The dirname of that path should give us the missing prefix. factory DartUri(String uri, [String serverUri]) { if (uri.startsWith('package:')) { - return DartUri._fromPackageUri(uri, serverUri); + return DartUri._fromPackageUri(uri, serverUri: serverUri); } if (uri.startsWith('org-dartlang-app:')) { - return DartUri._fromAppUri(uri, serverUri); + return DartUri._fromAppUri(uri, serverUri: serverUri); } if (uri.startsWith('google3:')) return DartUri._fromGoogleUri(uri); if (uri.startsWith('file:')) return DartUri._fromFileUri(uri); @@ -127,7 +127,7 @@ class DartUri { p.dirname(Uri.parse(uri).path.substring(1)); /// Construct from a package: URI - factory DartUri._fromPackageUri(String uri, String serverUri) { + factory DartUri._fromPackageUri(String uri, {String serverUri}) { var packagePath = 'packages/${uri.substring("package:".length)}'; if (serverUri != null) { return DartUri._fromServerPath( @@ -150,7 +150,7 @@ class DartUri { } /// Construct from an org-dartlang-app: URI. - factory DartUri._fromAppUri(String uri, String serverUri) { + factory DartUri._fromAppUri(String uri, {String serverUri}) { // We ignore the first segment of the path, which is the root // from which we're serving. var path = Uri.parse(uri).pathSegments.skip(1).join('/').toString(); From 3ee0991258a830bb0524421b89711f88959a3d9f Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 7 Oct 2019 16:10:18 -0700 Subject: [PATCH 5/7] fix tests and account for source maps --- dwds/lib/src/debugging/debugger.dart | 2 +- dwds/lib/src/debugging/sources.dart | 9 +++++++-- dwds/lib/src/utilities/dart_uri.dart | 26 ++++++++++---------------- dwds/test/dart_uri_test.dart | 5 ----- dwds/test/debugging/sources_test.dart | 2 +- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index f4c5a7a9c..d17f1272f 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -169,7 +169,7 @@ class Debugger extends Domain { } Future _initialize() async { - sources = Sources(_assetHandler, _remoteDebugger, _logWriter); + sources = Sources(_assetHandler, _remoteDebugger, _logWriter, _root); // We must add a listener before enabling the debugger otherwise we will // miss events. // Allow a null debugger/connection for unit tests. diff --git a/dwds/lib/src/debugging/sources.dart b/dwds/lib/src/debugging/sources.dart index 8c6fa84ba..d3f643763 100644 --- a/dwds/lib/src/debugging/sources.dart +++ b/dwds/lib/src/debugging/sources.dart @@ -47,7 +47,10 @@ class Sources { final RemoteDebugger _remoteDebugger; - Sources(this._assetHandler, this._remoteDebugger, this._logWriter); + final String _root; + + Sources( + this._assetHandler, this._remoteDebugger, this._logWriter, this._root); /// Returns all [Location] data for a provided Dart source. Set locationsForDart(String serverPath) => @@ -77,7 +80,9 @@ class Sources { for (var entry in lineEntry.entries) { var index = entry.sourceUrlId; if (index == null) continue; - var dartUri = DartUri(mapping.urls[index], script.url); + var path = p.join( + p.dirname(Uri.parse(script.url).path), mapping.urls[index]); + var dartUri = DartUri(path, _root); var location = Location.from( script.scriptId, lineEntry, diff --git a/dwds/lib/src/utilities/dart_uri.dart b/dwds/lib/src/utilities/dart_uri.dart index 958968923..ece66d9ca 100644 --- a/dwds/lib/src/utilities/dart_uri.dart +++ b/dwds/lib/src/utilities/dart_uri.dart @@ -100,27 +100,20 @@ class DartUri { if (uri.startsWith('package:')) { return DartUri._fromPackageUri(uri, serverUri: serverUri); } - if (uri.startsWith('org-dartlang-app:')) { - return DartUri._fromAppUri(uri, serverUri: serverUri); - } + if (uri.startsWith('org-dartlang-app:')) return DartUri._fromAppUri(uri); if (uri.startsWith('google3:')) return DartUri._fromGoogleUri(uri); if (uri.startsWith('file:')) return DartUri._fromFileUri(uri); - if (uri.startsWith('/packages/')) return DartUri._fromServerPath(uri); + if (uri.startsWith('/packages/')) { + return DartUri._fromServerPath(uri, serverUri: serverUri); + } if (uri.startsWith('/')) return DartUri._fromServerPath(uri); if (uri.startsWith('http:') || uri.startsWith('https:')) { return DartUri(Uri.parse(uri).path); } - if (serverUri != null) return DartUri._fromSourceMapUri(uri, serverUri); throw FormatException('Unsupported URI form', uri); } - /// Construct from a sourceMap URI. - factory DartUri._fromSourceMapUri(String uri, String serverUri) { - return DartUri._fromServerPath( - p.normalize(p.join(_dirForServerUri(serverUri), uri))); - } - /// Returns the dirname for the server URI. static String _dirForServerUri(String uri) => // Path will be of the form `/foo/bar` so ignore the leading `/`. @@ -150,24 +143,25 @@ class DartUri { } /// Construct from an org-dartlang-app: URI. - factory DartUri._fromAppUri(String uri, {String serverUri}) { + factory DartUri._fromAppUri(String uri) { // We ignore the first segment of the path, which is the root // from which we're serving. var path = Uri.parse(uri).pathSegments.skip(1).join('/').toString(); // TODO: To be able to convert to an org-dartlang-app: URI we will // need to know the root - possibly keep it as a static? - if (serverUri != null) { - return DartUri._(p.normalize(p.join(_dirForServerUri(serverUri), path))); - } return DartUri._(path); } DartUri._(this.serverPath); /// Construct from a path, relative to the directory being served. - factory DartUri._fromServerPath(String uri) { + factory DartUri._fromServerPath(String uri, {String serverUri}) { uri = uri[0] == '.' ? uri.substring(1) : uri; uri = uri[0] == '/' ? uri.substring(1) : uri; + + if (serverUri != null) { + return DartUri._fromServerPath(p.join(_dirForServerUri(serverUri), uri)); + } return DartUri._(uri); } } diff --git a/dwds/test/dart_uri_test.dart b/dwds/test/dart_uri_test.dart index a2cecb3d4..9a397e255 100644 --- a/dwds/test/dart_uri_test.dart +++ b/dwds/test/dart_uri_test.dart @@ -7,11 +7,6 @@ import 'package:test/test.dart'; void main() { group('DartUri', () { - test('normalizes server paths', () { - var uri = DartUri('../foo.dart', '/packages/blah/src/blah.dart'); - expect(uri.serverPath, 'packages/blah/foo.dart'); - }); - test('parses package : paths', () { var uri = DartUri('package:path/path.dart'); expect(uri.serverPath, 'packages/path/path.dart'); diff --git a/dwds/test/debugging/sources_test.dart b/dwds/test/debugging/sources_test.dart index 851dbfb48..41806c7a6 100644 --- a/dwds/test/debugging/sources_test.dart +++ b/dwds/test/debugging/sources_test.dart @@ -18,7 +18,7 @@ void main() { }; var logs = []; var sources = Sources(TestingAssetHandler(assets), null, - (level, message) => logs.add(LogRecord(level, message, ''))); + (level, message) => logs.add(LogRecord(level, message, '')), ''); var serverUri = 'http://localhost:1234/'; await sources.scriptParsed(ScriptParsedEvent(WipEvent({ 'params': { From e4412aa650d6fe9f509d81c319ee932f72140dcd Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 7 Oct 2019 16:52:13 -0700 Subject: [PATCH 6/7] don't worry about leading slash --- dwds/lib/src/utilities/dart_uri.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dwds/lib/src/utilities/dart_uri.dart b/dwds/lib/src/utilities/dart_uri.dart index ece66d9ca..3dbe8e380 100644 --- a/dwds/lib/src/utilities/dart_uri.dart +++ b/dwds/lib/src/utilities/dart_uri.dart @@ -115,9 +115,7 @@ class DartUri { } /// Returns the dirname for the server URI. - static String _dirForServerUri(String uri) => - // Path will be of the form `/foo/bar` so ignore the leading `/`. - p.dirname(Uri.parse(uri).path.substring(1)); + static String _dirForServerUri(String uri) => p.dirname(Uri.parse(uri).path); /// Construct from a package: URI factory DartUri._fromPackageUri(String uri, {String serverUri}) { From 6fc70d58f81efda9952ab28b08c33ae782caab0e Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 7 Oct 2019 17:10:41 -0700 Subject: [PATCH 7/7] clean up --- dwds/lib/src/debugging/sources.dart | 2 ++ dwds/lib/src/utilities/dart_uri.dart | 13 ++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dwds/lib/src/debugging/sources.dart b/dwds/lib/src/debugging/sources.dart index d3f643763..f2204e76c 100644 --- a/dwds/lib/src/debugging/sources.dart +++ b/dwds/lib/src/debugging/sources.dart @@ -80,6 +80,8 @@ class Sources { for (var entry in lineEntry.entries) { var index = entry.sourceUrlId; if (index == null) continue; + // TODO(grouma) - This work seems expensive and likely should be + // cached. var path = p.join( p.dirname(Uri.parse(script.url).path), mapping.urls[index]); var dartUri = DartUri(path, _root); diff --git a/dwds/lib/src/utilities/dart_uri.dart b/dwds/lib/src/utilities/dart_uri.dart index 3dbe8e380..575e0eae8 100644 --- a/dwds/lib/src/utilities/dart_uri.dart +++ b/dwds/lib/src/utilities/dart_uri.dart @@ -104,9 +104,9 @@ class DartUri { if (uri.startsWith('google3:')) return DartUri._fromGoogleUri(uri); if (uri.startsWith('file:')) return DartUri._fromFileUri(uri); if (uri.startsWith('/packages/')) { - return DartUri._fromServerPath(uri, serverUri: serverUri); + return DartUri._fromRelativePath(uri, serverUri: serverUri); } - if (uri.startsWith('/')) return DartUri._fromServerPath(uri); + if (uri.startsWith('/')) return DartUri._fromRelativePath(uri); if (uri.startsWith('http:') || uri.startsWith('https:')) { return DartUri(Uri.parse(uri).path); } @@ -121,7 +121,7 @@ class DartUri { factory DartUri._fromPackageUri(String uri, {String serverUri}) { var packagePath = 'packages/${uri.substring("package:".length)}'; if (serverUri != null) { - return DartUri._fromServerPath( + return DartUri._fromRelativePath( p.join(_dirForServerUri(serverUri), packagePath)); } return DartUri._(packagePath); @@ -145,20 +145,19 @@ class DartUri { // We ignore the first segment of the path, which is the root // from which we're serving. var path = Uri.parse(uri).pathSegments.skip(1).join('/').toString(); - // TODO: To be able to convert to an org-dartlang-app: URI we will - // need to know the root - possibly keep it as a static? return DartUri._(path); } DartUri._(this.serverPath); /// Construct from a path, relative to the directory being served. - factory DartUri._fromServerPath(String uri, {String serverUri}) { + factory DartUri._fromRelativePath(String uri, {String serverUri}) { uri = uri[0] == '.' ? uri.substring(1) : uri; uri = uri[0] == '/' ? uri.substring(1) : uri; if (serverUri != null) { - return DartUri._fromServerPath(p.join(_dirForServerUri(serverUri), uri)); + return DartUri._fromRelativePath( + p.join(_dirForServerUri(serverUri), uri)); } return DartUri._(uri); }