Skip to content

Account for alternate "SDK" layouts.#29297

Closed
pylaligand wants to merge 1 commit intodart-lang:masterfrom
pylaligand:analysis
Closed

Account for alternate "SDK" layouts.#29297
pylaligand wants to merge 1 commit intodart-lang:masterfrom
pylaligand:analysis

Conversation

@pylaligand
Copy link
Contributor

The sources for standard Dart libraries are coming via an embedder (Flutter) in Fuchsia, with a slightly different tree layout.

The sources for standard Dart libraries are coming via an embedder (Flutter) in Fuchsia, with a slightly different tree layout.
@pylaligand
Copy link
Contributor Author

pylaligand commented Apr 6, 2017

On Fuchsia we get the Dart standard libraries via Flutter. core.dart is located in a tree that looks like:

base
  |
  |------- lib
  |          |------- embedder.yaml (points to ../dart_sdk)
  |
  |------- dart_sdk
             |------- async
             |------- core
             |          |------- core.dart
             |
             |------- etc...

When trying to locate the root of the SDK, the while loop would hit / as there's never a lib folder in core.dart's path and then keep looping forever. The result is that the analysis server would remain stuck and not give any results.

This is a quick fix, but I'm open to other suggestions. For example I think we could put all the stuff that's in dart_sdk under lib so that the layout resembles that of the standard Dart SDK.

/cc @abarth

@abarth
Copy link
Contributor

abarth commented Apr 6, 2017

We can change the layout in flutter to whatever is useful. I don't think we've given any thought to the current layout.

@bwilkerson
Copy link
Member

Unless there is a strong need to deviate from the standard SDK layout, I would strongly urge you to not change it. Even then, I would encourage to talk to the Dart team so that we can see whether we can change the standard SDK layout to meet both sets of needs. Having to special case things like this makes it harder for us to test everything thoroughly.

Copy link
Member

@bwilkerson bwilkerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be cherry-picked into 1.23? If so, please file a merge request.

@pylaligand
Copy link
Contributor Author

I'll make the change on the Flutter side to match the standard layout better.

In any case, there's no need to cherry-pick anything as we build our Dart stuff from sources in Fuchsia.

@pylaligand
Copy link
Contributor Author

flutter/engine#3575

@pylaligand pylaligand closed this Apr 7, 2017
@pylaligand pylaligand deleted the analysis branch April 7, 2017 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants