Unverified Commit 4845df90 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] do not include material icon incorrectly (#58335)

If a dependency specified uses-material-design: true and the main pubspec specifies uses-material-design: false, then the MaterialIcons font would be included in the font manifest, but not in the AssetManifest or final bundle. Remove it from the FontManifest if this occurs
parent eb236bd6
...@@ -176,10 +176,12 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -176,10 +176,12 @@ class ManifestAssetBundle implements AssetBundle {
return 1; return 1;
} }
final bool includesMaterialFonts = flutterManifest.usesMaterialDesign;
final List<Map<String, dynamic>> fonts = _parseFonts( final List<Map<String, dynamic>> fonts = _parseFonts(
flutterManifest, flutterManifest,
includeDefaultFonts, includeDefaultFonts,
packageConfig, packageConfig,
primary: true,
); );
// Add fonts and assets from packages. // Add fonts and assets from packages.
...@@ -215,12 +217,20 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -215,12 +217,20 @@ class ManifestAssetBundle implements AssetBundle {
return 1; return 1;
} }
assetVariants.addAll(packageAssets); assetVariants.addAll(packageAssets);
if (!includesMaterialFonts && packageFlutterManifest.usesMaterialDesign) {
globals.printError(
'package:${package.name} has `uses-material-design: true` set but '
'the primary pubspec contains `uses-material-design: false`. '
'If the application needs material icons, then `uses-material-design` '
' must be set to true.'
);
}
fonts.addAll(_parseFonts( fonts.addAll(_parseFonts(
packageFlutterManifest, packageFlutterManifest,
includeDefaultFonts, includeDefaultFonts,
packageConfig, packageConfig,
packageName: package.name, packageName: package.name,
primary: false,
)); ));
} }
} }
...@@ -251,7 +261,6 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -251,7 +261,6 @@ class ManifestAssetBundle implements AssetBundle {
entries[variant.entryUri.path] ??= DevFSFileContent(variant.assetFile); entries[variant.entryUri.path] ??= DevFSFileContent(variant.assetFile);
} }
} }
final List<_Asset> materialAssets = <_Asset>[ final List<_Asset> materialAssets = <_Asset>[
if (flutterManifest.usesMaterialDesign && includeDefaultFonts) if (flutterManifest.usesMaterialDesign && includeDefaultFonts)
..._getMaterialAssets(_kFontSetMaterial), ..._getMaterialAssets(_kFontSetMaterial),
...@@ -523,9 +532,10 @@ List<Map<String, dynamic>> _parseFonts( ...@@ -523,9 +532,10 @@ List<Map<String, dynamic>> _parseFonts(
bool includeDefaultFonts, bool includeDefaultFonts,
PackageConfig packageConfig, { PackageConfig packageConfig, {
String packageName, String packageName,
@required bool primary,
}) { }) {
return <Map<String, dynamic>>[ return <Map<String, dynamic>>[
if (manifest.usesMaterialDesign && includeDefaultFonts) if (primary && manifest.usesMaterialDesign && includeDefaultFonts)
..._getMaterialFonts(ManifestAssetBundle._kFontSetMaterial), ..._getMaterialFonts(ManifestAssetBundle._kFontSetMaterial),
if (packageName == null) if (packageName == null)
...manifest.fontsDescriptor ...manifest.fontsDescriptor
......
...@@ -399,6 +399,44 @@ flutter: ...@@ -399,6 +399,44 @@ flutter:
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(operatingSystem: 'linux'), Platform: () => FakePlatform(operatingSystem: 'linux'),
}); });
testUsingContext('does not include material design assets if uses-material-design: true is '
'specified only by a dependency', () async {
globals.fs.file('.packages').writeAsStringSync(r'''
example:lib/
foo:foo/lib/
''');
globals.fs.file('pubspec.yaml')
..createSync()
..writeAsStringSync(r'''
name: example
dependencies:
foo: any
flutter:
uses-material-design: false
''');
globals.fs.file('foo/pubspec.yaml')
..createSync(recursive: true)
..writeAsStringSync(r'''
name: foo
flutter:
uses-material-design: true
''');
final AssetBundle bundle = AssetBundleFactory.instance.createBundle();
expect(await bundle.build(manifestPath: 'pubspec.yaml'), 0);
expect((bundle.entries['FontManifest.json'] as DevFSStringContent).string, '[]');
expect((bundle.entries['AssetManifest.json'] as DevFSStringContent).string, '{}');
expect(testLogger.errorText, contains(
'package:foo has `uses-material-design: true` set'
));
}, overrides: <Type, Generator>{
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(operatingSystem: 'linux'),
});
} }
class MockDirectory extends Mock implements Directory {} 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