Ensure //flutter/fml is dependency free. #25749
Conversation
flutter-dashboard
bot
added
embedder
|
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 Hixie on the #hackers channel in Chat. 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. |
|
This improves build times of profile and release modes because some tests were importing libdart_jit solely to satisfy the Dart VM dependency. So the tests in those modes that did not use the VM compiled the JIT VM and the tests that did use the VM compiled the AOT VM. One VM variant was compiled for no reason and if the tests that did not use a VM started to use one in the future, there would be linker errors due to duplicate symbols. |
|
This should also fix the mysterious link errors in Fuchsia flutter/flutter#81135 as deps are now introduced at the level in which they are used instead of at the top level target. |
|
An unrelated flutter_tester Dart test is failing repeatedly. I filed a flake report and now I am not sure its really flaky. Trying to reproduce it locally. |
|
Hmm, Flutter Tester is being tested in release mode where AOT needs to be used but the tester was never configured for AOT. |
chinmaygarde
force-pushed
the
no_dart_in_fml
branch
from
7204b57 to
bc30a41
Compare
This patch removes tracing from FML and moves it into a separate library at //flutter/fml/trace. This is similar to the pattern followed by //flutter/fml/dart which contains Dart converter for FML utilities. The presence of tracing in FML was problematic because the tracing subsystem is provided by Dart. So, depending on FML would automatically pull in a Dart dependency. This happened for unit-test targets that did not depend on Dart and event had no mechanisms to collect traces from the Dart timeline. Now, applications that want to trace will explicitly need to depend on the trace library with a clear message indicating the GN rule to be included. This is an improvement over the current situation where including FML but not including the Dart dependency would lead to link errors with no indication of which target to include. This has tripped up the others in the past flutter/flutter#41414. With this patch, FML has no other dependencies and users of FML that don't need Dart can be small and compile quickly. Fixes flutter/flutter#41414.
chinmaygarde
force-pushed
the
no_dart_in_fml
branch
from
bc30a41 to
97affb0
Compare
|
This one is blocked because we still have CI steps that depend on a JIT variant of flutter_tester to be necessary in AOT modes. |
3 participants
This patch removes tracing from FML and moves it into a separate library at
//flutter/fml/trace. This is similar to the pattern followed by
//flutter/fml/dart which contains Dart converters for FML utilities.
The presence of tracing in FML was problematic because the tracing subsystem is
provided by Dart. So, depending on FML would automatically pull in a Dart
dependency. This happened for unit-test targets that did not depend on Dart and
even had no mechanisms to collect traces from the Dart timeline.
Now, applications that want to trace will explicitly need to depend on the trace
library with a clear message indicating the GN rule to be included. This is an
improvement over the current situation where including FML but not including the
Dart dependency would lead to link errors with no indication of which target to
include. This has tripped up the others in the past
flutter/flutter#41414.
With this patch, FML has no other dependencies and users of FML that don't need
Dart can be small and compile quickly.
Fixes flutter/flutter#41414.