Add support for package cycles linking, more tests.

Add support for package cycles linking, more tests.

R=paulberry@google.com
BUG=

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

[Paul Berry, 2016-08-10 11:58:28]

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/pub_summary.dart (right):

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:362: * If the given [uri] has the
`package` scheme, return the names of the
s/names/name/

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:421: // SDK is always available.
It looks like you're not going with my suggestion from the previous CL review
(consider the SDK to be a regular summary just like the others, so we don't have
to special-case "dart:" imports).  I see how your approach could work for
ordinary SDK's (not modified by _embedder.yaml and _sdkext).  But I don't see
how this is going to work for Flutter.  How do you plan to address the Flutter
use case?

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:422: // It's API signature is
always mixed in.
s/It's/Its/

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:453: void evaluate(_LinkedNode
node) {
There's a lot of commonality between evaluate() and evaluateScc() now.  I think
it would be better to eliminate code dupliation by replacing evaluate() with:

void evaluate(_LinkedNode node) {
  evaluateScc([node]);
}

(This has the disadvantage that we'll be doing slightly more bookkeeping during
each evaluate() call, since we'll be using the more general algorithm that
supports a cycle among packages.  But I think that's outweighed by the fact that
the code will be shorter overall, and the more complex logic in evaluateScc()
will actually get exercised in real-world cases).

[scheglov, 2016-08-10 16:08:19]

PTAL

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/pub_summary.dart (right):

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:362: * If the given [uri] has the
`package` scheme, return the names of the
On 2016/08/10 11:58:28, Paul Berry wrote:
> s/names/name/

Done.

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:421: // SDK is always available.
On 2016/08/10 11:58:27, Paul Berry wrote:
> It looks like you're not going with my suggestion from the previous CL review
> (consider the SDK to be a regular summary just like the others, so we don't
have
> to special-case "dart:" imports).  I see how your approach could work for
> ordinary SDK's (not modified by _embedder.yaml and _sdkext).  But I don't see
> how this is going to work for Flutter.  How do you plan to address the Flutter
> use case?

getLinkedBundles() accepts "PackageBundle sdkBundle". As I understand after
discussion with Brian, the decision about Flutter was that the Flutter package
will come with its own full SDK summary bundle. So, we will use its API
signatures and so ensure non-sharing of linked summaries for Flutter and
non-Flutter users.

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:422: // It's API signature is
always mixed in.
On 2016/08/10 11:58:27, Paul Berry wrote:
> s/It's/Its/

Done.

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:453: void evaluate(_LinkedNode
node) {
On 2016/08/10 11:58:28, Paul Berry wrote:
> There's a lot of commonality between evaluate() and evaluateScc() now.  I
think
> it would be better to eliminate code dupliation by replacing evaluate() with:
> 
> void evaluate(_LinkedNode node) {
>   evaluateScc([node]);
> }
> 
> (This has the disadvantage that we'll be doing slightly more bookkeeping
during
> each evaluate() call, since we'll be using the more general algorithm that
> supports a cycle among packages.  But I think that's outweighed by the fact
that
> the code will be shorter overall, and the more complex logic in evaluateScc()
> will actually get exercised in real-world cases).

Done.

[Paul Berry, 2016-08-10 21:39:37]

lgtm

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/pub_summary.dart (right):

https://codereview.chromium.org/2227393002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/pub_summary.dart:421: // SDK is always available.
On 2016/08/10 16:08:18, scheglov wrote:
> On 2016/08/10 11:58:27, Paul Berry wrote:
> > It looks like you're not going with my suggestion from the previous CL
review
> > (consider the SDK to be a regular summary just like the others, so we don't
> have
> > to special-case "dart:" imports).  I see how your approach could work for
> > ordinary SDK's (not modified by _embedder.yaml and _sdkext).  But I don't
see
> > how this is going to work for Flutter.  How do you plan to address the
Flutter
> > use case?
> 
> getLinkedBundles() accepts "PackageBundle sdkBundle". As I understand after
> discussion with Brian, the decision about Flutter was that the Flutter package
> will come with its own full SDK summary bundle. So, we will use its API
> signatures and so ensure non-sharing of linked summaries for Flutter and
> non-Flutter users.

Ok, based on our discussions, I'm happy landing this as is and improving it in
follow-up commits.  Consider adding a TODO comment.

[scheglov, 2016-08-10 21:53:27]

Description was changed from

==========
Add support for package cycles linking, more tests.

R=paulberry@google.com
BUG=
==========

to

==========
Add support for package cycles linking, more tests.

R=paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/5f864e237a7097ec2eb0d975a993dcb819fcd3b4
==========

[scheglov, 2016-08-10 21:53:29]

Committed patchset #3 (id:40001) manually as
5f864e237a7097ec2eb0d975a993dcb819fcd3b4 (presubmit successful).