Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Oct 23, 2020

Description

A dill compiled in sound/unsound mode cannot be reused in --initialize-from-dill for compiling unsound/sound mode. Rename the file if sound mode is detected via command line argument.

This does not work in the case of autodetect.

Fixes #68891

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 23, 2020
@google-cla google-cla bot added the cla: yes label Oct 23, 2020
@jonahwilliams
Copy link
Contributor Author

Most of this change is plumbing, because the test config does not use buildInfo mostly. I can refactor that to be a bit easier to follow later (will require updating g3).

// Temporary work around until --initialize-from-dill accounts for sound mode.
// The tool does not know the compilation mode if [NullSafetyMode.autodetect] is
// selected.
if (nullSafetyMode == NullSafetyMode.sound) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual fix, everything else is plumbing

@required bool trackWidgetCreation,
@required NullSafetyMode nullSafetyMode,
}) {
// Temporary work around until --initialize-from-dill accounts for sound mode.
Copy link
Member

Choose a reason for hiding this comment

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

Could you file an issue for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filled #68901

String randomSeed,
@required List<String> extraFrontEndOptions,
bool nullAssertions = false,
BuildInfo buildInfo // TODO(jonahwilliams): make the default argument
Copy link
Member

Choose a reason for hiding this comment

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

nit: missing comma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm about to head out, if you want to apply this change and re-run CI that is fine - otherwise I can clean this up next week when I make these required and update g3

Copy link
Member

Choose a reason for hiding this comment

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

later is fine.

@zanderso
Copy link
Member

Landing before presubmits are finished to get rollers unblocked.

@zanderso zanderso merged commit 0f28eda into flutter:master Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The engine -> framework roll is failing

3 participants