Do not require main asset files if variants are provided (#10901)

packages/flutter_tools/lib/src/asset.dart

@@ -77,7 +77,7 @@ class AssetBundle {
       manifest = _loadFlutterManifest(manifestPath);
     } catch (e) {
       printStatus('Error detected in pubspec.yaml:', emphasis: true);
-      printError(e);
+      printError('$e');
       return 1;
     }
     if (manifest == null) {
@@ -113,8 +113,13 @@ class AssetBundle {
         manifestDescriptor['uses-material-design'];
 
     for (_Asset asset in assetVariants.keys) {
-      assert(asset.assetFileExists);
-      entries[asset.assetEntry] = new DevFSFileContent(asset.assetFile);
+      if (!asset.assetFileExists && assetVariants[asset].isEmpty) {
+        printStatus('Error detected in pubspec.yaml:', emphasis: true);
+        printError('No file or variants found for $asset.\n');
+        return 1;
+      }
+      if (asset.assetFileExists)
+        entries[asset.assetEntry] = new DevFSFileContent(asset.assetFile);
       for (_Asset variant in assetVariants[asset]) {
         assert(variant.assetFileExists);
         entries[variant.assetEntry] = new DevFSFileContent(variant.assetFile);
@@ -313,6 +318,50 @@ DevFSContent _createFontManifest(Map<String, dynamic> manifestDescriptor,
   return new DevFSStringContent(JSON.encode(fonts));
 }
 
+// Given an assets directory like this:
+//
+// assets/foo
+// assets/var1/foo
+// assets/var2/foo
+// assets/bar
+//
+// variantsFor('assets/foo') => ['/assets/var1/foo', '/assets/var2/foo']
+// variantsFor('assets/bar') => []
+class _AssetDirectoryCache {
+  _AssetDirectoryCache(Iterable<String> excluded) {
+    _excluded = excluded.map<String>((String path) => fs.path.absolute(path) + fs.path.separator);
+  }
+
+  Iterable<String> _excluded;
+  final Map<String, Map<String, List<String>>> _cache = <String, Map<String, List<String>>>{};
+
+  List<String> variantsFor(String assetPath) {
+    final String assetName = fs.path.basename(assetPath);
+    final String directory = fs.path.dirname(assetPath);
+
+    if (_cache[directory] == null) {
+      final List<String> paths = <String>[];
+      for (FileSystemEntity entity in fs.directory(directory).listSync(recursive: true)) {
+        final String path = entity.path;
+        if (fs.isFileSync(path) && !_excluded.any((String exclude) => path.startsWith(exclude)))
+          paths.add(path);
+      }
+
+      final Map<String, List<String>> variants = <String, List<String>>{};
+      for (String path in paths) {
+        final String variantName = fs.path.basename(path);
+        if (directory == fs.path.dirname(path))
+          continue;
+        variants[variantName] ??= <String>[];
+        variants[variantName].add(path);
+      }
+      _cache[directory] = variants;
+    }
+
+    return _cache[directory][assetName] ?? const <String>[];
+  }
+}
+
 /// Given an assetBase location and a pubspec.yaml Flutter manifest, return a
 /// map of assets to asset variants.
 ///
@@ -328,45 +377,21 @@ Map<_Asset, List<_Asset>> _parseAssets(
   if (manifestDescriptor == null)
     return result;
 
-  excludeDirs = excludeDirs.map<String>(
-    (String exclude) => fs.path.absolute(exclude) + fs.path.separator
-  ).toList();
-
   if (manifestDescriptor.containsKey('assets')) {
-    for (String asset in manifestDescriptor['assets']) {
-      final _Asset baseAsset = _resolveAsset(packageMap, assetBase, asset);
-
-      if (!baseAsset.assetFileExists) {
-        printError('Error: unable to locate asset entry in pubspec.yaml: "$asset".');
-        return null;
-      }
-
+    final _AssetDirectoryCache cache = new _AssetDirectoryCache(excludeDirs);
+    for (String assetName in manifestDescriptor['assets']) {
+      final _Asset asset = _resolveAsset(packageMap, assetBase, assetName);
       final List<_Asset> variants = <_Asset>[];
-      result[baseAsset] = variants;
-
-      // Find asset variants
-      final String assetPath = baseAsset.assetFile.path;
-      final String assetFilename = fs.path.basename(assetPath);
-      final Directory assetDir = fs.directory(fs.path.dirname(assetPath));
 
-      final List<FileSystemEntity> files = assetDir.listSync(recursive: true);
-
-      for (FileSystemEntity entity in files) {
-        if (!fs.isFileSync(entity.path))
-          continue;
-
-        // Exclude any files in the given directories.
-        if (excludeDirs.any((String exclude) => entity.path.startsWith(exclude)))
-          continue;
-
-        if (fs.path.basename(entity.path) == assetFilename && entity.path != assetPath) {
-          final String key = fs.path.relative(entity.path, from: baseAsset.base);
-          String assetEntry;
-          if (baseAsset.symbolicPrefix != null)
-            assetEntry = fs.path.join(baseAsset.symbolicPrefix, key);
-          variants.add(new _Asset(base: baseAsset.base, assetEntry: assetEntry, relativePath: key));
-        }
+      for (String path in cache.variantsFor(asset.assetFile.path)) {
+        final String key = fs.path.relative(path, from: asset.base);
+        String assetEntry;
+        if (asset.symbolicPrefix != null)
+          assetEntry = fs.path.join(asset.symbolicPrefix, key);
+        variants.add(new _Asset(base: asset.base, assetEntry: assetEntry, relativePath: key));
       }
+
+      result[asset] = variants;
     }
   }
 

packages/flutter_tools/templates/create/pubspec.yaml.tmpl

@@ -25,16 +25,18 @@ flutter:
   # the Icons class.
   uses-material-design: true
 
-  # To add assets to your application, add an assets section here, in
-  # this "flutter" section, as in:
+  # To add assets to your application, add an assets section, like this:
   # assets:
   #  - images/a_dot_burr.jpeg
   #  - images/a_dot_ham.jpeg
 
+  # An image asset can refer to one or more resolution-specific "variants", see
+  # https://flutter.io/assets-and-images/.
+
   # To add assets from package dependencies, first ensure the asset
   # is in the lib/ directory of the dependency. Then,
   # refer to the asset with a path prefixed with
-  # `packages/PACKAGE_NAME/`. Note: the `lib/` is implied, do not
+  # `packages/PACKAGE_NAME/`. The `lib/` is implied, do not
   # include `lib/` in the asset path.
   #
   # Here is an example:

packages/flutter_tools/test/asset_bundle_test.dart

@@ -3,25 +3,20 @@
 // found in the LICENSE file.
 
 import 'dart:convert';
+
+import 'package:file/file.dart';
+
 import 'package:flutter_tools/src/asset.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/devfs.dart';
+
 import 'package:test/test.dart';
 
 import 'src/common.dart';
+import 'src/context.dart';
 
 void main()  {
-  // Create a temporary directory and write a single file into it.
-  final FileSystem fs = const LocalFileSystem();
-  final Directory tempDir = fs.systemTempDirectory.createTempSync();
-  final String projectRoot = tempDir.path;
-  final String assetPath = 'banana.txt';
-  final String assetContents = 'banana';
-  final File tempFile = fs.file(fs.path.join(projectRoot, assetPath));
-  tempFile.parent.createSync(recursive: true);
-  tempFile.writeAsBytesSync(UTF8.encode(assetContents));
-
   setUpAll(() {
     Cache.flutterRoot = getFlutterRoot();
   });
@@ -56,7 +51,17 @@ void main()  {
       expect(archivePaths[0], 'apple.txt');
       expect(archivePaths[1], 'packages/flutter_gallery_assets/shrine/products/heels.png');
     });
-    test('file contents', () async {
+
+    testUsingContext('file contents', () async {
+      // Create a temporary directory and write a single file into it.
+      final Directory tempDir = fs.systemTempDirectory.createTempSync();
+      final String projectRoot = tempDir.path;
+      final String assetPath = 'banana.txt';
+      final String assetContents = 'banana';
+      final File tempFile = fs.file(fs.path.join(projectRoot, assetPath));
+      tempFile.parent.createSync(recursive: true);
+      tempFile.writeAsBytesSync(UTF8.encode(assetContents));
+
       final AssetBundle ab = new AssetBundle.fixed(projectRoot, assetPath);
       expect(ab.entries, isNotEmpty);
       expect(ab.entries.length, 1);
@@ -64,6 +69,8 @@ void main()  {
       final DevFSContent content = ab.entries[archivePath];
       expect(archivePath, assetPath);
       expect(assetContents, UTF8.decode(await content.contentsAsBytes()));
+    }, overrides: <Type, Generator>{
+      FileSystem: () => const LocalFileSystem(),
     });
   });
 
@@ -74,4 +81,5 @@ void main()  {
       expect(ab.entries.length, greaterThan(0));
     });
   });
+
 }

packages/flutter_tools/test/asset_bundle_variant_test.dart

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:convert';
+
+import 'package:file/file.dart';
+import 'package:file/memory.dart';
+
+import 'package:flutter_tools/src/asset.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/cache.dart';
+
+import 'package:test/test.dart';
+
+import 'src/common.dart';
+import 'src/context.dart';
+
+void main()  {
+  group('AssetBundle asset variants', () {
+    testUsingContext('main asset and variants', () async {
+      // Setting flutterRoot here so that it picks up the MemoryFileSystem's
+      // path separator.
+      Cache.flutterRoot = getFlutterRoot();
+
+      fs.file("pubspec.yaml")
+        ..createSync()
+        ..writeAsStringSync(
+'''
+name: test
+dependencies:
+  flutter:
+    sdk: flutter
+flutter:
+  assets:
+    - a/b/c/foo
+'''
+      );
+      fs.file(".packages")..createSync();
+
+      final List<String> assets = <String>[
+        'a/b/c/foo',
+        'a/b/c/var1/foo',
+        'a/b/c/var2/foo',
+        'a/b/c/var3/foo',
+      ];
+      for (String asset in assets) {
+        fs.file(asset)
+          ..createSync(recursive: true)
+          ..writeAsStringSync(asset);
+      }
+
+      AssetBundle bundle = new AssetBundle();
+      await bundle.build(manifestPath: 'pubspec.yaml');
+
+      // The main asset file, /a/b/c/foo, and its variants exist.
+      for (String asset in assets) {
+        expect(bundle.entries.containsKey(asset), true);
+        expect(UTF8.decode(await bundle.entries[asset].contentsAsBytes()), asset);
+      }
+
+      fs.file('/a/b/c/foo').deleteSync();
+      bundle = new AssetBundle();
+      await bundle.build(manifestPath: 'pubspec.yaml');
+
+      // Now the main asset file, /a/b/c/foo, does not exist. This is OK because
+      // the /a/b/c/*/foo variants do exist.
+      expect(bundle.entries.containsKey('/a/b/c/foo'), false);
+      for (String asset in assets.skip(1)) {
+        expect(bundle.entries.containsKey(asset), true);
+        expect(UTF8.decode(await bundle.entries[asset].contentsAsBytes()), asset);
+      }
+    }, overrides: <Type, Generator>{
+      FileSystem: () => new MemoryFileSystem(),
+    });
+
+  });
+}