Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@iinozemtsev
Copy link
Contributor

  • use core vm snapshot instead of core-jit, which does not seem to support sound null safety agnostic mode
  • remove hardcoded --no-sound-null-safety flags from native code and ninja args
  • add sound_null_safety attribute to flutter_component in ninja, which propagates to dart_kernel
  • migrate one integration test to sound null safety, so that there are integration tests in both modes

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@zanderso
Copy link
Member

@CaseyHillers is there any way to try this patch in g3 before landing it?

@CaseyHillers
Copy link
Contributor

@CaseyHillers is there any way to try this patch in g3 before landing it?

Yes, we can follow go/flutter-life-of-a-pr#google-testing-failed-how-do-i-debug once all the GitHub checks are green.

@iinozemtsev
Copy link
Contributor Author

All checks are green now and I can do this tomorrow. Are Fuchsia artifacts for pending PRs also fetched into google3 now?

@CaseyHillers
Copy link
Contributor

All checks are green now and I can do this tomorrow. Are Fuchsia artifacts for pending PRs also fetched into google3 now?

Yes, but you may need to patch in cl/600934274 for the short term. That will fix the issue where the wrong hash is being used on engine PR rolls. 🤞 it's submitted tomorrow.

@iinozemtsev
Copy link
Contributor Author

I followed go/flutter-on-fuchsia-build#building-the-engine-for-smart-display to bring fuchsia artifacts into g3, everything works as intended (I can use sound kernels with flutter jit runner now) and couldn't find any breakages

@jrwang jrwang self-requested a review January 30, 2024 17:14
@jrwang jrwang assigned iinozemtsev and unassigned jrwang Jan 30, 2024
Copy link
Contributor

@jrwang jrwang left a comment

Choose a reason for hiding this comment

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

LGTM

@iinozemtsev
Copy link
Contributor Author

iinozemtsev commented Jan 30, 2024 via email

@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 30, 2024
@auto-submit auto-submit bot merged commit 85293d7 into flutter:main Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 30, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 31, 2024
…142564)

flutter/engine@d20ed24...e6e1d6b

2024-01-30 [email protected] Roll Dart SDK from 734aae9604e8 to 488e33cd39de (1 revision) (flutter/engine#50185)
2024-01-30 [email protected] [Impeller]  Reland: add interface for submitting multiple command buffers at once. (flutter/engine#50180)
2024-01-30 [email protected] Support running sound null safe kernels from flutter_jit_runner (flutter/engine#50002)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-fuchsia

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants