Skip to content

Commit 824868f

Browse files
auto-submit[bot]auto-submit[bot]
andauthored
Reverts "Data assets (flutter#169273)" (flutter#170034)
<!-- start_original_pr_link --> Reverts: flutter#169273 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: jason-simmons <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: seeing flakes of `dart_data_asset_test.dart` CI runs for various PRs got errors in the `tool_integration_tests` shards. The errors occurred on multiple platforms (Linux, Mac, and Windows). Affected PRs include: flutter#170022, flutter#169995 LUCI log of an error: https://ci.chromium.org/ui/p/flutter/builders/try/Windows%20t <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: mosuem <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {bkonyi} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Refiling of flutter#164094, which itself is a rebase of flutter#159675 This PR adds bundling support for the experimental dart data asset feature: Dart packages with hooks can now emit data assets which the flutter tool will bundle. It relies on flutter's existing asset bundling mechanism (e.g. entries in AssetManifest.json, DevFS syncing in reload/restart, ...). The support is added under an experimental flag (similar to the existing native assets experimental flag). Also, kNativeAssets is removed to also bundle data assets on flutter build bundle. The chrome sandbox is disabled as per flutter#165664. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
1 parent e094c18 commit 824868f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+592
-1806
lines changed

packages/flutter_tools/lib/executable.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import 'src/base/template.dart';
1111
import 'src/base/terminal.dart';
1212
import 'src/base/user_messages.dart';
1313
import 'src/build_system/build_targets.dart';
14-
import 'src/build_system/targets/hook_runner_native.dart' show FlutterHookRunnerNative;
1514
import 'src/cache.dart';
1615
import 'src/commands/analyze.dart';
1716
import 'src/commands/assemble.dart';
@@ -49,7 +48,6 @@ import 'src/devtools_launcher.dart';
4948
import 'src/features.dart';
5049
import 'src/globals.dart' as globals;
5150
// Files in `isolated` are intentionally excluded from google3 tooling.
52-
import 'src/hook_runner.dart' show FlutterHookRunner;
5351
import 'src/isolated/build_targets.dart';
5452
import 'src/isolated/mustache_template.dart';
5553
import 'src/isolated/native_assets/test/native_assets.dart';
@@ -119,7 +117,6 @@ Future<void> main(List<String> args) async {
119117
muteCommandLogging: muteCommandLogging,
120118
verboseHelp: verboseHelp,
121119
overrides: <Type, Generator>{
122-
FlutterHookRunner: () => FlutterHookRunnerNative(),
123120
// The web runner is not supported in google3 because it depends
124121
// on dwds.
125122
WebRunnerFactory: () => DwdsWebRunnerFactory(),

packages/flutter_tools/lib/src/asset.dart

Lines changed: 8 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -25,84 +25,6 @@ import 'flutter_manifest.dart';
2525
import 'license_collector.dart';
2626
import 'project.dart';
2727

28-
class FlutterHookResult {
29-
const FlutterHookResult({
30-
required this.buildStart,
31-
required this.buildEnd,
32-
required this.dataAssets,
33-
required this.dependencies,
34-
});
35-
36-
FlutterHookResult.empty()
37-
: this(
38-
buildStart: DateTime.fromMillisecondsSinceEpoch(0),
39-
buildEnd: DateTime.fromMillisecondsSinceEpoch(0),
40-
dataAssets: <HookAsset>[],
41-
dependencies: <Uri>[],
42-
);
43-
44-
final List<HookAsset> dataAssets;
45-
46-
/// The timestamp at which we start a build - so the timestamp of the inputs.
47-
final DateTime buildStart;
48-
49-
/// The timestamp at which we finish a build - so the timestamp of the
50-
/// outputs.
51-
final DateTime buildEnd;
52-
53-
/// The dependencies of the build are used to check if the build needs to be
54-
/// rerun.
55-
final List<Uri> dependencies;
56-
57-
/// Whether caller may need to re-run the Dart build.
58-
bool hasAnyModifiedFiles(FileSystem fileSystem) =>
59-
_wasAnyFileModifiedSince(fileSystem, buildStart, dependencies);
60-
61-
/// Whether the files produced by the build are up-to-date.
62-
///
63-
/// NOTICE: The build itself may be up-to-date but the output may not be (as
64-
/// the output may be existing on disk and not be produced by the build
65-
/// itself - in which case we may not need to re-build if the file changes,
66-
/// but we may need to make a new asset bundle with the modified file).
67-
bool isOutputDirty(FileSystem fileSystem) => _wasAnyFileModifiedSince(
68-
fileSystem,
69-
buildEnd,
70-
dataAssets.map((HookAsset e) => e.file).toList(),
71-
);
72-
73-
static bool _wasAnyFileModifiedSince(FileSystem fileSystem, DateTime since, List<Uri> uris) {
74-
for (final Uri uri in uris) {
75-
final DateTime modified = fileSystem.statSync(uri.toFilePath()).modified;
76-
if (modified.isAfter(since)) {
77-
return true;
78-
}
79-
}
80-
return false;
81-
}
82-
83-
@override
84-
String toString() {
85-
return dataAssets.toString();
86-
}
87-
}
88-
89-
/// A convenience class to wrap native assets
90-
///
91-
/// When translating from a `DartHooksResult` to a [FlutterHookResult], where we
92-
/// need to have different classes to not import `isolated/` stuff.
93-
class HookAsset {
94-
const HookAsset({required this.file, required this.name, required this.package});
95-
96-
final Uri file;
97-
final String name;
98-
final String package;
99-
100-
@override
101-
String toString() {
102-
return 'HookAsset(file: $file, name: $name, package: $package)';
103-
}
104-
}
105-
10628
const String defaultManifestPath = 'pubspec.yaml';
10729

10830
const String kFontManifestJson = 'FontManifest.json';
@@ -191,7 +113,6 @@ abstract class AssetBundle {
191113

192114
/// Returns 0 for success; non-zero for failure.
193115
Future<int> build({
194-
FlutterHookResult? flutterHookResult,
195116
String manifestPath = defaultManifestPath,
196117
required String packageConfigPath,
197118
bool deferredComponentsEnabled = false,
@@ -242,8 +163,7 @@ class ManifestAssetBundle implements AssetBundle {
242163
_platform = platform,
243164
_flutterRoot = flutterRoot,
244165
_splitDeferredAssets = splitDeferredAssets,
245-
_licenseCollector = LicenseCollector(fileSystem: fileSystem),
246-
_lastHookResult = FlutterHookResult.empty();
166+
_licenseCollector = LicenseCollector(fileSystem: fileSystem);
247167

248168
final Logger _logger;
249169
final FileSystem _fileSystem;
@@ -269,8 +189,6 @@ class ManifestAssetBundle implements AssetBundle {
269189

270190
DateTime? _lastBuildTimestamp;
271191

272-
FlutterHookResult _lastHookResult;
273-
274192
// We assume the main asset is designed for a device pixel ratio of 1.0.
275193
static const String _kAssetManifestJsonFilename = 'AssetManifest.json';
276194
static const String _kAssetManifestBinFilename = 'AssetManifest.bin';
@@ -290,19 +208,13 @@ class ManifestAssetBundle implements AssetBundle {
290208

291209
@override
292210
bool needsBuild({String manifestPath = defaultManifestPath}) {
293-
if (!wasBuiltOnce() ||
294-
// We need to re-run the Dart build.
295-
_lastHookResult.hasAnyModifiedFiles(_fileSystem) ||
296-
// We don't have to re-run the Dart build, but some files the Dart build
297-
// wants us to bundle have changed contents.
298-
_lastHookResult.isOutputDirty(_fileSystem)) {
211+
final DateTime? lastBuildTimestamp = _lastBuildTimestamp;
212+
if (lastBuildTimestamp == null) {
299213
return true;
300214
}
301-
final DateTime lastBuildTimestamp = _lastBuildTimestamp!;
302215

303216
final FileStat manifestStat = _fileSystem.file(manifestPath).statSync();
304-
if (manifestStat.type == FileSystemEntityType.notFound ||
305-
manifestStat.modified.isAfter(lastBuildTimestamp)) {
217+
if (manifestStat.type == FileSystemEntityType.notFound) {
306218
return true;
307219
}
308220

@@ -311,19 +223,18 @@ class ManifestAssetBundle implements AssetBundle {
311223
return true; // directory was deleted.
312224
}
313225
for (final File file in directory.listSync().whereType<File>()) {
314-
final DateTime lastModified = file.statSync().modified;
315-
if (lastModified.isAfter(lastBuildTimestamp)) {
226+
final DateTime dateTime = file.statSync().modified;
227+
if (dateTime.isAfter(lastBuildTimestamp)) {
316228
return true;
317229
}
318230
}
319231
}
320232

321-
return false;
233+
return manifestStat.modified.isAfter(lastBuildTimestamp);
322234
}
323235

324236
@override
325237
Future<int> build({
326-
FlutterHookResult? flutterHookResult,
327238
String manifestPath = defaultManifestPath,
328239
FlutterProject? flutterProject,
329240
required String packageConfigPath,
@@ -347,7 +258,6 @@ class ManifestAssetBundle implements AssetBundle {
347258
// hang on hot reload, as the incremental dill files will never be copied to the
348259
// device.
349260
_lastBuildTimestamp = DateTime.now();
350-
_lastHookResult = flutterHookResult ?? FlutterHookResult.empty();
351261
if (flutterManifest.isEmpty) {
352262
entries[_kAssetManifestJsonFilename] = AssetBundleEntry(
353263
DevFSStringContent('{}'),
@@ -515,38 +425,9 @@ class ManifestAssetBundle implements AssetBundle {
515425
);
516426
}
517427
}
518-
for (final HookAsset dataAsset in flutterHookResult?.dataAssets ?? <HookAsset>[]) {
519-
final Package package = packageConfig[dataAsset.package]!;
520-
final Uri fileUri = dataAsset.file;
521-
522-
final String baseDir;
523-
final Uri relativeUri;
524-
if (fileUri.isAbsolute) {
525-
final String filePath = fileUri.toFilePath();
526-
baseDir = _fileSystem.path.dirname(filePath);
527-
relativeUri = Uri(path: _fileSystem.path.basename(filePath));
528-
} else {
529-
baseDir = package.root.toFilePath();
530-
relativeUri = fileUri;
531-
}
532-
533-
final _Asset asset = _Asset(
534-
baseDir: baseDir,
535-
relativeUri: relativeUri,
536-
entryUri: Uri.parse(_fileSystem.path.join('packages', dataAsset.package, dataAsset.name)),
537-
package: package,
538-
);
539-
if (assetVariants.containsKey(asset)) {
540-
_logger.printError(
541-
'Conflicting assets: The asset "$asset" was declared in the pubspec and the hook.',
542-
);
543-
return 1;
544-
}
545-
assetVariants[asset] = <_Asset>[asset];
546-
}
547428

548429
// Save the contents of each image, image variant, and font
549-
// asset in [entries].
430+
// asset in entries.
550431
for (final _Asset asset in assetVariants.keys) {
551432
final File assetFile = asset.lookupAssetFile(_fileSystem);
552433
final List<_Asset> variants = assetVariants[asset]!;

packages/flutter_tools/lib/src/build_info.dart

Lines changed: 14 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -595,33 +595,6 @@ enum TargetPlatform {
595595
}
596596
}
597597

598-
String get osName {
599-
switch (this) {
600-
case TargetPlatform.linux_x64:
601-
case TargetPlatform.linux_arm64:
602-
return 'linux';
603-
case TargetPlatform.darwin:
604-
return 'macos';
605-
case TargetPlatform.windows_x64:
606-
case TargetPlatform.windows_arm64:
607-
return 'windows';
608-
case TargetPlatform.android:
609-
case TargetPlatform.android_arm:
610-
case TargetPlatform.android_arm64:
611-
case TargetPlatform.android_x64:
612-
return 'android';
613-
case TargetPlatform.fuchsia_arm64:
614-
case TargetPlatform.fuchsia_x64:
615-
return 'fuchsia';
616-
case TargetPlatform.ios:
617-
return 'ios';
618-
case TargetPlatform.tester:
619-
return 'flutter-tester';
620-
case TargetPlatform.web_javascript:
621-
return 'web';
622-
}
623-
}
624-
625598
String get simpleName {
626599
switch (this) {
627600
case TargetPlatform.linux_x64:
@@ -744,15 +717,6 @@ DarwinArch getDarwinArchForName(String arch) {
744717
};
745718
}
746719

747-
List<DarwinArch> getDarwinArchsFromEnv(Map<String, String> defines) {
748-
const List<DarwinArch> defaultDarwinArchitectures = <DarwinArch>[
749-
DarwinArch.x86_64,
750-
DarwinArch.arm64,
751-
];
752-
return defines[kDarwinArchs]?.split(' ').map(getDarwinArchForName).toList() ??
753-
defaultDarwinArchitectures;
754-
}
755-
756720
String getNameForTargetPlatform(TargetPlatform platform, {DarwinArch? darwinArch}) {
757721
return switch (platform) {
758722
TargetPlatform.ios when darwinArch != null => 'ios-${darwinArch.name}',
@@ -989,6 +953,20 @@ const String kSdkRoot = 'SdkRoot';
989953
/// Whether to enable Dart obfuscation and where to save the symbol map.
990954
const String kDartObfuscation = 'DartObfuscation';
991955

956+
/// Whether to enable Native Assets.
957+
///
958+
/// If true, native assets are built and the mapping for native assets lookup
959+
/// at runtime is embedded in the kernel file.
960+
///
961+
/// If false, native assets are not built, and an empty mapping is embedded in
962+
/// the kernel file. Used for targets that trigger kernel builds but
963+
/// are not OS/architecture specific.
964+
///
965+
/// Supported values are 'true' and 'false'.
966+
///
967+
/// Defaults to 'true'.
968+
const String kNativeAssets = 'NativeAssets';
969+
992970
/// An output directory where one or more code-size measurements may be written.
993971
const String kCodeSizeDirectory = 'CodeSizeDirectory';
994972

packages/flutter_tools/lib/src/build_system/targets/android.dart

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import '../../base/file_system.dart';
99
import '../../build_info.dart';
1010
import '../../devfs.dart';
1111
import '../../globals.dart' as globals show xcode;
12-
import '../../isolated/native_assets/dart_hook_result.dart';
1312
import '../../project.dart';
1413
import '../build_system.dart';
1514
import '../depfile.dart';
@@ -71,11 +70,9 @@ abstract class AndroidAssetBundle extends Target {
7170
.file(isolateSnapshotData)
7271
.copySync(outputDirectory.childFile('isolate_snapshot_data').path);
7372
}
74-
final DartHookResult dartHookResult = await DartBuild.loadHookResult(environment);
7573
final Depfile assetDepfile = await copyAssets(
7674
environment,
7775
outputDirectory,
78-
dartHookResult: dartHookResult,
7976
targetPlatform: TargetPlatform.android,
8077
buildMode: buildMode,
8178
flavor: environment.defines[kFlavor],
@@ -92,11 +89,7 @@ abstract class AndroidAssetBundle extends Target {
9289
}
9390

9491
@override
95-
List<Target> get dependencies => const <Target>[
96-
DartBuildForNative(),
97-
KernelSnapshot(),
98-
InstallCodeAssets(),
99-
];
92+
List<Target> get dependencies => const <Target>[KernelSnapshot(), InstallCodeAssets()];
10093
}
10194

10295
/// An implementation of [AndroidAssetBundle] that includes dependencies on vm

packages/flutter_tools/lib/src/build_system/targets/assets.dart

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import '../../build_info.dart';
1212
import '../../dart/package_map.dart';
1313
import '../../devfs.dart';
1414
import '../../flutter_manifest.dart';
15-
import '../../isolated/native_assets/dart_hook_result.dart';
1615
import '../build_system.dart';
1716
import '../depfile.dart';
1817
import '../exceptions.dart';
@@ -34,7 +33,6 @@ import 'native_assets.dart';
3433
Future<Depfile> copyAssets(
3534
Environment environment,
3635
Directory outputDirectory, {
37-
required DartHookResult dartHookResult,
3836
Map<String, DevFSContent> additionalContent = const <String, DevFSContent>{},
3937
required TargetPlatform targetPlatform,
4038
required BuildMode buildMode,
@@ -51,7 +49,6 @@ Future<Depfile> copyAssets(
5149
splitDeferredAssets: buildMode != BuildMode.debug && buildMode != BuildMode.jitRelease,
5250
).createBundle();
5351
final int resultCode = await assetBundle.build(
54-
flutterHookResult: dartHookResult.asFlutterResult,
5552
manifestPath: pubspecFile.path,
5653
packageConfigPath: findPackageConfigFileOrDefault(environment.projectDir).path,
5754
deferredComponentsEnabled: environment.defines[kDeferredComponents] == 'true',
@@ -251,11 +248,7 @@ class CopyAssets extends Target {
251248
String get name => 'copy_assets';
252249

253250
@override
254-
List<Target> get dependencies => const <Target>[
255-
DartBuildForNative(),
256-
KernelSnapshot(),
257-
InstallCodeAssets(),
258-
];
251+
List<Target> get dependencies => const <Target>[KernelSnapshot(), InstallCodeAssets()];
259252

260253
@override
261254
List<Source> get inputs => const <Source>[
@@ -281,11 +274,9 @@ class CopyAssets extends Target {
281274
final BuildMode buildMode = BuildMode.fromCliName(buildModeEnvironment);
282275
final Directory output = environment.buildDir.childDirectory('flutter_assets');
283276
output.createSync(recursive: true);
284-
final DartHookResult dartHookResult = await DartBuild.loadHookResult(environment);
285277
final Depfile depfile = await copyAssets(
286278
environment,
287279
output,
288-
dartHookResult: dartHookResult,
289280
targetPlatform: TargetPlatform.android,
290281
buildMode: buildMode,
291282
flavor: environment.defines[kFlavor],

0 commit comments

Comments
 (0)