From c89d473031e47c63c9b56836bf3c306f191d5593 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 8 Jul 2020 18:07:27 -0700 Subject: [PATCH] [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. --- packages/flutter_tools/lib/src/asset.dart | 39 ++++++---- .../flutter_tools/lib/src/context_runner.dart | 2 +- packages/flutter_tools/lib/src/device.dart | 78 ++++++++++--------- .../commands.shard/hermetic/devices_test.dart | 6 ++ .../test/general.shard/asset_bundle_test.dart | 33 ++++++++ 5 files changed, 106 insertions(+), 52 deletions(-) diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart index 0473f18e50daf..7cb9b4879b0a7 100644 --- a/packages/flutter_tools/lib/src/asset.dart +++ b/packages/flutter_tools/lib/src/asset.dart @@ -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: [assetDirPath, getBuildDirectory()], + excludeDirs: [ + 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> _createFontsDescriptor(List fonts) { // variantsFor('assets/foo') => ['/assets/var1/foo', '/assets/var2/foo'] // variantsFor('assets/bar') => [] class _AssetDirectoryCache { - _AssetDirectoryCache(Iterable excluded) { - _excluded = excluded.map((String path) => globals.fs.path.absolute(path) + globals.fs.path.separator); - } + _AssetDirectoryCache(Iterable excluded) + : _excluded = excluded + .map(globals.fs.path.absolute) + .toList(); - Iterable _excluded; + final List _excluded; final Map>> _cache = >>{}; List variantsFor(String assetPath) { @@ -613,7 +622,9 @@ class _AssetDirectoryCache { final List paths = []; 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); } } diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index dd714b2d58fe3..43a374825fbd7 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -124,7 +124,7 @@ Future runInContext( client: globals.httpClientFactory?.call() ?? HttpClient(), ), DevFSConfig: () => DevFSConfig(), - DeviceManager: () => DeviceManager(), + DeviceManager: () => FlutterDeviceManager(), Doctor: () => Doctor(logger: globals.logger), DoctorValidatorsProvider: () => DoctorValidatorsProvider.defaultInstance, EmulatorManager: () => EmulatorManager( diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index db157cd1178bc..2b9912a3f834f 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -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 get deviceDiscoverers => _deviceDiscoverers; - final List _deviceDiscoverers = List.unmodifiable([ - 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 get deviceDiscoverers; String _specifiedDeviceId; @@ -303,6 +268,45 @@ class DeviceManager { } } +class FlutterDeviceManager extends DeviceManager { + @override + final List deviceDiscoverers = [ + 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; diff --git a/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart index 0e6dd41e4e1be..21d09e2faaf25 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/devices_test.dart @@ -176,6 +176,9 @@ class _FakeDeviceManager extends DeviceManager { Future> getDeviceDiagnostics() => Future>.value( ['Cannot connect to device ABC'] ); + + @override + List get deviceDiscoverers => []; } class NoDevicesManager extends DeviceManager { @@ -185,6 +188,9 @@ class NoDevicesManager extends DeviceManager { @override Future> refreshAllConnectedDevices({Duration timeout}) => getAllConnectedDevices(); + +@override + List get deviceDiscoverers => []; } class MockCache extends Mock implements Cache {} diff --git a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart index 155a6f5240ca3..3a9f3eddab95a 100644 --- a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart +++ b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart @@ -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: { + FileSystem: () => MemoryFileSystem.test(), + ProcessManager: () => FakeProcessManager.any(), + Platform: () => FakePlatform(operatingSystem: 'linux'), + }); } class MockDirectory extends Mock implements Directory {}