From 445b5a14f2c7dba95738c52eab696949bfbbda7e Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 1 Apr 2020 10:54:59 -0700 Subject: [PATCH] Precache platform filter change (#53701) Makes the following changes to the behavior of precache: - The --all-platforms flags now fetches all artifacts, rather than just turning off platform filtering of selected artifacts. - Explicitly requested artifacts are no longer subject to platform filtering. E.g., 'precache --ios' will download iOS artifacts on Windows and Linux (but 'precache' without an 'ios' flag will still only download iOS artifacts on macOS). - Desktop platform artifacts now respect the bypassing of platform filtering. Fixes #53272 --- packages/flutter_tools/lib/src/cache.dart | 22 +++- .../lib/src/commands/precache.dart | 53 +++++---- .../hermetic/precache_test.dart | 109 ++++++++++++++++++ .../test/general.shard/cache_test.dart | 43 ++++++- packages/flutter_tools/test/src/testbed.dart | 3 + 5 files changed, 199 insertions(+), 31 deletions(-) diff --git a/packages/flutter_tools/lib/src/cache.dart b/packages/flutter_tools/lib/src/cache.dart index 004ce80d3f0..046b4d50c7a 100644 --- a/packages/flutter_tools/lib/src/cache.dart +++ b/packages/flutter_tools/lib/src/cache.dart @@ -156,6 +156,10 @@ class Cache { // artifacts for the current platform. bool includeAllPlatforms = false; + // Names of artifacts which should be cached even if they would normally + // be filtered out for the current platform. + Set platformOverrideArtifacts; + // Whether to cache the unsigned mac binaries. Defaults to caching the signed binaries. bool useUnsignedMacBinaries = false; @@ -523,6 +527,12 @@ abstract class CachedArtifact extends ArtifactSet { @visibleForTesting final List downloadedFiles = []; + // Whether or not to bypass normal platform filtering for this artifact. + bool get ignorePlatformFiltering { + return cache.includeAllPlatforms || + (cache.platformOverrideArtifacts != null && cache.platformOverrideArtifacts.contains(developmentArtifact.name)); + } + @override bool isUpToDate() { if (!location.existsSync()) { @@ -859,7 +869,7 @@ class MacOSEngineArtifacts extends EngineCachedArtifact { @override List> getBinaryDirs() { - if (globals.platform.isMacOS) { + if (globals.platform.isMacOS || ignorePlatformFiltering) { return _macOSDesktopBinaryDirs; } return const >[]; @@ -881,7 +891,7 @@ class WindowsEngineArtifacts extends EngineCachedArtifact { @override List> getBinaryDirs() { - if (globals.platform.isWindows) { + if (globals.platform.isWindows || ignorePlatformFiltering) { return _windowsDesktopBinaryDirs; } return const >[]; @@ -903,7 +913,7 @@ class LinuxEngineArtifacts extends EngineCachedArtifact { @override List> getBinaryDirs() { - if (globals.platform.isLinux) { + if (globals.platform.isLinux || ignorePlatformFiltering) { return _linuxDesktopBinaryDirs; } return const >[]; @@ -1020,14 +1030,14 @@ class IOSEngineArtifacts extends EngineCachedArtifact { @override List> getBinaryDirs() { return >[ - if (globals.platform.isMacOS || cache.includeAllPlatforms) + if (globals.platform.isMacOS || ignorePlatformFiltering) ..._iosBinaryDirs, ]; } @override List getLicenseDirs() { - if (cache.includeAllPlatforms || globals.platform.isMacOS) { + if (globals.platform.isMacOS || ignorePlatformFiltering) { return const ['ios', 'ios-profile', 'ios-release']; } return const []; @@ -1302,7 +1312,7 @@ class IosUsbArtifacts extends CachedArtifact { @override Future updateInner() { - if (!globals.platform.isMacOS && !cache.includeAllPlatforms) { + if (!globals.platform.isMacOS && !ignorePlatformFiltering) { return Future.value(); } if (location.existsSync()) { diff --git a/packages/flutter_tools/lib/src/commands/precache.dart b/packages/flutter_tools/lib/src/commands/precache.dart index e2d380d59de..11cdb7100b4 100644 --- a/packages/flutter_tools/lib/src/commands/precache.dart +++ b/packages/flutter_tools/lib/src/commands/precache.dart @@ -66,6 +66,33 @@ class PrecacheCommand extends FlutterCommand { ] }; + /// Returns a reverse mapping of _expandedArtifacts, from child artifact name + /// to umbrella name. + Map _umbrellaForArtifactMap() { + return { + for (final MapEntry> entry in _expandedArtifacts.entries) + for (final String childArtifactName in entry.value) + childArtifactName: entry.key + }; + } + + /// Returns the name of all artifacts that were explicitly chosen via flags. + /// + /// If an umbrella is chosen, its children will be included as well. + Set _explicitArtifactSelections() { + final Map umbrellaForArtifact = _umbrellaForArtifactMap(); + final Set selections = {}; + bool explicitlySelected(String name) => boolArg(name) && argResults.wasParsed(name); + for (final DevelopmentArtifact artifact in DevelopmentArtifact.values) { + final String umbrellaName = umbrellaForArtifact[artifact.name]; + if (explicitlySelected(artifact.name) || + (umbrellaName != null && explicitlySelected(umbrellaName))) { + selections.add(artifact.name); + } + } + return selections; + } + @override Future validateCommand() { _expandedArtifacts.forEach((String umbrellaName, List childArtifactNames) { @@ -84,12 +111,15 @@ class PrecacheCommand extends FlutterCommand { @override Future runCommand() async { - if (boolArg('all-platforms')) { + final bool includeAllPlatforms = boolArg('all-platforms'); + if (includeAllPlatforms) { globals.cache.includeAllPlatforms = true; } if (boolArg('use-unsigned-mac-binaries')) { globals.cache.useUnsignedMacBinaries = true; } + globals.cache.platformOverrideArtifacts = _explicitArtifactSelections(); + final Map umbrellaForArtifact = _umbrellaForArtifactMap(); final Set requiredArtifacts = {}; for (final DevelopmentArtifact artifact in DevelopmentArtifact.values) { // Don't include unstable artifacts on stable branches. @@ -100,25 +130,8 @@ class PrecacheCommand extends FlutterCommand { continue; } - bool expandedArtifactProcessed = false; - _expandedArtifacts.forEach((String umbrellaName, List childArtifactNames) { - if (!childArtifactNames.contains(artifact.name)) { - return; - } - expandedArtifactProcessed = true; - - // Expanded artifacts options are true by default. - // Explicitly ignore them if umbrella name is excluded. - // Example: --no-android [--android_gen_snapshot] - if (!boolArg(umbrellaName)) { - return; - } - - // Example: --android [--android_gen_snapshot] - requiredArtifacts.add(artifact); - }); - - if (!expandedArtifactProcessed && boolArg(artifact.name)) { + final String argumentName = umbrellaForArtifact[artifact.name] ?? artifact.name; + if (includeAllPlatforms || boolArg(argumentName)) { requiredArtifacts.add(artifact); } } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart index 681f6601f6e..2316a3ed80c 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/precache_test.dart @@ -8,6 +8,7 @@ import 'package:flutter_tools/src/features.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/version.dart'; import 'package:mockito/mockito.dart'; +import 'package:platform/platform.dart'; import '../../src/common.dart'; import '../../src/context.dart'; @@ -270,6 +271,111 @@ void main() { FeatureFlags: () => TestFeatureFlags(isWebEnabled: false), }); + testUsingContext('precache downloads iOS and Android artifacts by default', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + + await createTestCommandRunner(command).run( + const [ + 'precache', + ], + ); + + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.iOS, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + DevelopmentArtifact.androidInternalBuild, + })); + }, overrides: { + Cache: () => cache, + }); + + testUsingContext('precache --all-platforms gets all artifacts', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + + await createTestCommandRunner(command).run( + const [ + 'precache', + '--all-platforms', + ], + ); + + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.iOS, + DevelopmentArtifact.androidGenSnapshot, + DevelopmentArtifact.androidMaven, + DevelopmentArtifact.androidInternalBuild, + DevelopmentArtifact.web, + DevelopmentArtifact.macOS, + DevelopmentArtifact.linux, + DevelopmentArtifact.windows, + DevelopmentArtifact.fuchsia, + DevelopmentArtifact.flutterRunner, + })); + }, overrides: { + Cache: () => cache, + FeatureFlags: () => TestFeatureFlags( + isWebEnabled: true, + isLinuxEnabled: true, + isMacOSEnabled: true, + isWindowsEnabled: true, + ), + FlutterVersion: () => masterFlutterVersion, + }); + + testUsingContext('precache with default artifacts does not override platform filtering', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + + await createTestCommandRunner(command).run( + const [ + 'precache', + ], + ); + + verify(cache.platformOverrideArtifacts = {}); + }, overrides: { + Cache: () => cache, + FlutterVersion: () => masterFlutterVersion, + }); + + testUsingContext('precache with explicit artifact options overrides platform filtering', () async { + final PrecacheCommand command = PrecacheCommand(); + applyMocksToCommand(command); + + await createTestCommandRunner(command).run( + const [ + 'precache', + '--no-ios', + '--no-android', + '--macos', + ], + ); + + expect(artifacts, unorderedEquals({ + DevelopmentArtifact.universal, + DevelopmentArtifact.macOS, + })); + verify(cache.platformOverrideArtifacts = {'macos'}); + }, overrides: { + Cache: () => cache, + FlutterVersion: () => masterFlutterVersion, + FeatureFlags: () => TestFeatureFlags( + isMacOSEnabled: true, + ), + Platform: () => FakePlatform( + operatingSystem: 'windows', + environment: { + 'FLUTTER_ROOT': 'flutter', + 'FLUTTER_ALREADY_LOCKED': 'true', + }, + ), + }); + testUsingContext('precache downloads artifacts when --force is provided', () async { when(cache.isUpToDate()).thenReturn(true); final PrecacheCommand command = PrecacheCommand(); @@ -285,6 +391,9 @@ void main() { }, overrides: { Cache: () => cache, FlutterVersion: () => flutterVersion, + FeatureFlags: () => TestFeatureFlags( + isMacOSEnabled: true, + ), }); } diff --git a/packages/flutter_tools/test/general.shard/cache_test.dart b/packages/flutter_tools/test/general.shard/cache_test.dart index 8d9c0333b65..9a8b1eb7abf 100644 --- a/packages/flutter_tools/test/general.shard/cache_test.dart +++ b/packages/flutter_tools/test/general.shard/cache_test.dart @@ -520,7 +520,7 @@ void main() { when(mockCache.includeAllPlatforms).thenReturn(false); expect(artifacts.getBinaryDirs(), >[['linux-x64', 'linux-x64/font-subset.zip']]); }, overrides: { - Platform: () => FakePlatform()..operatingSystem = 'linux', + Platform: () => FakePlatform(operatingSystem: 'linux'), }); testUsingContext('FontSubset artifacts on windows', () { @@ -529,7 +529,7 @@ void main() { when(mockCache.includeAllPlatforms).thenReturn(false); expect(artifacts.getBinaryDirs(), >[['windows-x64', 'windows-x64/font-subset.zip']]); }, overrides: { - Platform: () => FakePlatform()..operatingSystem = 'windows', + Platform: () => FakePlatform(operatingSystem: 'windows'), }); testUsingContext('FontSubset artifacts on macos', () { @@ -538,7 +538,7 @@ void main() { when(mockCache.includeAllPlatforms).thenReturn(false); expect(artifacts.getBinaryDirs(), >[['darwin-x64', 'darwin-x64/font-subset.zip']]); }, overrides: { - Platform: () => FakePlatform()..operatingSystem = 'macos', + Platform: () => FakePlatform(operatingSystem: 'macos'), }); testUsingContext('FontSubset artifacts on fuchsia', () { @@ -547,7 +547,7 @@ void main() { when(mockCache.includeAllPlatforms).thenReturn(false); expect(() => artifacts.getBinaryDirs(), throwsToolExit(message: 'Unsupported operating system: ${globals.platform.operatingSystem}')); }, overrides: { - Platform: () => FakePlatform()..operatingSystem = 'fuchsia', + Platform: () => FakePlatform(operatingSystem: 'fuchsia'), }); testUsingContext('FontSubset artifacts for all platforms', () { @@ -560,7 +560,40 @@ void main() { ['windows-x64', 'windows-x64/font-subset.zip'], ]); }, overrides: { - Platform: () => FakePlatform()..operatingSystem = 'fuchsia', + Platform: () => FakePlatform(operatingSystem: 'fuchsia'), + }); + + testUsingContext('macOS desktop artifacts ignore filtering when requested', () { + final MockCache mockCache = MockCache(); + final MacOSEngineArtifacts artifacts = MacOSEngineArtifacts(mockCache); + when(mockCache.includeAllPlatforms).thenReturn(false); + when(mockCache.platformOverrideArtifacts).thenReturn({'macos'}); + + expect(artifacts.getBinaryDirs(), isNotEmpty); + }, overrides: { + Platform: () => FakePlatform(operatingSystem: 'linux'), + }); + + testUsingContext('Windows desktop artifacts ignore filtering when requested', () { + final MockCache mockCache = MockCache(); + final WindowsEngineArtifacts artifacts = WindowsEngineArtifacts(mockCache); + when(mockCache.includeAllPlatforms).thenReturn(false); + when(mockCache.platformOverrideArtifacts).thenReturn({'windows'}); + + expect(artifacts.getBinaryDirs(), isNotEmpty); + }, overrides: { + Platform: () => FakePlatform(operatingSystem: 'linux'), + }); + + testUsingContext('Linux desktop artifacts ignore filtering when requested', () { + final MockCache mockCache = MockCache(); + final LinuxEngineArtifacts artifacts = LinuxEngineArtifacts(mockCache); + when(mockCache.includeAllPlatforms).thenReturn(false); + when(mockCache.platformOverrideArtifacts).thenReturn({'linux'}); + + expect(artifacts.getBinaryDirs(), isNotEmpty); + }, overrides: { + Platform: () => FakePlatform(operatingSystem: 'macos'), }); } diff --git a/packages/flutter_tools/test/src/testbed.dart b/packages/flutter_tools/test/src/testbed.dart index 5302c09688a..408df4e7f27 100644 --- a/packages/flutter_tools/test/src/testbed.dart +++ b/packages/flutter_tools/test/src/testbed.dart @@ -829,6 +829,9 @@ class FakeCache implements Cache { @override bool includeAllPlatforms; + @override + Set platformOverrideArtifacts; + @override bool useUnsignedMacBinaries;