Unverified Commit afdc4408 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] surface missing assets originating package (#55701)

parent b026c366
...@@ -208,6 +208,7 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -208,6 +208,7 @@ class ManifestAssetBundle implements AssetBundle {
<Uri>[], <Uri>[],
packageBasePath, packageBasePath,
packageName: package.name, packageName: package.name,
attributedPackage: package,
); );
if (packageAssets == null) { if (packageAssets == null) {
...@@ -230,6 +231,9 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -230,6 +231,9 @@ class ManifestAssetBundle implements AssetBundle {
if (!asset.assetFileExists && assetVariants[asset].isEmpty) { if (!asset.assetFileExists && assetVariants[asset].isEmpty) {
globals.printStatus('Error detected in pubspec.yaml:', emphasis: true); globals.printStatus('Error detected in pubspec.yaml:', emphasis: true);
globals.printError('No file or variants found for $asset.\n'); globals.printError('No file or variants found for $asset.\n');
if (asset.package != null) {
globals.printError('This asset was included from package ${asset.package.name}.');
}
return 1; return 1;
} }
// The file name for an asset's "main" entry is whatever appears in // The file name for an asset's "main" entry is whatever appears in
...@@ -291,10 +295,12 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -291,10 +295,12 @@ class ManifestAssetBundle implements AssetBundle {
@immutable @immutable
class _Asset { class _Asset {
const _Asset({ this.baseDir, this.relativeUri, this.entryUri }); const _Asset({ this.baseDir, this.relativeUri, this.entryUri, @required this.package });
final String baseDir; final String baseDir;
final Package package;
/// A platform-independent URL where this asset can be found on disk on the /// A platform-independent URL where this asset can be found on disk on the
/// host system relative to [baseDir]. /// host system relative to [baseDir].
final Uri relativeUri; final Uri relativeUri;
...@@ -367,6 +373,7 @@ List<_Asset> _getMaterialAssets(String fontSet) { ...@@ -367,6 +373,7 @@ List<_Asset> _getMaterialAssets(String fontSet) {
baseDir: globals.fs.path.join(Cache.flutterRoot, 'bin', 'cache', 'artifacts', 'material_fonts'), baseDir: globals.fs.path.join(Cache.flutterRoot, 'bin', 'cache', 'artifacts', 'material_fonts'),
relativeUri: Uri(path: entryUri.pathSegments.last), relativeUri: Uri(path: entryUri.pathSegments.last),
entryUri: entryUri, entryUri: entryUri,
package: null,
)); ));
} }
} }
...@@ -638,6 +645,7 @@ Map<_Asset, List<_Asset>> _parseAssets( ...@@ -638,6 +645,7 @@ Map<_Asset, List<_Asset>> _parseAssets(
String assetBase, { String assetBase, {
List<String> excludeDirs = const <String>[], List<String> excludeDirs = const <String>[],
String packageName, String packageName,
Package attributedPackage,
}) { }) {
final Map<_Asset, List<_Asset>> result = <_Asset, List<_Asset>>{}; final Map<_Asset, List<_Asset>> result = <_Asset, List<_Asset>>{};
...@@ -645,13 +653,29 @@ Map<_Asset, List<_Asset>> _parseAssets( ...@@ -645,13 +653,29 @@ Map<_Asset, List<_Asset>> _parseAssets(
for (final Uri assetUri in flutterManifest.assets) { for (final Uri assetUri in flutterManifest.assets) {
if (assetUri.toString().endsWith('/')) { if (assetUri.toString().endsWith('/')) {
wildcardDirectories.add(assetUri); wildcardDirectories.add(assetUri);
_parseAssetsFromFolder(packageConfig, flutterManifest, assetBase, _parseAssetsFromFolder(
cache, result, assetUri, packageConfig,
excludeDirs: excludeDirs, packageName: packageName); flutterManifest,
assetBase,
cache,
result,
assetUri,
excludeDirs: excludeDirs,
packageName: packageName,
attributedPackage: attributedPackage,
);
} else { } else {
_parseAssetFromFile(packageConfig, flutterManifest, assetBase, _parseAssetFromFile(
cache, result, assetUri, packageConfig,
excludeDirs: excludeDirs, packageName: packageName); flutterManifest,
assetBase,
cache,
result,
assetUri,
excludeDirs: excludeDirs,
packageName: packageName,
attributedPackage: attributedPackage,
);
} }
} }
...@@ -663,6 +687,7 @@ Map<_Asset, List<_Asset>> _parseAssets( ...@@ -663,6 +687,7 @@ Map<_Asset, List<_Asset>> _parseAssets(
assetBase, assetBase,
fontAsset.assetUri, fontAsset.assetUri,
packageName, packageName,
attributedPackage,
); );
if (!baseAsset.assetFileExists) { if (!baseAsset.assetFileExists) {
globals.printError('Error: unable to locate asset entry in pubspec.yaml: "${fontAsset.assetUri}".'); globals.printError('Error: unable to locate asset entry in pubspec.yaml: "${fontAsset.assetUri}".');
...@@ -685,6 +710,7 @@ void _parseAssetsFromFolder( ...@@ -685,6 +710,7 @@ void _parseAssetsFromFolder(
Uri assetUri, { Uri assetUri, {
List<String> excludeDirs = const <String>[], List<String> excludeDirs = const <String>[],
String packageName, String packageName,
Package attributedPackage,
}) { }) {
final String directoryPath = globals.fs.path.join( final String directoryPath = globals.fs.path.join(
assetBase, assetUri.toFilePath(windows: globals.platform.isWindows)); assetBase, assetUri.toFilePath(windows: globals.platform.isWindows));
...@@ -699,11 +725,18 @@ void _parseAssetsFromFolder( ...@@ -699,11 +725,18 @@ void _parseAssetsFromFolder(
for (final FileSystemEntity entity in lister) { for (final FileSystemEntity entity in lister) {
if (entity is File) { if (entity is File) {
final String relativePath = globals.fs.path.relative(entity.path, from: assetBase); final String relativePath = globals.fs.path.relative(entity.path, from: assetBase);
final Uri uri = Uri.file(relativePath, windows: globals.platform.isWindows); final Uri uri = Uri.file(relativePath, windows: globals.platform.isWindows);
_parseAssetFromFile(packageConfig, flutterManifest, assetBase, cache, result, _parseAssetFromFile(
uri, packageName: packageName); packageConfig,
flutterManifest,
assetBase,
cache,
result,
uri,
packageName: packageName,
attributedPackage: attributedPackage,
);
} }
} }
} }
...@@ -717,12 +750,14 @@ void _parseAssetFromFile( ...@@ -717,12 +750,14 @@ void _parseAssetFromFile(
Uri assetUri, { Uri assetUri, {
List<String> excludeDirs = const <String>[], List<String> excludeDirs = const <String>[],
String packageName, String packageName,
Package attributedPackage,
}) { }) {
final _Asset asset = _resolveAsset( final _Asset asset = _resolveAsset(
packageConfig, packageConfig,
assetBase, assetBase,
assetUri, assetUri,
packageName, packageName,
attributedPackage,
); );
final List<_Asset> variants = <_Asset>[]; final List<_Asset> variants = <_Asset>[];
for (final String path in cache.variantsFor(asset.assetFile.path)) { for (final String path in cache.variantsFor(asset.assetFile.path)) {
...@@ -737,6 +772,7 @@ void _parseAssetFromFile( ...@@ -737,6 +772,7 @@ void _parseAssetFromFile(
baseDir: asset.baseDir, baseDir: asset.baseDir,
entryUri: entryUri, entryUri: entryUri,
relativeUri: relativeUri, relativeUri: relativeUri,
package: attributedPackage,
), ),
); );
} }
...@@ -749,12 +785,17 @@ _Asset _resolveAsset( ...@@ -749,12 +785,17 @@ _Asset _resolveAsset(
String assetsBaseDir, String assetsBaseDir,
Uri assetUri, Uri assetUri,
String packageName, String packageName,
Package attributedPackage,
) { ) {
final String assetPath = globals.fs.path.fromUri(assetUri); final String assetPath = globals.fs.path.fromUri(assetUri);
if (assetUri.pathSegments.first == 'packages' && !globals.fs.isFileSync(globals.fs.path.join(assetsBaseDir, assetPath))) { if (assetUri.pathSegments.first == 'packages' && !globals.fs.isFileSync(globals.fs.path.join(assetsBaseDir, assetPath))) {
// The asset is referenced in the pubspec.yaml as // The asset is referenced in the pubspec.yaml as
// 'packages/PACKAGE_NAME/PATH/TO/ASSET . // 'packages/PACKAGE_NAME/PATH/TO/ASSET .
final _Asset packageAsset = _resolvePackageAsset(assetUri, packageConfig); final _Asset packageAsset = _resolvePackageAsset(
assetUri,
packageConfig,
attributedPackage,
);
if (packageAsset != null) { if (packageAsset != null) {
return packageAsset; return packageAsset;
} }
...@@ -766,23 +807,29 @@ _Asset _resolveAsset( ...@@ -766,23 +807,29 @@ _Asset _resolveAsset(
? assetUri // Asset from the current application. ? assetUri // Asset from the current application.
: Uri(pathSegments: <String>['packages', packageName, ...assetUri.pathSegments]), // Asset from, and declared in $packageName. : Uri(pathSegments: <String>['packages', packageName, ...assetUri.pathSegments]), // Asset from, and declared in $packageName.
relativeUri: assetUri, relativeUri: assetUri,
package: attributedPackage,
); );
} }
_Asset _resolvePackageAsset(Uri assetUri, PackageConfig packageConfig) { _Asset _resolvePackageAsset(Uri assetUri, PackageConfig packageConfig, Package attributedPackage) {
assert(assetUri.pathSegments.first == 'packages'); assert(assetUri.pathSegments.first == 'packages');
if (assetUri.pathSegments.length > 1) { if (assetUri.pathSegments.length > 1) {
final String packageName = assetUri.pathSegments[1]; final String packageName = assetUri.pathSegments[1];
final Uri packageUri = packageConfig[packageName]?.packageUriRoot; final Package package = packageConfig[packageName];
final Uri packageUri = package?.packageUriRoot;
if (packageUri != null && packageUri.scheme == 'file') { if (packageUri != null && packageUri.scheme == 'file') {
return _Asset( return _Asset(
baseDir: globals.fs.path.fromUri(packageUri), baseDir: globals.fs.path.fromUri(packageUri),
entryUri: assetUri, entryUri: assetUri,
relativeUri: Uri(pathSegments: assetUri.pathSegments.sublist(2)), relativeUri: Uri(pathSegments: assetUri.pathSegments.sublist(2)),
package: attributedPackage,
); );
} }
} }
globals.printStatus('Error detected in pubspec.yaml:', emphasis: true); globals.printStatus('Error detected in pubspec.yaml:', emphasis: true);
globals.printError('Could not resolve package for asset $assetUri.\n'); globals.printError('Could not resolve package for asset $assetUri.\n');
if (attributedPackage != null) {
globals.printError('This asset was included from package ${attributedPackage.name}');
}
return null; return null;
} }
...@@ -173,7 +173,8 @@ $assetsSection ...@@ -173,7 +173,8 @@ $assetsSection
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
}); });
testUsingContext('One asset is bundled when the package has and lists one asset its pubspec', () async { testUsingContext('One asset is bundled when the package has and lists one '
'asset its pubspec', () async {
establishFlutterRoot(); establishFlutterRoot();
writeEmptySchemaFile(globals.fs); writeEmptySchemaFile(globals.fs);
...@@ -201,7 +202,8 @@ $assetsSection ...@@ -201,7 +202,8 @@ $assetsSection
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
}); });
testUsingContext("One asset is bundled when the package has one asset, listed in the app's pubspec", () async { testUsingContext('One asset is bundled when the package has one asset, '
"listed in the app's pubspec", () async {
establishFlutterRoot(); establishFlutterRoot();
writeEmptySchemaFile(globals.fs); writeEmptySchemaFile(globals.fs);
...@@ -229,7 +231,8 @@ $assetsSection ...@@ -229,7 +231,8 @@ $assetsSection
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
}); });
testUsingContext('One asset and its variant are bundled when the package has an asset and a variant, and lists the asset in its pubspec', () async { testUsingContext('One asset and its variant are bundled when the package '
'has an asset and a variant, and lists the asset in its pubspec', () async {
establishFlutterRoot(); establishFlutterRoot();
writeEmptySchemaFile(globals.fs); writeEmptySchemaFile(globals.fs);
...@@ -257,7 +260,8 @@ $assetsSection ...@@ -257,7 +260,8 @@ $assetsSection
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
}); });
testUsingContext('One asset and its variant are bundled when the package has an asset and a variant, and the app lists the asset in its pubspec', () async { testUsingContext('One asset and its variant are bundled when the package '
'has an asset and a variant, and the app lists the asset in its pubspec', () async {
establishFlutterRoot(); establishFlutterRoot();
writeEmptySchemaFile(globals.fs); writeEmptySchemaFile(globals.fs);
...@@ -288,7 +292,8 @@ $assetsSection ...@@ -288,7 +292,8 @@ $assetsSection
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
}); });
testUsingContext('Two assets are bundled when the package has and lists two assets in its pubspec', () async { testUsingContext('Two assets are bundled when the package has and lists '
'two assets in its pubspec', () async {
establishFlutterRoot(); establishFlutterRoot();
writeEmptySchemaFile(globals.fs); writeEmptySchemaFile(globals.fs);
...@@ -436,7 +441,8 @@ $assetsSection ...@@ -436,7 +441,8 @@ $assetsSection
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
}); });
testUsingContext('One asset is bundled when the app depends on a package, listing in its pubspec an asset from another package', () async { testUsingContext('One asset is bundled when the app depends on a package, '
'listing in its pubspec an asset from another package', () async {
establishFlutterRoot(); establishFlutterRoot();
writeEmptySchemaFile(globals.fs); writeEmptySchemaFile(globals.fs);
writePubspecFile( writePubspecFile(
...@@ -584,7 +590,9 @@ $assetsSection ...@@ -584,7 +590,9 @@ $assetsSection
final AssetBundle bundle = AssetBundleFactory.instance.createBundle(); final AssetBundle bundle = AssetBundleFactory.instance.createBundle();
await bundle.build(manifestPath: 'pubspec.yaml'); await bundle.build(manifestPath: 'pubspec.yaml');
assert(bundle.entries['AssetManifest.json'] == null,'Invalid pubspec.yaml should not generate AssetManifest.json' );
expect(bundle.entries['AssetManifest.json'], isNull,
reason: 'Invalid pubspec.yaml should not generate AssetManifest.json' );
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FileSystem: () => testFileSystem, FileSystem: () => testFileSystem,
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
......
...@@ -265,6 +265,63 @@ flutter: ...@@ -265,6 +265,63 @@ flutter:
ProcessManager: () => FakeProcessManager.any(), ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(operatingSystem: 'linux'), Platform: () => FakePlatform(operatingSystem: 'linux'),
}); });
testUsingContext('reports package that causes asset bundle error when it is '
'a dependency', () async {
globals.fs.file('.packages').writeAsStringSync(r'''
example:lib/
foo:foo/lib/
''');
globals.fs.file(globals.fs.path.join('assets', 'foo', 'bar.txt'))
.createSync(recursive: true);
globals.fs.file('pubspec.yaml')
..createSync()
..writeAsStringSync(r'''
name: example
dependencies:
foo: any
''');
globals.fs.file('foo/pubspec.yaml')
..createSync(recursive: true)
..writeAsStringSync(r'''
name: foo
flutter:
assets:
- bar.txt
''');
final AssetBundle bundle = AssetBundleFactory.instance.createBundle();
expect(await bundle.build(manifestPath: 'pubspec.yaml'), 1);
expect(testLogger.errorText, contains('This asset was included from package foo'));
}, overrides: <Type, Generator>{
FileSystem: () => MemoryFileSystem.test(),
ProcessManager: () => FakeProcessManager.any(),
Platform: () => FakePlatform(operatingSystem: 'linux'),
});
testUsingContext('does not report package that causes asset bundle error '
'when it is from own pubspec', () async {
globals.fs.file('.packages').writeAsStringSync(r'''
example:lib/
''');
globals.fs.file('pubspec.yaml')
..createSync()
..writeAsStringSync(r'''
name: example
flutter:
assets:
- bar.txt
''');
final AssetBundle bundle = AssetBundleFactory.instance.createBundle();
expect(await bundle.build(manifestPath: 'pubspec.yaml'), 1);
expect(testLogger.errorText, isNot(contains('This asset was included from')));
}, 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