Skip to content

Commit

Permalink
[flutter_tools] fix recursive asset variant issue (flutter#61129)
Browse files Browse the repository at this point in the history
Fixes flutter#45075
Fixes flutter#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.
  • Loading branch information
jonahwilliams authored and mingwandroid committed Sep 6, 2020
1 parent 9a41e8f commit c89d473
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 52 deletions.
39 changes: 25 additions & 14 deletions packages/flutter_tools/lib/src/asset.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/context_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
78 changes: 41 additions & 37 deletions packages/flutter_tools/lib/src/device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {}
33 changes: 33 additions & 0 deletions packages/flutter_tools/test/general.shard/asset_bundle_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

0 comments on commit c89d473

Please sign in to comment.