[flutter_tools] include LICENSE files as build dependencies (#50945)

packages/flutter_tools/lib/src/asset.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:meta/meta.dart';
 import 'package:yaml/yaml.dart';
 
 import 'base/context.dart';
@@ -37,6 +38,10 @@ abstract class AssetBundleFactory {
 abstract class AssetBundle {
   Map<String, DevFSContent> get entries;
 
+  /// Additional files that this bundle depends on that are not included in the
+  /// output result.
+  List<File> get additionalDependencies;
+
   bool wasBuiltOnce();
 
   bool needsBuild({ String manifestPath = defaultManifestPath });
@@ -70,6 +75,8 @@ class _ManifestAssetBundle implements AssetBundle {
   // updated without changes to the manifest.
   final Map<Uri, Directory> _wildcardDirectories = <Uri, Directory>{};
 
+  final LicenseCollector licenseCollector = LicenseCollector(fileSystem: globals.fs);
+
   DateTime _lastBuildTimestamp;
 
   static const String _assetManifestJson = 'AssetManifest.json';
@@ -246,10 +253,15 @@ class _ManifestAssetBundle implements AssetBundle {
     entries[kFontManifestJson] = DevFSStringContent(json.encode(fonts));
 
     // TODO(ianh): Only do the following line if we've changed packages or if our LICENSE file changed
-    entries[_license] = _obtainLicenses(packageMap, assetBasePath, reportPackages: reportLicensedPackages);
+    final LicenseResult licenseResult = licenseCollector.obtainLicenses(packageMap);
+    entries[_license] = DevFSStringContent(licenseResult.combinedLicenses);
+    additionalDependencies = licenseResult.dependencies;
 
     return 0;
   }
+
+  @override
+  List<File> additionalDependencies = <File>[];
 }
 
 class _Asset {
@@ -336,78 +348,108 @@ List<_Asset> _getMaterialAssets(String fontSet) {
   return result;
 }
 
-final String _licenseSeparator = '\n' + ('-' * 80) + '\n';
 
-/// Returns a DevFSContent representing the license file.
-DevFSContent _obtainLicenses(
-  PackageMap packageMap,
-  String assetBase, {
-  bool reportPackages,
-}) {
-  // Read the LICENSE file from each package in the .packages file, splitting
-  // each one into each component license (so that we can de-dupe if possible).
-  //
-  // Individual licenses inside each LICENSE file should be separated by 80
-  // hyphens on their own on a line.
-  //
-  // If a LICENSE file contains more than one component license, then each
-  // component license must start with the names of the packages to which the
-  // component license applies, with each package name on its own line, and the
-  // list of package names separated from the actual license text by a blank
-  // line. (The packages need not match the names of the pub package. For
-  // example, a package might itself contain code from multiple third-party
-  // sources, and might need to include a license for each one.)
-  final Map<String, Set<String>> packageLicenses = <String, Set<String>>{};
-  final Set<String> allPackages = <String>{};
-  for (final String packageName in packageMap.map.keys) {
-    final Uri package = packageMap.map[packageName];
-    if (package == null || package.scheme != 'file') {
-      continue;
-    }
-    final File file = globals.fs.file(package.resolve('../LICENSE'));
-    if (!file.existsSync()) {
-      continue;
-    }
-    final List<String> rawLicenses =
-        file.readAsStringSync().split(_licenseSeparator);
-    for (final String rawLicense in rawLicenses) {
-      List<String> packageNames;
-      String licenseText;
-      if (rawLicenses.length > 1) {
-        final int split = rawLicense.indexOf('\n\n');
-        if (split >= 0) {
-          packageNames = rawLicense.substring(0, split).split('\n');
-          licenseText = rawLicense.substring(split + 2);
-        }
+/// Processes dependencies into a string representing the license file.
+///
+/// Reads the LICENSE file from each package in the .packages file, splitting
+/// each one into each component license (so that we can de-dupe if possible).
+/// If provided with a pubspec.yaml file, only direct depedencies are included
+/// in the resulting LICENSE file.
+///
+/// Individual licenses inside each LICENSE file should be separated by 80
+/// hyphens on their own on a line.
+///
+/// If a LICENSE file contains more than one component license, then each
+/// component license must start with the names of the packages to which the
+/// component license applies, with each package name on its own line, and the
+/// list of package names separated from the actual license text by a blank
+/// line. (The packages need not match the names of the pub package. For
+/// example, a package might itself contain code from multiple third-party
+/// sources, and might need to include a license for each one.)
+// Note: this logic currently has a bug, in that we collect LICENSE information
+// for dev_dependencies and transitive dev_dependencies. These are not actually
+// compiled into the released application and don't need to be included. Unfortunately,
+// the pubspec.yaml alone does not have enough information to determine which
+// dependencies are transitive of dev_dependencies, so a simple filter isn't sufficient.
+class LicenseCollector {
+  LicenseCollector({
+    @required FileSystem fileSystem
+  }) : _fileSystem = fileSystem;
+
+  final FileSystem _fileSystem;
+
+  /// The expected separator for multiple licenses.
+  static final String licenseSeparator = '\n' + ('-' * 80) + '\n';
+
+  /// Obtain licenses from the `packageMap` into a single result.
+  LicenseResult obtainLicenses(
+    PackageMap packageMap,
+  ) {
+    final Map<String, Set<String>> packageLicenses = <String, Set<String>>{};
+    final Set<String> allPackages = <String>{};
+    final List<File> dependencies = <File>[];
+
+    for (final String packageName in packageMap.map.keys) {
+      final Uri package = packageMap.map[packageName];
+      if (package == null || package.scheme != 'file') {
+        continue;
       }
-      if (licenseText == null) {
-        packageNames = <String>[packageName];
-        licenseText = rawLicense;
+      final File file = _fileSystem.file(package.resolve('../LICENSE'));
+      if (!file.existsSync()) {
+        continue;
       }
-      packageLicenses.putIfAbsent(licenseText, () => <String>{})
-        ..addAll(packageNames);
-      allPackages.addAll(packageNames);
-    }
-  }
 
-  if (reportPackages) {
-    final List<String> allPackagesList = allPackages.toList()..sort();
-    globals.printStatus('Licenses were found for the following packages:');
-    globals.printStatus(allPackagesList.join(', '));
+      dependencies.add(file);
+      final List<String> rawLicenses = file
+        .readAsStringSync()
+        .split(licenseSeparator);
+      for (final String rawLicense in rawLicenses) {
+        List<String> packageNames;
+        String licenseText;
+        if (rawLicenses.length > 1) {
+          final int split = rawLicense.indexOf('\n\n');
+          if (split >= 0) {
+            packageNames = rawLicense.substring(0, split).split('\n');
+            licenseText = rawLicense.substring(split + 2);
+          }
+        }
+        if (licenseText == null) {
+          packageNames = <String>[packageName];
+          licenseText = rawLicense;
+        }
+        packageLicenses.putIfAbsent(licenseText, () => <String>{})
+          ..addAll(packageNames);
+        allPackages.addAll(packageNames);
+      }
+    }
+    final List<String> combinedLicensesList = packageLicenses.keys
+      .map<String>((String license) {
+        final List<String> packageNames = packageLicenses[license].toList()
+          ..sort();
+        return packageNames.join('\n') + '\n\n' + license;
+      }).toList();
+    combinedLicensesList.sort();
+    final String combinedLicenses = combinedLicensesList.join(licenseSeparator);
+
+    return LicenseResult(
+      combinedLicenses: combinedLicenses,
+      dependencies: dependencies,
+    );
   }
+}
 
-  final List<String> combinedLicensesList = packageLicenses.keys.map<String>(
-    (String license) {
-      final List<String> packageNames = packageLicenses[license].toList()
-       ..sort();
-      return packageNames.join('\n') + '\n\n' + license;
-    }
-  ).toList();
-  combinedLicensesList.sort();
+/// The result of processing licenses with a [LicenseCollector].
+class LicenseResult {
+  const LicenseResult({
+    @required this.combinedLicenses,
+    @required this.dependencies,
+  });
 
-  final String combinedLicenses = combinedLicensesList.join(_licenseSeparator);
+  /// The raw text of the consumed licenses.
+  final String combinedLicenses;
 
-  return DevFSStringContent(combinedLicenses);
+  /// Each license file that was consumed as input.
+  final List<File> dependencies;
 }
 
 int _byBasename(_Asset a, _Asset b) {

packages/flutter_tools/lib/src/build_system/targets/assets.dart

@@ -72,7 +72,7 @@ Future<Depfile> copyAssets(Environment environment, Directory outputDirectory) a
         resource.release();
       }
   }));
-  return Depfile(inputs, outputs);
+  return Depfile(inputs + assetBundle.additionalDependencies, outputs);
 }
 
 /// Copy the assets defined in the flutter manifest into a build directory.

packages/flutter_tools/lib/src/dart/package_map.dart

@@ -17,6 +17,11 @@ Map<String, Uri> _parse(String packagesPath) {
 class PackageMap {
   PackageMap(this.packagesPath);
 
+  /// Create a [PackageMap] for testing.
+  PackageMap.test(Map<String, Uri> input)
+    : packagesPath = '.packages',
+      _map = input;
+
   static String get globalPackagesPath => _globalPackagesPath ?? kPackagesFileName;
 
   static set globalPackagesPath(String value) {

packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart

@@ -2,35 +2,40 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:file/memory.dart';
+import 'package:file_testing/file_testing.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/build_system/build_system.dart';
+import 'package:flutter_tools/src/build_system/depfile.dart';
 import 'package:flutter_tools/src/build_system/targets/assets.dart';
-import 'package:flutter_tools/src/globals.dart' as globals;
+import 'package:platform/platform.dart';
 
 import '../../../src/common.dart';
-import '../../../src/testbed.dart';
+import '../../../src/context.dart';
 
 void main() {
   Environment environment;
-  Testbed testbed;
+  FileSystem fileSystem;
+  Platform platform;
 
   setUp(() {
-    testbed = Testbed(setup: () {
-      environment = Environment.test(
-        globals.fs.currentDirectory,
-      );
-      globals.fs.file(environment.buildDir.childFile('app.dill')).createSync(recursive: true);
-      globals.fs.file(globals.fs.path.join('packages', 'flutter_tools', 'lib', 'src',
-          'build_system', 'targets', 'assets.dart'))
-        ..createSync(recursive: true);
-      globals.fs.file(globals.fs.path.join('assets', 'foo', 'bar.png'))
-        ..createSync(recursive: true);
-      globals.fs.file(globals.fs.path.join('assets', 'wildcard', '#bar.png'))
-        ..createSync(recursive: true);
-      globals.fs.file('.packages')
-        ..createSync();
-      globals.fs.file('pubspec.yaml')
-        ..createSync()
-        ..writeAsStringSync('''
+    platform = FakePlatform();
+    fileSystem = MemoryFileSystem.test();
+    environment = Environment.test(
+      fileSystem.currentDirectory,
+    );
+    fileSystem.file(environment.buildDir.childFile('app.dill')).createSync(recursive: true);
+    fileSystem.file('packages/flutter_tools/lib/src/build_system/targets/assets.dart')
+      ..createSync(recursive: true);
+    fileSystem.file('assets/foo/bar.png')
+      ..createSync(recursive: true);
+    fileSystem.file('assets/wildcard/#bar.png')
+      ..createSync(recursive: true);
+    fileSystem.file('.packages')
+      ..createSync();
+    fileSystem.file('pubspec.yaml')
+      ..createSync()
+      ..writeAsStringSync('''
 name: example
 
 flutter:
@@ -38,29 +43,65 @@ flutter:
     - assets/foo/bar.png
     - assets/wildcard/
 ''');
-    });
   });
 
-  test('Copies files to correct asset directory', () => testbed.run(() async {
+  testUsingContext('includes LICENSE file inputs in dependencies', () async {
+    fileSystem.file('.packages')
+      .writeAsStringSync('foo:file:///bar/lib');
+    fileSystem.file('bar/LICENSE')
+      ..createSync(recursive: true)
+      ..writeAsStringSync('THIS IS A LICENSE');
+
+    await const CopyAssets().build(environment);
+
+    final File depfile = environment.buildDir.childFile('flutter_assets.d');
+
+    expect(depfile, exists);
+
+    final DepfileService depfileService = DepfileService(
+      logger: null,
+      fileSystem: fileSystem,
+      platform: platform,
+    );
+    final Depfile dependencies = depfileService.parse(depfile);
+
+    expect(
+      dependencies.inputs.firstWhere((File file) => file.path == '/bar/LICENSE', orElse: () => null),
+      isNotNull,
+    );
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+    Platform: () => platform,
+  });
+
+  testUsingContext('Copies files to correct asset directory', () async {
     await const CopyAssets().build(environment);
 
-    expect(globals.fs.file(globals.fs.path.join(environment.buildDir.path, 'flutter_assets', 'AssetManifest.json')).existsSync(), true);
-    expect(globals.fs.file(globals.fs.path.join(environment.buildDir.path, 'flutter_assets', 'FontManifest.json')).existsSync(), true);
-    expect(globals.fs.file(globals.fs.path.join(environment.buildDir.path, 'flutter_assets', 'LICENSE')).existsSync(), true);
+    expect(fileSystem.file('${environment.buildDir.path}/flutter_assets/AssetManifest.json'), exists);
+    expect(fileSystem.file('${environment.buildDir.path}/flutter_assets/FontManifest.json'), exists);
+    expect(fileSystem.file('${environment.buildDir.path}/flutter_assets/LICENSE'), exists);
     // See https://github.com/flutter/flutter/issues/35293
-    expect(globals.fs.file(globals.fs.path.join(environment.buildDir.path, 'flutter_assets', 'assets/foo/bar.png')).existsSync(), true);
+    expect(fileSystem.file('${environment.buildDir.path}/flutter_assets/assets/foo/bar.png'), exists);
     // See https://github.com/flutter/flutter/issues/46163
-    expect(globals.fs.file(globals.fs.path.join(environment.buildDir.path, 'flutter_assets', 'assets/wildcard/%23bar.png')).existsSync(), true);
-  }));
+    expect(fileSystem.file('${environment.buildDir.path}/flutter_assets/assets/wildcard/%23bar.png'), exists);
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+    Platform: () => platform,
+  });
 
-  test('FlutterPlugins updates required files as needed', () => testbed.run(() async {
-    globals.fs.file('pubspec.yaml')
+  testUsingContext('FlutterPlugins updates required files as needed', () async {
+    fileSystem.file('pubspec.yaml')
       ..writeAsStringSync('name: foo\ndependencies:\n  foo: any\n');
 
     await const FlutterPlugins().build(Environment.test(
-      globals.fs.currentDirectory,
+      fileSystem.currentDirectory,
     ));
 
-    expect(globals.fs.file('.flutter-plugins').existsSync(), true);
-  }));
+    expect(fileSystem.file('.flutter-plugins'), exists);
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+  });
 }