Unverified Commit 741608a2 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] fix recursive asset variant issue (#61129)

Fixes #45075
Fixes #57210

If an asset was included directly from the project root directory, then the same asset when copied to various output or ephemeral directories would also be picked up as an asset variant. This could cause assets to be recursively copied into asset/build/ephemeral directories, as each time it would run it would pick up all of the previous "variants".

The solution is to include project ephemeral directories, in addition to the build directory.
parent ad247e6b
......@@ -18,6 +18,7 @@ import 'dart/package_map.dart';
import 'devfs.dart';
import 'flutter_manifest.dart';
import 'globals.dart' as globals;
import 'project.dart';
const AssetBundleFactory _kManifestFactory = _ManifestAssetBundleFactory();
......@@ -127,22 +128,18 @@ class ManifestAssetBundle implements AssetBundle {
bool reportLicensedPackages = false,
}) async {
assetDirPath ??= getAssetBuildDirectory();
FlutterManifest flutterManifest;
FlutterProject flutterProject;
try {
flutterManifest = FlutterManifest.createFromPath(
manifestPath,
logger: globals.logger,
fileSystem: globals.fs,
);
flutterProject = FlutterProject.fromDirectory(globals.fs.file(manifestPath).parent);
} on Exception catch (e) {
globals.printStatus('Error detected in pubspec.yaml:', emphasis: true);
globals.printError('$e');
return 1;
}
if (flutterManifest == null) {
if (flutterProject == null) {
return 1;
}
final FlutterManifest flutterManifest = flutterProject.manifest;
// If the last build time isn't set before this early return, empty pubspecs will
// hang on hot reload, as the incremental dill files will never be copied to the
// device.
......@@ -168,7 +165,18 @@ class ManifestAssetBundle implements AssetBundle {
flutterManifest,
wildcardDirectories,
assetBasePath,
excludeDirs: <String>[assetDirPath, getBuildDirectory()],
excludeDirs: <String>[
assetDirPath,
getBuildDirectory(),
if (flutterProject.ios.existsSync())
flutterProject.ios.hostAppRoot.path,
if (flutterProject.macos.existsSync())
flutterProject.macos.managedDirectory.path,
if (flutterProject.windows.existsSync())
flutterProject.windows.managedDirectory.path,
if (flutterProject.linux.existsSync())
flutterProject.linux.managedDirectory.path,
],
);
if (assetVariants == null) {
......@@ -594,11 +602,12 @@ List<Map<String, dynamic>> _createFontsDescriptor(List<Font> fonts) {
// variantsFor('assets/foo') => ['/assets/var1/foo', '/assets/var2/foo']
// variantsFor('assets/bar') => []
class _AssetDirectoryCache {
_AssetDirectoryCache(Iterable<String> excluded) {
_excluded = excluded.map<String>((String path) => globals.fs.path.absolute(path) + globals.fs.path.separator);
}
_AssetDirectoryCache(Iterable<String> excluded)
: _excluded = excluded
.map<String>(globals.fs.path.absolute)
.toList();
Iterable<String> _excluded;
final List<String> _excluded;
final Map<String, Map<String, List<String>>> _cache = <String, Map<String, List<String>>>{};
List<String> variantsFor(String assetPath) {
......@@ -613,7 +622,9 @@ class _AssetDirectoryCache {
final List<String> paths = <String>[];
for (final FileSystemEntity entity in globals.fs.directory(directory).listSync(recursive: true)) {
final String path = entity.path;
if (globals.fs.isFileSync(path) && !_excluded.any((String exclude) => path.startsWith(exclude))) {
if (globals.fs.isFileSync(path)
&& assetPath != path
&& !_excluded.any((String exclude) => globals.fs.path.isWithin(exclude, path))) {
paths.add(path);
}
}
......
......@@ -124,7 +124,7 @@ Future<T> runInContext<T>(
client: globals.httpClientFactory?.call() ?? HttpClient(),
),
DevFSConfig: () => DevFSConfig(),
DeviceManager: () => DeviceManager(),
DeviceManager: () => FlutterDeviceManager(),
Doctor: () => Doctor(logger: globals.logger),
DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance,
EmulatorManager: () => EmulatorManager(
......
......@@ -67,46 +67,11 @@ class PlatformType {
}
/// A class to get all available devices.
class DeviceManager {
abstract class DeviceManager {
/// Constructing DeviceManagers is cheap; they only do expensive work if some
/// of their methods are called.
List<DeviceDiscovery> get deviceDiscoverers => _deviceDiscoverers;
final List<DeviceDiscovery> _deviceDiscoverers = List<DeviceDiscovery>.unmodifiable(<DeviceDiscovery>[
AndroidDevices(
logger: globals.logger,
androidSdk: globals.androidSdk,
androidWorkflow: androidWorkflow,
processManager: globals.processManager,
),
IOSDevices(
platform: globals.platform,
xcdevice: globals.xcdevice,
iosWorkflow: globals.iosWorkflow,
logger: globals.logger,
),
IOSSimulators(iosSimulatorUtils: globals.iosSimulatorUtils),
FuchsiaDevices(
fuchsiaSdk: fuchsiaSdk,
logger: globals.logger,
fuchsiaWorkflow: fuchsiaWorkflow,
platform: globals.platform,
),
FlutterTesterDevices(),
MacOSDevices(),
LinuxDevices(
platform: globals.platform,
featureFlags: featureFlags,
),
WindowsDevices(),
WebDevices(
featureFlags: featureFlags,
fileSystem: globals.fs,
platform: globals.platform,
processManager: globals.processManager,
logger: globals.logger,
),
]);
List<DeviceDiscovery> get deviceDiscoverers;
String _specifiedDeviceId;
......@@ -303,6 +268,45 @@ class DeviceManager {
}
}
class FlutterDeviceManager extends DeviceManager {
@override
final List<DeviceDiscovery> deviceDiscoverers = <DeviceDiscovery>[
AndroidDevices(
logger: globals.logger,
androidSdk: globals.androidSdk,
androidWorkflow: androidWorkflow,
processManager: globals.processManager,
),
IOSDevices(
platform: globals.platform,
xcdevice: globals.xcdevice,
iosWorkflow: globals.iosWorkflow,
logger: globals.logger,
),
IOSSimulators(iosSimulatorUtils: globals.iosSimulatorUtils),
FuchsiaDevices(
fuchsiaSdk: fuchsiaSdk,
logger: globals.logger,
fuchsiaWorkflow: fuchsiaWorkflow,
platform: globals.platform,
),
FlutterTesterDevices(),
MacOSDevices(),
LinuxDevices(
platform: globals.platform,
featureFlags: featureFlags,
),
WindowsDevices(),
WebDevices(
featureFlags: featureFlags,
fileSystem: globals.fs,
platform: globals.platform,
processManager: globals.processManager,
logger: globals.logger,
),
];
}
/// An abstract class to discover and enumerate a specific type of devices.
abstract class DeviceDiscovery {
bool get supportsPlatform;
......
......@@ -176,6 +176,9 @@ class _FakeDeviceManager extends DeviceManager {
Future<List<String>> getDeviceDiagnostics() => Future<List<String>>.value(
<String>['Cannot connect to device ABC']
);
@override
List<DeviceDiscovery> get deviceDiscoverers => <DeviceDiscovery>[];
}
class NoDevicesManager extends DeviceManager {
......@@ -185,6 +188,9 @@ class NoDevicesManager extends DeviceManager {
@override
Future<List<Device>> refreshAllConnectedDevices({Duration timeout}) =>
getAllConnectedDevices();
@override
List<DeviceDiscovery> get deviceDiscoverers => <DeviceDiscovery>[];
}
class MockCache extends Mock implements Cache {}
......@@ -437,6 +437,39 @@ flutter:
ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(operatingSystem: 'linux'),
});
testUsingContext('does not include assets in project directories as asset variants', () async {
globals.fs.file('.packages').writeAsStringSync(r'''
example:lib/
''');
globals.fs.file('pubspec.yaml')
..createSync()
..writeAsStringSync(r'''
name: example
flutter:
assets:
- foo.txt
''');
globals.fs.file('assets/foo.txt').createSync(recursive: true);
// Potential build artifacts outisde of build directory.
globals.fs.file('linux/flutter/foo.txt').createSync(recursive: true);
globals.fs.file('windows/flutter/foo.txt').createSync(recursive: true);
globals.fs.file('windows/CMakeLists.txt').createSync();
globals.fs.file('macos/Flutter/foo.txt').createSync(recursive: true);
globals.fs.file('ios/foo.txt').createSync(recursive: true);
globals.fs.file('build/foo.txt').createSync(recursive: true);
final AssetBundle bundle = AssetBundleFactory.instance.createBundle();
expect(await bundle.build(manifestPath: 'pubspec.yaml', packagesPath: '.packages'), 0);
expect(bundle.entries.length, 4);
}, overrides: <Type, Generator>{
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(operatingSystem: 'linux'),
});
}
class MockDirectory extends Mock implements Directory {}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment