- Properly load the core libraries as libraries and not as scripts.

- Properly load the core libraries as libraries and not as scripts.
  This will honor their imports and library names.
Committed: https://code.google.com/p/dart/source/detail?r=19017

[siva, 2013-02-23 00:11:02]

https://codereview.chromium.org/12321082/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/12321082/diff/1/runtime/vm/object.cc#newcode838
runtime/vm/object.cc:838: Library::InitScalarlistLibrary(isolate);
When we clean up the library init stuff some of this may move around. As
discussed we could do that in another CL.

https://codereview.chromium.org/12321082/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/12321082/diff/1/runtime/vm/parser.cc#newcode4276
runtime/vm/parser.cc:4276: UNREACHABLE();
Why UNREACHABLE here (AppendErrorMsg already has one)?

https://codereview.chromium.org/12321082/diff/1/sdk/lib/_collection_dev/colle...
File sdk/lib/_collection_dev/collection_dev_sources.gypi (right):

https://codereview.chromium.org/12321082/diff/1/sdk/lib/_collection_dev/colle...
sdk/lib/_collection_dev/collection_dev_sources.gypi:8: 'collection_dev.dart',
Comment that this should be the first file?

[Ivan Posva, 2013-02-25 22:20:14]

PTAL -ip

https://codereview.chromium.org/12321082/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/12321082/diff/1/runtime/vm/object.cc#newcode838
runtime/vm/object.cc:838: Library::InitScalarlistLibrary(isolate);
On 2013/02/23 00:11:03, siva wrote:
> When we clean up the library init stuff some of this may move around. As
> discussed we could do that in another CL.

Already cleaned up.

https://codereview.chromium.org/12321082/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/12321082/diff/1/runtime/vm/parser.cc#newcode4276
runtime/vm/parser.cc:4276: UNREACHABLE();
On 2013/02/23 00:11:03, siva wrote:
> Why UNREACHABLE here (AppendErrorMsg already has one)?

I wanted to make it clear that we will not be able to continue below. Instead I
will add an appropriate comment.

https://codereview.chromium.org/12321082/diff/1/sdk/lib/_collection_dev/colle...
File sdk/lib/_collection_dev/collection_dev_sources.gypi (right):

https://codereview.chromium.org/12321082/diff/1/sdk/lib/_collection_dev/colle...
sdk/lib/_collection_dev/collection_dev_sources.gypi:8: 'collection_dev.dart',
On 2013/02/23 00:11:03, siva wrote:
> Comment that this should be the first file?

Done.

[siva, 2013-02-25 23:12:25]

LGTM.

(some more TODO(ager) can be removed from *sources*.gypi)

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/crypto/crypto_sou...
File sdk/lib/crypto/crypto_sources.gypi (right):

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/crypto/crypto_sou...
sdk/lib/crypto/crypto_sources.gypi:9: # At that point crypto.dart should be the
only crypto library file.
This TODO can go.
We discussed and said we would retain the pattern of
having one top file which does just the import/part stuff and
the rest of the stuff is in individual part files.
This already follows the pattern and hence no more changes needed.

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/utf/utf_sources.gypi
File sdk/lib/utf/utf_sources.gypi (right):

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/utf/utf_sources.g...
sdk/lib/utf/utf_sources.gypi:8: # At that point utf.dart should be the only utf
library file.
This TODO can go.

https://codereview.chromium.org/12321082/diff/2008/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/12321082/diff/2008/runtime/vm/object.cc#newco...
runtime/vm/object.cc:639: lib = Library::NewLibraryHelper(url, true);           
                    \
ASSERT(!lib.IsNull());

[Ivan Posva, 2013-02-25 23:23:56]

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/crypto/crypto_sou...
File sdk/lib/crypto/crypto_sources.gypi (right):

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/crypto/crypto_sou...
sdk/lib/crypto/crypto_sources.gypi:9: # At that point crypto.dart should be the
only crypto library file.
On 2013/02/25 23:12:25, siva wrote:
> This TODO can go.
> We discussed and said we would retain the pattern of
> having one top file which does just the import/part stuff and
> the rest of the stuff is in individual part files.
> This already follows the pattern and hence no more changes needed.

Done.

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/utf/utf_sources.gypi
File sdk/lib/utf/utf_sources.gypi (right):

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/utf/utf_sources.g...
sdk/lib/utf/utf_sources.gypi:8: # At that point utf.dart should be the only utf
library file.
On 2013/02/25 23:12:25, siva wrote:
> This TODO can go.

Done.

https://codereview.chromium.org/12321082/diff/2008/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/12321082/diff/2008/runtime/vm/object.cc#newco...
runtime/vm/object.cc:639: lib = Library::NewLibraryHelper(url, true);           
                    \
On 2013/02/25 23:12:25, siva wrote:
> ASSERT(!lib.IsNull());

We do not check for allocation success anywhere else either.

[Ivan Posva, 2013-02-25 23:25:46]

Committed patchset #4 manually as r19017 (presubmit successful).

[Mads Ager (google), 2013-02-26 07:40:51]

Excellent! Thanks!

I wonder if we should complicate the python script a bit to avoid having to list
the parts in the .gypi file.

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/utf/utf_sources.gypi
File sdk/lib/utf/utf_sources.gypi (right):

https://codereview.chromium.org/12321082/diff/11002/sdk/lib/utf/utf_sources.g...
sdk/lib/utf/utf_sources.gypi:8: # At that point utf.dart should be the only utf
library file.
On 2013/02/25 23:23:58, Ivan Posva wrote:
> On 2013/02/25 23:12:25, siva wrote:
> > This TODO can go.
> 
> Done.

This is great, thanks! We are only half-way to the goal of the TODO. We still
need to list all the parts in this file and cannot just use the utf.dart library
file. How hard would it be to avoid that? The information is already in the
utf.dart file and it would be cool if that was enough. That would make it easier
to hack on these core libraries without having to mess with gyp files. I guess
the understanding of the part declarations would have to be in the python script
which is of course an added complication in that script.

[Mads Ager (google), 2013-02-26 09:08:40]

On 2013/02/26 07:40:51, Mads Ager wrote:
> Excellent! Thanks!
> 
> I wonder if we should complicate the python script a bit to avoid having to
list
> the parts in the .gypi file.

That turned out to be very easy. I have uploaded a codereview to remove the
duplication of the 'part' declarations in the gypi files. Let me know what you
think: https://codereview.chromium.org/12321129/