From 0b95e9c278a564f9758503b26e611374ae5cc0a1 Mon Sep 17 00:00:00 2001 From: Sarah Zakarias Date: Tue, 12 Sep 2017 09:13:41 +0200 Subject: [PATCH] Support transitive asset dependency (#12032) --- packages/flutter_tools/lib/src/asset.dart | 81 +++++---- .../test/asset_bundle_package_test.dart | 168 +++++++++++++++++- 2 files changed, 213 insertions(+), 36 deletions(-) diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart index 24f94d9eaa..62e9a8b02c 100644 --- a/packages/flutter_tools/lib/src/asset.dart +++ b/packages/flutter_tools/lib/src/asset.dart @@ -137,7 +137,7 @@ class AssetBundle { packageMap, packageManifestDescriptor['flutter'], packageBasePath, - packageKey: packageName, + packageName: packageName, )); } } @@ -434,7 +434,7 @@ Map<_Asset, List<_Asset>> _parseAssets( Map manifestDescriptor, String assetBase, { List excludeDirs: const [], - String packageKey + String packageName }) { final Map<_Asset, List<_Asset>> result = <_Asset, List<_Asset>>{}; @@ -444,11 +444,13 @@ Map<_Asset, List<_Asset>> _parseAssets( if (manifestDescriptor.containsKey('assets')) { final _AssetDirectoryCache cache = new _AssetDirectoryCache(excludeDirs); for (String assetName in manifestDescriptor['assets']) { - final _Asset asset = packageKey != null - ? _resolvePackageAsset(assetBase, packageKey, assetName) - : _resolveAsset(packageMap, assetBase, assetName); + final _Asset asset = _resolveAsset( + packageMap, + assetBase, + assetName, + packageName, + ); final List<_Asset> variants = <_Asset>[]; - for (String path in cache.variantsFor(asset.assetFile.path)) { final String key = fs.path.relative(path, from: asset.base); String assetEntry; @@ -473,7 +475,12 @@ Map<_Asset, List<_Asset>> _parseAssets( if (asset == null) continue; - final _Asset baseAsset = _resolveAsset(packageMap, assetBase, asset); + final _Asset baseAsset = _resolveAsset( + packageMap, + assetBase, + asset, + packageName + ); if (!baseAsset.assetFileExists) { printError('Error: unable to locate asset entry in pubspec.yaml: "$asset".'); return null; @@ -487,42 +494,46 @@ Map<_Asset, List<_Asset>> _parseAssets( return result; } -_Asset _resolvePackageAsset( - String assetBase, - String packageName, - String asset, -) { - return new _Asset( - base: assetBase, - assetEntry: 'packages/$packageName/$asset', - relativePath: asset, - ); -} - _Asset _resolveAsset( PackageMap packageMap, String assetBase, String asset, + String packageName, ) { if (asset.startsWith('packages/') && !fs.isFileSync(fs.path.join(assetBase, asset))) { - // Convert packages/flutter_gallery_assets/clouds-0.png to clouds-0.png. - String packageKey = asset.substring(9); - String relativeAsset = asset; - - final int index = packageKey.indexOf('/'); - if (index != -1) { - relativeAsset = packageKey.substring(index + 1); - packageKey = packageKey.substring(0, index); - } - - final Uri uri = packageMap.map[packageKey]; - if (uri != null && uri.scheme == 'file') { - final File file = fs.file(uri); - return new _Asset(base: file.path, assetEntry: asset, relativePath: relativeAsset); - } + // The asset is referenced in the pubspec.yaml as + // 'packages/PACKAGE_NAME/PATH/TO/ASSET + final _Asset packageAsset = _resolvePackageAsset(asset, packageMap); + if (packageAsset != null) + return packageAsset; } - return new _Asset(base: assetBase, relativePath: asset); + final String assetEntry = packageName != null + ? 'packages/$packageName/$asset' // Asset from, and declared in $packageName + : null; // Asset from the current application + return new _Asset(base: assetBase, assetEntry: assetEntry, relativePath: asset); +} + +_Asset _resolvePackageAsset(String asset, PackageMap packageMap) { + assert(asset.startsWith('packages/')); + String packageKey = asset.substring(9); + String relativeAsset = asset; + + final int index = packageKey.indexOf('/'); + if (index != -1) { + relativeAsset = packageKey.substring(index + 1); + packageKey = packageKey.substring(0, index); + } + + final Uri uri = packageMap.map[packageKey]; + if (uri != null && uri.scheme == 'file') { + final File file = fs.file(uri); + final String base = file.path.substring(0, file.path.length - 1); + return new _Asset(base: base, assetEntry: asset, relativePath: relativeAsset); + } + printStatus('Error detected in pubspec.yaml:', emphasis: true); + printError('Could not resolve $packageKey for asset $asset.\n'); + return null; } dynamic _loadFlutterManifest(String manifestPath) { diff --git a/packages/flutter_tools/test/asset_bundle_package_test.dart b/packages/flutter_tools/test/asset_bundle_package_test.dart index a072610e1f..f9a3b397fd 100644 --- a/packages/flutter_tools/test/asset_bundle_package_test.dart +++ b/packages/flutter_tools/test/asset_bundle_package_test.dart @@ -133,6 +133,32 @@ $assetsSection }, overrides: { FileSystem: () => new MemoryFileSystem(), }); + + testUsingContext('One package with one asset not specified', () async { + establishFlutterRoot(); + + final List assetEntries = ['packages/test_package/a/foo']; + writePubspecFile( + 'pubspec.yaml', + 'test', + assets: assetEntries, + ); + writePackagesFile('test_package:p/p/lib/'); + writePubspecFile('p/p/pubspec.yaml', 'test_package'); + + final List assets = ['a/foo']; + writeAssets('p/p/lib/', assets); + + final String expectedAssetManifest = '{"packages/test_package/a/foo":' + '["packages/test_package/a/foo"]}'; + await buildAndVerifyAssets( + assets, + ['test_package'], + expectedAssetManifest, + ); + }, overrides: { + FileSystem: () => new MemoryFileSystem(), + }); testUsingContext('One package with asset variants', () async { establishFlutterRoot(); @@ -160,6 +186,35 @@ $assetsSection FileSystem: () => new MemoryFileSystem(), }); + testUsingContext('One package with asset variants not specified', () async { + establishFlutterRoot(); + + writePubspecFile( + 'pubspec.yaml', + 'test', + assets: ['packages/test_package/a/foo'], + ); + writePackagesFile('test_package:p/p/lib/'); + writePubspecFile( + 'p/p/pubspec.yaml', + 'test_package', + ); + + final List assets = ['a/foo', 'a/v/foo']; + writeAssets('p/p/lib/', assets); + + final String expectedManifest = '{"packages/test_package/a/foo":' + '["packages/test_package/a/foo","packages/test_package/a/v/foo"]}'; + + await buildAndVerifyAssets( + assets, + ['test_package'], + expectedManifest, + ); + }, overrides: { + FileSystem: () => new MemoryFileSystem(), + }); + testUsingContext('One package with two assets', () async { establishFlutterRoot(); @@ -187,10 +242,47 @@ $assetsSection FileSystem: () => new MemoryFileSystem(), }); + testUsingContext('One package with two assets not specified', () async { + establishFlutterRoot(); + + final List assetEntries = [ + 'packages/test_package/a/foo', + 'packages/test_package/a/bar', + ]; + writePubspecFile( + 'pubspec.yaml', + 'test', + assets: assetEntries, + ); + writePackagesFile('test_package:p/p/lib/'); + + final List assets = ['a/foo', 'a/bar']; + writePubspecFile( + 'p/p/pubspec.yaml', + 'test_package', + ); + + writeAssets('p/p/lib/', assets); + final String expectedAssetManifest = + '{"packages/test_package/a/foo":["packages/test_package/a/foo"],' + '"packages/test_package/a/bar":["packages/test_package/a/bar"]}'; + + await buildAndVerifyAssets( + assets, + ['test_package'], + expectedAssetManifest, + ); + }, overrides: { + FileSystem: () => new MemoryFileSystem(), + }); + testUsingContext('Two packages with assets', () async { establishFlutterRoot(); - writePubspecFile('pubspec.yaml', 'test'); + writePubspecFile( + 'pubspec.yaml', + 'test', + ); writePackagesFile('test_package:p/p/lib/\ntest_package2:p2/p/lib/'); writePubspecFile( 'p/p/pubspec.yaml', @@ -221,5 +313,79 @@ $assetsSection }, overrides: { FileSystem: () => new MemoryFileSystem(), }); + + testUsingContext('Two packages with assets not specified', () async { + establishFlutterRoot(); + + final List assetEntries = [ + 'packages/test_package/a/foo', + 'packages/test_package2/a/foo', + ]; + writePubspecFile( + 'pubspec.yaml', + 'test', + assets: assetEntries, + ); + writePackagesFile('test_package:p/p/lib/\ntest_package2:p2/p/lib/'); + writePubspecFile( + 'p/p/pubspec.yaml', + 'test_package', + ); + writePubspecFile( + 'p2/p/pubspec.yaml', + 'test_package2', + ); + + final List assets = ['a/foo', 'a/v/foo']; + writeAssets('p/p/lib/', assets); + writeAssets('p2/p/lib/', assets); + + final String expectedAssetManifest = + '{"packages/test_package/a/foo":' + '["packages/test_package/a/foo","packages/test_package/a/v/foo"],' + '"packages/test_package2/a/foo":' + '["packages/test_package2/a/foo","packages/test_package2/a/v/foo"]}'; + + await buildAndVerifyAssets( + assets, + ['test_package', 'test_package2'], + expectedAssetManifest, + ); + }, overrides: { + FileSystem: () => new MemoryFileSystem(), + }); + + testUsingContext('Transitive asset dependency', () async { + establishFlutterRoot(); + writePubspecFile( + 'pubspec.yaml', + 'test', + ); + writePackagesFile('test_package:p/p/lib/\ntest_package2:p2/p/lib/'); + writePubspecFile( + 'p/p/pubspec.yaml', + 'test_package', + assets: ['packages/test_package2/a/foo'] + ); + writePubspecFile( + 'p2/p/pubspec.yaml', + 'test_package2', + ); + + final List assets = ['a/foo', 'a/v/foo']; + writeAssets('p2/p/lib/', assets); + + final String expectedAssetManifest = + '{"packages/test_package2/a/foo":' + '["packages/test_package2/a/foo","packages/test_package2/a/v/foo"]}'; + + await buildAndVerifyAssets( + assets, + ['test_package2'], + expectedAssetManifest, + ); + }, overrides: { + FileSystem: () => new MemoryFileSystem(), + }); }); }