Introduce "platform configurations" to replace categories and libraries.dart.

Introduce "platform configurations" to replace categories and libraries.dart.

A small writeup of the thoughts behind this concept can be found at:
https://docs.google.com/a/google.com/document/d/1WkqJVPphuThH8h2jDqBOb6h1iMkrkQIO7PF638-qlso/edit?usp=sharing

BUG=
R=floitsch@google.com, johnniwinther@google.com, whesse@google.com

Committed: https://github.com/dart-lang/sdk/commit/c0d5f73bd02506e697e507e172dc4effe07f0787

[sigurdm, 2015-10-29 15:27:18]

Patchset #1 (id:1) has been deleted

[sigurdm, 2015-10-29 15:27:31]

sigurdm@google.com changed reviewers:
+ johnniwinther@google.com

[sigurdm, 2015-10-30 10:23:12]

I made a test of consistency between the known platform files.

[Johnni Winther, 2015-10-30 10:38:39]

LGTM

Can you add a test of the consistency between the different .platform files?

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
File pkg/compiler/lib/src/apiimpl.dart (right):

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:58: /// Initialized in [setupSdk()].
Remove ()

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:191: List<String> options) {
Indent to (

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:197: print("%%%
${libraryRoot.resolve(_dart2dartPlatform)}");
Remove debug code. Maybe add as reporter.log(...) call.

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/categories_test.dart (left):

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/categories_test.dart:22: await runTest("import
'dart:async'; main() {}", "Embedded", 1);
We should test this on core_embedded.dart using a dart_embedded.platform
specification.

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/library_resolution_test.dart (right):

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/library_resolution_test.dart:29: import
'package:sdk_library_metadata/libraries.dart'
Remove this.

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/mock_libraries.dart (right):

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/mock_libraries.dart:10: [libraries]
Shouldn't this be empty for the MockCompiler?

[Johnni Winther, 2015-10-30 10:57:54]

Test LGTM

[sigurdm, 2015-10-30 11:38:27]

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
File pkg/compiler/lib/src/apiimpl.dart (right):

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:58: /// Initialized in [setupSdk()].
On 2015/10/30 10:38:39, Johnni Winther wrote:
> Remove ()

Done.

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:191: List<String> options) {
On 2015/10/30 10:38:39, Johnni Winther wrote:
> Indent to (

Done.

https://codereview.chromium.org/1408253006/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:197: print("%%%
${libraryRoot.resolve(_dart2dartPlatform)}");
On 2015/10/30 10:38:39, Johnni Winther wrote:
> Remove debug code. Maybe add as reporter.log(...) call.

Removed

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/categories_test.dart (left):

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/categories_test.dart:22: await runTest("import
'dart:async'; main() {}", "Embedded", 1);
On 2015/10/30 10:38:39, Johnni Winther wrote:
> We should test this on core_embedded.dart using a dart_embedded.platform
> specification.

I actually did not mean to commit core_embedded.dart. It was created as an
experiment
It does not work with the js backend, as the patches are not matching.

I have removed the core_embedded, it should live in the fletch repo - there
should also be a test.

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/library_resolution_test.dart (right):

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/library_resolution_test.dart:29: import
'package:sdk_library_metadata/libraries.dart'
On 2015/10/30 10:38:39, Johnni Winther wrote:
> Remove this.

Done.

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
File tests/compiler/dart2js/mock_libraries.dart (right):

https://codereview.chromium.org/1408253006/diff/20001/tests/compiler/dart2js/...
tests/compiler/dart2js/mock_libraries.dart:10: [libraries]
On 2015/10/30 10:38:39, Johnni Winther wrote:
> Shouldn't this be empty for the MockCompiler?
No - but most of the lines are not needed.

Done.

[sigurdm, 2015-10-30 11:44:46]

Lasse, Florian: Do you have comments/objections to the proposed direction?

[floitsch, 2015-10-30 20:43:07]

floitsch@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2015-10-30 20:43:07]

LGTM.

https://codereview.chromium.org/1408253006/diff/60001/pkg/compiler/lib/src/ap...
File pkg/compiler/lib/src/apiimpl.dart (right):

https://codereview.chromium.org/1408253006/diff/60001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:345: bool allowInternalLibraryAccess = false;
bool allowInternalLibraryAccess = importingLibrary != null &&
    (importingLibrary.isPlatformLibrary ||
     importingLibrary.isPatch ||
     importingLibrary.canonicalUri.path.contains(
          'sdk/tests/compiler/dart2js_native'));

?

https://codereview.chromium.org/1408253006/diff/60001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:414: List<Future> setupFunctions = new
List<Future>();
These are not functions.
I find even "setupFutures" better.

https://codereview.chromium.org/1408253006/diff/60001/sdk/lib/dart_client.pla...
File sdk/lib/dart_client.platform (right):

https://codereview.chromium.org/1408253006/diff/60001/sdk/lib/dart_client.pla...
sdk/lib/dart_client.platform:17: async:async/async.dart
can we allow a space after the ":" ?
we probably won't touch these files very often, but it feels like that would
increase readability tremendously.

[sigurdm, 2015-11-02 10:06:38]

sigurdm@google.com changed reviewers:
+ whesse@google.com

[sigurdm, 2015-11-02 10:06:39]

Bill, could you review the changes to create_sdk.gyp and create_sdk.py?

https://codereview.chromium.org/1408253006/diff/60001/pkg/compiler/lib/src/ap...
File pkg/compiler/lib/src/apiimpl.dart (right):

https://codereview.chromium.org/1408253006/diff/60001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:345: bool allowInternalLibraryAccess = false;
On 2015/10/30 20:43:07, floitsch wrote:
> bool allowInternalLibraryAccess = importingLibrary != null &&
>     (importingLibrary.isPlatformLibrary ||
>      importingLibrary.isPatch ||
>      importingLibrary.canonicalUri.path.contains(
>           'sdk/tests/compiler/dart2js_native'));
> 
> ?

Done.

https://codereview.chromium.org/1408253006/diff/60001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:414: List<Future> setupFunctions = new
List<Future>();
On 2015/10/30 20:43:07, floitsch wrote:
> These are not functions.
> I find even "setupFutures" better.

Done.

https://codereview.chromium.org/1408253006/diff/60001/sdk/lib/dart_client.pla...
File sdk/lib/dart_client.platform (right):

https://codereview.chromium.org/1408253006/diff/60001/sdk/lib/dart_client.pla...
sdk/lib/dart_client.platform:17: async:async/async.dart
On 2015/10/30 20:43:07, floitsch wrote:
> can we allow a space after the ":" ?
> we probably won't touch these files very often, but it feels like that would
> increase readability tremendously.

Good idea.
I now strip all white space from the beginning/end of the keys/values

[Bill Hesse, 2015-11-02 10:33:27]

Build changes LGTM to me.

https://codereview.chromium.org/1408253006/diff/80001/create_sdk.gyp
File create_sdk.gyp (right):

https://codereview.chromium.org/1408253006/diff/80001/create_sdk.gyp#newcode28
create_sdk.gyp:28: '<!@(["python", "tools/list_files.py",'
Would it be better to explicitly list the files here?

https://codereview.chromium.org/1408253006/diff/80001/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/1408253006/diff/80001/tools/create_sdk.py#new...
tools/create_sdk.py:41: # ....lib/
Add the platform files to this comment.

[sigurdm, 2015-11-02 11:56:11]

https://codereview.chromium.org/1408253006/diff/80001/create_sdk.gyp
File create_sdk.gyp (right):

https://codereview.chromium.org/1408253006/diff/80001/create_sdk.gyp#newcode28
create_sdk.gyp:28: '<!@(["python", "tools/list_files.py",'
On 2015/11/02 10:33:27, Bill Hesse wrote:
> Would it be better to explicitly list the files here?

Done

https://codereview.chromium.org/1408253006/diff/80001/tools/create_sdk.py
File tools/create_sdk.py (right):

https://codereview.chromium.org/1408253006/diff/80001/tools/create_sdk.py#new...
tools/create_sdk.py:41: # ....lib/
On 2015/11/02 10:33:27, Bill Hesse wrote:
> Add the platform files to this comment.

Done.

[sigurdm, 2015-11-02 12:02:40]

Committed patchset #5 (id:100001) manually as
c0d5f73bd02506e697e507e172dc4effe07f0787 (presubmit successful).