Skip to content

Conversation

@tvolkert
Copy link
Contributor

@tvolkert tvolkert requested a review from pq June 23, 2017 14:41
Copy link
Contributor

@pq pq left a comment

Choose a reason for hiding this comment

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

As per our conversation, this would bring us ahead of internal which is a trade-off we need to measure. If we're OK with that, LGTM! 👍

@tvolkert tvolkert changed the title Bump Dart SDK version to 1.25.0-dev.3.0 Bump Dart SDK version to 1.24.0 (stable) Jun 23, 2017
@tvolkert
Copy link
Contributor Author

I lowered the version to 1.24.0 (stable). Will land on green.

@aam
Copy link
Member

aam commented Jun 23, 2017

For our work on bringing new dart frontend to flutter #10841 it is critical for us to have more or less latest dart in flutter engine as there are changes being made to dart vm to accommodate new frontend work.
With that in mind - is lowering to 1.24-stable temporary thing to troubleshoot #10650 or we need to come up with some mechanism of having parallel build of flutter engine/flutter tools that lives on more recent versions of dart?

@tvolkert
Copy link
Contributor Author

tvolkert commented Jun 23, 2017

@aam this isn't the Dart SDK version of the embedder (the engine) -- this is the Dart SDK version of the flutter tool (which is being upped from 1.24.0-dev.6.7 to 1.24.0).

It's admittedly unfortunate and confusing that the two aren't in sync.

@eseidelGoogle
Copy link
Contributor

We (rather trivially) could build our own Dart SDK on our bots and download it from the flutter_infra GCE bucket and always have the flutter/flutter version use the flutter/engine's exact Dart revision. We just haven't done it.

@aam
Copy link
Member

aam commented Jun 23, 2017

Oops, got you, thanks!
By the way, you can use 'hash/65a5707189dec6c5e0174cbc821acfb90f0c9b3f' syntax in bin/internal/dart-sdk.version to reference particular bleeding edge of dart.
Also, with dart frontend support rolling out to flutter, version of dart sdk used by the flutter tools will have to [closely] match version of dart sdk used in the engine since kernel(parse tree) binaries produced on the workstation have to be compatible with what kernel binary reader on the device expects.

@tvolkert
Copy link
Contributor Author

We should coordinate that with the team in Google that rolls the Dart SDK internally, since we want to make sure we don't put ourselves in a situation where we can't roll Flutter into Google because it depends on a Dart SDK version that's too far ahead of the internal SDK version.

/cc @keertip @srawlins

@aam
Copy link
Member

aam commented Jun 23, 2017

In short term, it will take some time for us to iron all kinks out flutter-with-new-frontend configuration. While we do that, new frontend functionality in flutter will be "hidden" behind configuration option (flutter run --is-kernel-mode, for example). During that phase, there is no need for everybody to have flutter tools and flutter engine on the same revision - only folks who decide to run with --is-kernel-mode have to do that, and they can accomplish this by manually updating dart-sdk.version to hash/..., that matches engine/src/flutter/DEPS dart_revision.
Hopefully by the time frontend integration with flutter is tested and verified, there will be no need to stay on bleeding edge of dart to pick up dart vm fixes for frontend, so we can go back to using dev channel versions.
Generally speaking, though, there will be [small?] risk in in rolling engine's dart dependency forward while leaving flutter tools's dart-sdk in place in case there are some changes to kernel format.

@a-siva

@tvolkert tvolkert merged commit 128967c into flutter:master Jun 23, 2017
@tvolkert tvolkert deleted the sdk branch June 23, 2017 21:08
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jul 1, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants