Library specification format: replace 'environment_overrides' with 'supported'.

Change-Id: Ic8172f4574a35ac31910ac0d5934c0d63db40a23
Reviewed-on: https://dart-review.googlesource.com/48060
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
This commit is contained in:
Sigmund Cherem 2018-03-23 18:28:59 +00:00 committed by commit-bot@chromium.org
parent 5be7955bb5
commit 34bb60bbc5
7 changed files with 93 additions and 113 deletions

View file

@ -32,11 +32,9 @@
/// }
/// "mirrors": {
/// "uri": "mirrors/mirrors.dart",
/// "supported": false
/// }
/// }
/// "environment_overrides": {
/// "mirrors": false
/// }
/// }
/// }
///
@ -44,8 +42,8 @@
/// - a top level entry for each target. Keys are target names (e.g. "vm"
/// above), and values contain the entire specification of a target.
///
/// - each target specification is a map. Today two keys are supported:
/// "libraries" and "environment_overrides".
/// - each target specification is a map. Today, only one key is supported on
/// this map: "libraries".
///
/// - The "libraries" entry contains details for how each platform library is
/// implemented. The entry is a map, where keys are the name of the platform
@ -63,20 +61,21 @@
/// which will be resolved relative to the location of the library
/// specification file.
///
/// - The "environment_overrides" entry contains rules to override the value
/// of environment variables that are derived from the platform libraries.
/// - The "supported" entry on the library information is optional. The value
/// is a boolean indicating whether the library is supported in the
/// underlying target. However, since the libraries are assumed to be
/// supported by default, we only expect users to use `false`.
///
/// By default every platform library that is available in the "libraries"
/// section implicitly defines an environment variable `dart.library.name`
/// as `"true"`, to indicate that the library is supported. Some backends
/// override this to allow imports to a platform library, but still report
/// that the library is not supported in conditional imports and const
/// `fromEnvironment` expressions.
/// The purpose of this value is to configure conditional imports and
/// environment constants. By default every platform library that is
/// available in the "libraries" section implicitly defines an environment
/// variable `dart.library.name` as `"true"`, to indicate that the library
/// is supported. Some backends allow imports to an unsupported platform
/// library (turning a static error into a runtime error when the library is
/// eventually accessed). These backends can use `supported: false` to
/// report that such library is still not supported in conditional imports
/// and const `fromEnvironment` expressions.
///
/// A name key listed in the "environment_overrides" section must match the
/// name of a library in the "libraries" section. The value is a bool,
/// however, since the libraries are assumed to be supported by default, we
/// only expect users to use `false`.
///
/// Note: we currently have several different files that need to be updated
/// when changing libraries, sources, and patch files:
@ -88,9 +87,6 @@
/// https://github.com/dart-lang/sdk/issues/28836), but for now we need to pay
/// close attention to change them consistently.
// TODO(sigmund): consider moving the overrides directly into the libraries
// section (e.g. add a "supported: false" entry).
// TODO(sigmund): move this file to a shared package.
import 'dart:convert' show JSON;
@ -112,7 +108,7 @@ class LibrariesSpecification {
var targetSpec = _targets[target];
if (targetSpec == null) {
throw new LibrariesSpecificationException(
'No library specification for target "$target"');
'No library specification for target "$target"');
}
return targetSpec;
}
@ -179,30 +175,17 @@ class LibrariesSpecification {
return _reportError(
"patches entry for '$name' is not a list or a string");
}
libraries[name] = new LibraryInfo(name, uri, patches);
});
Map<String, bool> environmentOverrides = <String, bool>{};
if (targetData.containsKey("environment_overrides")) {
var overridesData = targetData["environment_overrides"];
if (overridesData is! Map) {
return _reportError(
"environment_overrides entry for '$targetName' is not a map");
var supported = data['supported'] ?? true;
if (supported is! bool) {
return _reportError("\"supported\" entry: expected a 'bool' but "
"got a '${supported.runtimeType}' ('$supported')");
}
overridesData.forEach((String name, value) {
if (!libraries.containsKey(name)) {
return _reportError(
"entry '$name' does not correspond to an existing library "
"in '$targetName'");
}
if (value is bool) {
environmentOverrides[name] = value;
} else {
return _reportError("entry '$name' is not a bool");
}
});
}
targets[targetName] = new TargetLibrariesSpecification(
targetName, libraries, environmentOverrides);
libraries[name] =
new LibraryInfo(name, uri, patches, isSupported: supported);
});
targets[targetName] =
new TargetLibrariesSpecification(targetName, libraries);
});
return new LibrariesSpecification(targets);
}
@ -226,12 +209,11 @@ class LibrariesSpecification {
'uri': pathFor(lib.uri),
'patches': lib.patches.map(pathFor).toList(),
};
if (!lib.isSupported) {
libraries[name]['supported'] = false;
}
});
result[targetName] = {'libraries': libraries};
if (target._environmentOverrides.isNotEmpty) {
result[targetName]['environment_overrides'] =
target._environmentOverrides;
}
});
return result;
}
@ -244,21 +226,11 @@ class TargetLibrariesSpecification {
final Map<String, LibraryInfo> _libraries;
final Map<String, bool> _environmentOverrides;
const TargetLibrariesSpecification(this.targetName,
[this._libraries = const <String, LibraryInfo>{},
this._environmentOverrides = const <String, bool>{}]);
[this._libraries = const <String, LibraryInfo>{}]);
/// Details about a library whose import is `dart:$name`.
LibraryInfo libraryInfoFor(String name) => _libraries[name];
/// Environment override for a library whose import is `dart:$name`. The value
/// can be "true", "false", or null if no override was given.
String environmentOverrideFor(String name) {
var override = _environmentOverrides[name];
return override == null ? null : "$override";
}
}
/// Information about a `dart:` library in a specific target platform.
@ -273,7 +245,12 @@ class LibraryInfo {
/// Patch files used for this library in the target platform, if any.
final List<Uri> patches;
const LibraryInfo(this.name, this.uri, this.patches);
/// Whether the library is supported and thus `dart.library.name` is "true"
/// for conditional imports and fromEnvironment constants.
final bool isSupported;
const LibraryInfo(this.name, this.uri, this.patches,
{this.isSupported: true});
}
class LibrariesSpecificationException {

View file

@ -244,9 +244,7 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
const String prefix = "dart.library.";
if (!dottedName.startsWith(prefix)) return "";
dottedName = dottedName.substring(prefix.length);
String override =
loader.target.uriTranslator.environmentOverrideFor(dottedName);
if (override != null) return override;
if (!loader.target.uriTranslator.isLibrarySupported(dottedName)) return "";
LibraryBuilder imported =
loader.builders[new Uri(scheme: "dart", path: dottedName)];

View file

@ -29,7 +29,6 @@ abstract class UriTranslator {
/// [CompilerContext.current].
Uri translate(Uri uri, [bool reportMessage = true]);
/// Return the value of `dart.library.name`, if it was overriden in the
/// library specification file, otherwise return null.
String environmentOverrideFor(String libraryName);
/// Whether the given [libraryName] is supported by the underlying target.
bool isLibrarySupported(String libraryName);
}

View file

@ -46,8 +46,11 @@ class UriTranslatorImpl implements UriTranslator {
}
@override
String environmentOverrideFor(String libraryName) =>
dartLibraries.environmentOverrideFor(libraryName);
bool isLibrarySupported(String libraryName) {
// TODO(sigmund): change this to `?? false` when all backends provide the
// `libraries.json` file by default (Issue #32657).
return dartLibraries.libraryInfoFor(libraryName)?.isSupported ?? true;
}
/// Return the file URI that corresponds to the given `dart` URI, or `null`
/// if there is no corresponding Dart library registered.

View file

@ -198,41 +198,54 @@ main() {
Uri.parse('org-dartlang-test:///one/two/c/main.dart'));
});
test('environment_overrides entry must be a map', () async {
var jsonString = '{"vm" : {"libraries": {"core": {"uri": "main.dart"}},'
'"environment_overrides": []}}';
test('supported entry must be bool', () async {
var jsonString = '{"vm" : {"libraries": {"core": '
'{"uri": "main.dart", "supported": 3}},';
expect(
() => LibrariesSpecification.parse(
Uri.parse('org-dartlang-test:///f.json'), jsonString),
throwsA((e) => e is LibrariesSpecificationException));
});
test('environment_overrides values must be bool', () async {
var jsonString = '{"vm" : {"libraries": {"core": {"uri": "main.dart"}},'
'"environment_overrides": {"core": 3}}}';
expect(
() => LibrariesSpecification.parse(
Uri.parse('org-dartlang-test:///f.json'), jsonString),
throwsA((e) => e is LibrariesSpecificationException));
});
test('supported entry is copied correctly when parsing', () async {
var jsonString = '''
{
"vm": {
"libraries": {
"foo" : {
"uri": "a/main.dart",
"patches": [
"a/p1.dart",
"a/p2.dart"
],
"supported": false
test('environment_overrides correspond to existing libraries', () async {
var jsonString = '{"vm" : {"libraries": {"core": {"uri": "main.dart"}},'
'"environment_overrides": {"ui": false}}}';
expect(
() => LibrariesSpecification.parse(
Uri.parse('org-dartlang-test:///f.json'), jsonString),
throwsA((e) => e is LibrariesSpecificationException));
});
test('environment_overrides can be read from the public API', () async {
var jsonString = '{"vm" : {"libraries": {"core": {"uri": "main.dart"}},'
'"environment_overrides": {"core": false}}}';
},
"bar" : {
"uri": "b/main.dart",
"patches": [
"b/p3.dart"
],
"supported": true
},
"baz" : {
"uri": "b/main.dart",
"patches": [
"b/p3.dart"
]
}
}
}
}
''';
var spec = LibrariesSpecification.parse(
Uri.parse('org-dartlang-test:///one/two/f.json'), jsonString);
expect(
spec.specificationFor('vm').environmentOverrideFor('core'), "false");
expect(spec.specificationFor('vm').environmentOverrideFor('ui'), null);
spec.specificationFor('vm').libraryInfoFor('foo').isSupported, false);
expect(
spec.specificationFor('vm').libraryInfoFor('bar').isSupported, true);
expect(
spec.specificationFor('vm').libraryInfoFor('baz').isSupported, true);
});
});
@ -247,7 +260,8 @@ main() {
"patches": [
"a/p1.dart",
"a/p2.dart"
]
],
"supported": false
},
"bar" : {
"uri": "b/main.dart",
@ -263,9 +277,6 @@ main() {
"uri": "c/main.dart",
"patches": []
}
},
"environment_overrides": {
"c" : false
}
}
}

View file

@ -146,10 +146,6 @@
}
},
"dart2js": {
"environment_overrides": {
"mirrors": false,
"io": false
},
"libraries": {
"async": {
"patches": "_internal/js_runtime/lib/async_patch.dart",
@ -160,6 +156,7 @@
},
"mirrors": {
"patches": "_internal/js_runtime/lib/mirrors_patch_cfe.dart",
"supported": false,
"uri": "mirrors/mirrors.dart"
},
"_js_embedded_names": {
@ -167,6 +164,7 @@
},
"io": {
"patches": "_internal/js_runtime/lib/io_patch.dart",
"supported": false,
"uri": "io/io.dart"
},
"_internal": {
@ -268,10 +266,6 @@
}
},
"dart2js_server": {
"environment_overrides": {
"mirrors": false,
"io": false
},
"libraries": {
"async": {
"patches": "_internal/js_runtime/lib/async_patch.dart",
@ -279,6 +273,7 @@
},
"mirrors": {
"patches": "_internal/js_runtime/lib/mirrors_patch_cfe.dart",
"supported": false,
"uri": "mirrors/mirrors.dart"
},
"_interceptors": {
@ -289,6 +284,7 @@
},
"io": {
"patches": "_internal/js_runtime/lib/io_patch.dart",
"supported": false,
"uri": "io/io.dart"
},
"_internal": {

View file

@ -184,6 +184,7 @@ dart2js:
io:
uri: "io/io.dart"
patches: "_internal/js_runtime/lib/io_patch.dart"
supported: false
isolate:
uri: "isolate/isolate.dart"
@ -206,6 +207,7 @@ dart2js:
mirrors:
uri: "mirrors/mirrors.dart"
patches: "_internal/js_runtime/lib/mirrors_patch_cfe.dart"
supported: false
typed_data:
uri: "typed_data/typed_data.dart"
@ -260,10 +262,6 @@ dart2js:
_metadata:
uri: "html/html_common/metadata.dart"
environment_overrides:
io: false
mirrors: false
dart2js_server:
libraries:
async:
@ -292,6 +290,7 @@ dart2js_server:
io:
uri: "io/io.dart"
patches: "_internal/js_runtime/lib/io_patch.dart"
supported: false
isolate:
uri: "isolate/isolate.dart"
@ -314,6 +313,7 @@ dart2js_server:
mirrors:
uri: "mirrors/mirrors.dart"
patches: "_internal/js_runtime/lib/mirrors_patch_cfe.dart"
supported: false
typed_data:
uri: "typed_data/typed_data.dart"
@ -352,7 +352,3 @@ dart2js_server:
_async_await_error_codes:
uri: "_internal/js_runtime/lib/shared/async_await_error_codes.dart"
environment_overrides:
io: false
mirrors: false