From 8d72307c4710eee4e45abffb5ceb98b6af930d69 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 14 Jan 2021 16:59:04 -0800 Subject: [PATCH] [flutter_tools] allow applications to specify additional license files to be bundled into the application NOTICES automatically (#73430) --- packages/flutter_tools/lib/src/asset.dart | 121 ++----------- .../lib/src/flutter_manifest.dart | 32 ++++ .../lib/src/license_collector.dart | 161 ++++++++++++++++++ .../general.shard/flutter_manifest_test.dart | 60 +++++++ .../general.shard/license_collector_test.dart | 125 +++++++++++++- 5 files changed, 393 insertions(+), 106 deletions(-) create mode 100644 packages/flutter_tools/lib/src/license_collector.dart diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart index 5a9f4d46cb0..a6c59bae079 100644 --- a/packages/flutter_tools/lib/src/asset.dart +++ b/packages/flutter_tools/lib/src/asset.dart @@ -15,6 +15,7 @@ import 'convert.dart'; import 'dart/package_map.dart'; import 'devfs.dart'; import 'flutter_manifest.dart'; +import 'license_collector.dart'; import 'project.dart'; const String defaultManifestPath = 'pubspec.yaml'; @@ -224,7 +225,8 @@ class ManifestAssetBundle implements AssetBundle { primary: true, ); - // Add fonts and assets from packages. + // Add fonts, assets, and licenses from packages. + final Map> additionalLicenseFiles = >{}; for (final Package package in packageConfig.packages) { final Uri packageUri = package.packageUriRoot; if (packageUri != null && packageUri.scheme == 'file') { @@ -237,6 +239,14 @@ class ManifestAssetBundle implements AssetBundle { if (packageFlutterManifest == null) { continue; } + // Collect any additional licenses from each package. + final List licenseFiles = []; + for (final String relativeLicensePath in packageFlutterManifest.additionalLicenses) { + final String absoluteLicensePath = _fileSystem.path.fromUri(package.root.resolve(relativeLicensePath)); + licenseFiles.add(_fileSystem.file(absoluteLicensePath).absolute); + } + additionalLicenseFiles[packageFlutterManifest.appName] = licenseFiles; + // Skip the app itself if (packageFlutterManifest.appName == flutterManifest.appName) { continue; @@ -319,7 +329,12 @@ class ManifestAssetBundle implements AssetBundle { final DevFSStringContent assetManifest = _createAssetManifest(assetVariants); final DevFSStringContent fontManifest = DevFSStringContent(json.encode(fonts)); - final LicenseResult licenseResult = _licenseCollector.obtainLicenses(packageConfig); + final LicenseResult licenseResult = _licenseCollector.obtainLicenses(packageConfig, additionalLicenseFiles); + if (licenseResult.errorMessages.isNotEmpty) { + licenseResult.errorMessages.forEach(_logger.printError); + return 1; + } + final DevFSStringContent licenses = DevFSStringContent(licenseResult.combinedLicenses); additionalDependencies = licenseResult.dependencies; @@ -721,108 +736,6 @@ class _Asset { } } -/// Processes dependencies into a string representing the NOTICES file. -/// -/// Reads the NOTICES or LICENSE file from each package in the .packages file, -/// splitting each one into each component license so that it can be de-duped -/// if possible. If the NOTICES file exists, it is preferred over the LICENSE -/// file. -/// -/// Individual licenses inside each LICENSE file should be separated by 80 -/// hyphens on their own on a line. -/// -/// If a LICENSE or NOTICES 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. -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( - PackageConfig packageConfig, - ) { - final Map> packageLicenses = >{}; - final Set allPackages = {}; - final List dependencies = []; - - for (final Package package in packageConfig.packages) { - final Uri packageUri = package.packageUriRoot; - if (packageUri == null || packageUri.scheme != 'file') { - continue; - } - // First check for NOTICES, then fallback to LICENSE - File file = _fileSystem.file(packageUri.resolve('../NOTICES')); - if (!file.existsSync()) { - file = _fileSystem.file(packageUri.resolve('../LICENSE')); - } - if (!file.existsSync()) { - continue; - } - - dependencies.add(file); - final List rawLicenses = file - .readAsStringSync() - .split(licenseSeparator); - for (final String rawLicense in rawLicenses) { - List 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 = [package.name]; - licenseText = rawLicense; - } - packageLicenses.putIfAbsent(licenseText, () => {}) - .addAll(packageNames); - allPackages.addAll(packageNames); - } - } - final List combinedLicensesList = packageLicenses.keys - .map((String license) { - final List 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, - ); - } -} - -/// The result of processing licenses with a [LicenseCollector]. -class LicenseResult { - const LicenseResult({ - @required this.combinedLicenses, - @required this.dependencies, - }); - - /// The raw text of the consumed licenses. - final String combinedLicenses; - - /// Each license file that was consumed as input. - final List dependencies; -} - // Given an assets directory like this: // // assets/foo diff --git a/packages/flutter_tools/lib/src/flutter_manifest.dart b/packages/flutter_tools/lib/src/flutter_manifest.dart index 5918330d8b1..d3f303a6de1 100644 --- a/packages/flutter_tools/lib/src/flutter_manifest.dart +++ b/packages/flutter_tools/lib/src/flutter_manifest.dart @@ -140,6 +140,22 @@ class FlutterManifest { return false; } + /// Any additional license files listed under the `flutter` key. + /// + /// This is expected to be a list of file paths that should be treated as + /// relative to the pubspec in this directory. + /// + /// For example: + /// + /// ```yaml + /// flutter: + /// licenses: + /// - assets/foo_license.txt + /// ``` + List get additionalLicenses => _flutterDescriptor.containsKey('licenses') + ? (_flutterDescriptor['licenses'] as YamlList).map((dynamic element) => element.toString()).toList() + : []; + /// True if this manifest declares a Flutter module project. /// /// A Flutter project is considered a module when it has a `module:` @@ -434,6 +450,14 @@ void _validateFlutter(YamlMap yaml, List errors) { _validateFonts(kvp.value as YamlList, errors); } break; + case 'licenses': + final dynamic value = kvp.value; + if (value is YamlList) { + _validateListType(value, '${kvp.key}', errors); + } else { + errors.add('Expected "${kvp.key}" to be a list of files, but got $value (${value.runtimeType})'); + } + break; case 'module': if (kvp.value is! YamlMap) { errors.add('Expected "${kvp.key}" to be an object, but got ${kvp.value} (${kvp.value.runtimeType}).'); @@ -466,6 +490,14 @@ void _validateFlutter(YamlMap yaml, List errors) { } } +void _validateListType(YamlList yamlList, String context, List errors) { + for (int i = 0; i < yamlList.length; i++) { + if (yamlList[i] is! T) { + errors.add('Expected "$context" to be a list of files, but element $i was a ${yamlList[i].runtimeType}'); + } + } +} + void _validateFonts(YamlList fonts, List errors) { if (fonts == null) { return; diff --git a/packages/flutter_tools/lib/src/license_collector.dart b/packages/flutter_tools/lib/src/license_collector.dart new file mode 100644 index 00000000000..36de22ed254 --- /dev/null +++ b/packages/flutter_tools/lib/src/license_collector.dart @@ -0,0 +1,161 @@ +// Copyright 2014 The Flutter 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 'package:meta/meta.dart'; +import 'package:package_config/package_config.dart'; + +import 'base/file_system.dart'; + +/// Processes dependencies into a string representing the NOTICES file. +/// +/// Reads the NOTICES or LICENSE file from each package in the .packages file, +/// splitting each one into each component license so that it can be de-duped +/// if possible. If the NOTICES file exists, it is preferred over the LICENSE +/// file. +/// +/// Individual licenses inside each LICENSE file should be separated by 80 +/// hyphens on their own on a line. +/// +/// If a LICENSE or NOTICES 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. +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. + /// + /// [additionalLicenses] should contain aggregated license files from all + /// of the current applications dependencies. + LicenseResult obtainLicenses( + PackageConfig packageConfig, + Map> additionalLicenses, + ) { + final Map> packageLicenses = >{}; + final Set allPackages = {}; + final List dependencies = []; + + for (final Package package in packageConfig.packages) { + final Uri packageUri = package.packageUriRoot; + if (packageUri == null || packageUri.scheme != 'file') { + continue; + } + // First check for NOTICES, then fallback to LICENSE + File file = _fileSystem.file(packageUri.resolve('../NOTICES')); + if (!file.existsSync()) { + file = _fileSystem.file(packageUri.resolve('../LICENSE')); + } + if (!file.existsSync()) { + continue; + } + + dependencies.add(file); + final List rawLicenses = file + .readAsStringSync() + .split(licenseSeparator); + for (final String rawLicense in rawLicenses) { + List 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 = [package.name]; + licenseText = rawLicense; + } + packageLicenses.putIfAbsent(licenseText, () => {}).addAll(packageNames); + allPackages.addAll(packageNames); + } + } + + final List combinedLicensesList = packageLicenses.keys + .map((String license) { + final List packageNames = packageLicenses[license].toList() + ..sort(); + return packageNames.join('\n') + '\n\n' + license; + }).toList(); + combinedLicensesList.sort(); + + /// Append additional LICENSE files as specified in the pubspec.yaml. + final List additionalLicenseText = []; + final List errorMessages = []; + for (final String package in additionalLicenses.keys) { + for (final File license in additionalLicenses[package]) { + if (!license.existsSync()) { + errorMessages.add( + 'package $package specified an additional license at ${license.path}, but this file ' + 'does not exist.' + ); + continue; + } + dependencies.add(license); + try { + additionalLicenseText.add(license.readAsStringSync()); + } on FormatException catch (err) { + // File has an invalid encoding. + errorMessages.add( + 'package $package specified an additional license at ${license.path}, but this file ' + 'could not be read:\n$err' + ); + } on FileSystemException catch (err) { + // File cannot be parsed. + errorMessages.add( + 'package $package specified an additional license at ${license.path}, but this file ' + 'could not be read:\n$err' + ); + } + } + } + if (errorMessages.isNotEmpty) { + return LicenseResult( + combinedLicenses: '', + dependencies: [], + errorMessages: errorMessages, + ); + } + + final String combinedLicenses = combinedLicensesList + .followedBy(additionalLicenseText) + .join(licenseSeparator); + + return LicenseResult( + combinedLicenses: combinedLicenses, + dependencies: dependencies, + errorMessages: errorMessages, + ); + } +} + +/// The result of processing licenses with a [LicenseCollector]. +class LicenseResult { + const LicenseResult({ + @required this.combinedLicenses, + @required this.dependencies, + @required this.errorMessages, + }); + + /// The raw text of the consumed licenses. + final String combinedLicenses; + + /// Each license file that was consumed as input. + final List dependencies; + + /// If non-empty, license collection failed and this messages should + /// be displayed by the asset parser. + final List errorMessages; +} diff --git a/packages/flutter_tools/test/general.shard/flutter_manifest_test.dart b/packages/flutter_tools/test/general.shard/flutter_manifest_test.dart index 998ebc144cb..e23c93dd51e 100644 --- a/packages/flutter_tools/test/general.shard/flutter_manifest_test.dart +++ b/packages/flutter_tools/test/general.shard/flutter_manifest_test.dart @@ -30,6 +30,7 @@ void main() { expect(flutterManifest.fontsDescriptor, isEmpty); expect(flutterManifest.fonts, isEmpty); expect(flutterManifest.assets, isEmpty); + expect(flutterManifest.additionalLicenses, isEmpty); }); testWithoutContext('FlutterManifest is null when the pubspec.yaml file is not a map', () async { @@ -1082,6 +1083,65 @@ flutter: expect(logger.errorText, contains('Cannot find the `flutter.plugin.platforms` key in the `pubspec.yaml` file. ')); }); + + testWithoutContext('FlutterManifest can specify additional LICENSE files', () async { + const String manifest = ''' +name: test +dependencies: + flutter: + sdk: flutter +flutter: + licenses: + - foo.txt +'''; + final BufferLogger logger = BufferLogger.test(); + final FlutterManifest flutterManifest = FlutterManifest.createFromString( + manifest, + logger: logger, + ); + + expect(flutterManifest.additionalLicenses, ['foo.txt']); + }); + + testWithoutContext('FlutterManifest can validate incorrect licenses key', () async { + const String manifest = ''' +name: test +dependencies: + flutter: + sdk: flutter +flutter: + licenses: foo.txt +'''; + final BufferLogger logger = BufferLogger.test(); + final FlutterManifest flutterManifest = FlutterManifest.createFromString( + manifest, + logger: logger, + ); + + expect(flutterManifest, null); + expect(logger.errorText, 'Expected "licenses" to be a list of files, but got foo.txt (String)\n'); + }); + + testWithoutContext('FlutterManifest validates individual list items', () async { + const String manifest = ''' +name: test +dependencies: + flutter: + sdk: flutter +flutter: + licenses: + - foo.txt + - bar: fizz +'''; + final BufferLogger logger = BufferLogger.test(); + final FlutterManifest flutterManifest = FlutterManifest.createFromString( + manifest, + logger: logger, + ); + + expect(flutterManifest, null); + expect(logger.errorText, 'Expected "licenses" to be a list of files, but element 1 was a YamlMap\n'); + }); } Matcher matchesManifest({ diff --git a/packages/flutter_tools/test/general.shard/license_collector_test.dart b/packages/flutter_tools/test/general.shard/license_collector_test.dart index 4709c1e95b2..94a45679b62 100644 --- a/packages/flutter_tools/test/general.shard/license_collector_test.dart +++ b/packages/flutter_tools/test/general.shard/license_collector_test.dart @@ -3,9 +3,9 @@ // found in the LICENSE file. 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/convert.dart'; +import 'package:flutter_tools/src/license_collector.dart'; import 'package:package_config/package_config.dart'; import 'package:package_config/package_config_types.dart'; @@ -290,7 +290,7 @@ void main() { } )); final PackageConfig packageConfig = await loadPackageConfig(packageConfigFile.absolute); - final LicenseResult result = licenseCollector.obtainLicenses(packageConfig); + final LicenseResult result = licenseCollector.obtainLicenses(packageConfig, >{}); // All included licenses are combined in the result. expect(result.combinedLicenses, contains(_kApacheLicense)); @@ -310,4 +310,125 @@ void main() { '/fizz/LICENSE' ])); }); + + testWithoutContext('includes additional LICENSE files as specified by pubspec.yaml', () async { + fileSystem.file('foo/NOTICES') + ..createSync(recursive: true) + ..writeAsStringSync(_kMitLicense); + fileSystem.file('bar/NOTICES') + ..createSync(recursive: true) + ..writeAsStringSync(_kApacheLicense); + fileSystem.file('foo.txt').writeAsStringSync('foo.txt'); + fileSystem.file('bar.txt').writeAsStringSync('bar.txt'); + + final File packageConfigFile = fileSystem.file('package_config.json') + ..writeAsStringSync(json.encode( + { + 'configVersion': 2, + 'packages': [ + { + 'name': 'foo', + 'rootUri': 'file:///foo/', + 'packageUri': 'lib/', + 'languageVersion': '2.2' + }, + { + 'name': 'bar', + 'rootUri': 'file:///bar/', + 'packageUri': 'lib/', + 'languageVersion': '2.2' + }, + ], + } + )); + final PackageConfig packageConfig = await loadPackageConfig(packageConfigFile.absolute); + final LicenseResult result = licenseCollector.obtainLicenses(packageConfig, >{ + 'foo': [fileSystem.file('foo.txt').absolute], + 'bar': [fileSystem.file('bar.txt').absolute], + }); + + // Additional license files are included in the result. + expect(result.combinedLicenses, contains(_kApacheLicense)); + expect(result.combinedLicenses, contains(_kMitLicense)); + expect(result.combinedLicenses, contains('foo.txt')); + expect(result.combinedLicenses, contains('bar.txt')); + + // All input licenses included in result. + final Iterable filePaths = result.dependencies.map((File file) => file.path); + expect(filePaths, unorderedEquals([ + '/foo/NOTICES', + '/bar/NOTICES', + '/foo.txt', + '/bar.txt', + ])); + }); + + testWithoutContext('Returns a LicenseResult with an error message if an additional LICENSE file does not exist', () async { + fileSystem.file('foo/NOTICES') + ..createSync(recursive: true) + ..writeAsStringSync(_kMitLicense); + + final File packageConfigFile = fileSystem.file('package_config.json') + ..writeAsStringSync(json.encode( + { + 'configVersion': 2, + 'packages': [ + { + 'name': 'foo', + 'rootUri': 'file:///foo/', + 'packageUri': 'lib/', + 'languageVersion': '2.2' + }, + ], + } + )); + final PackageConfig packageConfig = await loadPackageConfig(packageConfigFile.absolute); + + final LicenseResult licenseResult = licenseCollector.obtainLicenses(packageConfig, >{ + 'foo': [fileSystem.file('foo.txt').absolute, fileSystem.file('foo_2.txt').absolute], // Files do not exist. + }); + + expect(licenseResult.combinedLicenses, ''); + expect(licenseResult.dependencies, isEmpty); + expect(licenseResult.errorMessages, [ + 'package foo specified an additional license at /foo.txt, but this file does not exist.', + 'package foo specified an additional license at /foo_2.txt, but this file does not exist.' + ]); + }); + + testWithoutContext('Returns a LicenseResult with an error message if an additional license file is not valid utf8', () async { + fileSystem.file('foo/NOTICES') + ..createSync(recursive: true) + ..writeAsStringSync(_kMitLicense); + fileSystem.file('foo.txt') + ..createSync(recursive: true) + ..writeAsBytesSync([0xFFFE]); + + final File packageConfigFile = fileSystem.file('package_config.json') + ..writeAsStringSync(json.encode( + { + 'configVersion': 2, + 'packages': [ + { + 'name': 'foo', + 'rootUri': 'file:///foo/', + 'packageUri': 'lib/', + 'languageVersion': '2.2' + }, + ], + } + )); + final PackageConfig packageConfig = await loadPackageConfig(packageConfigFile.absolute); + + final LicenseResult licenseResult = licenseCollector.obtainLicenses(packageConfig, >{ + 'foo': [fileSystem.file('foo.txt').absolute], + }); + + expect(licenseResult.combinedLicenses, ''); + expect(licenseResult.dependencies, isEmpty); + expect(licenseResult.errorMessages.single, + 'package foo specified an additional license at /foo.txt, but this file could not be read:' + '\nFormatException: Invalid UTF-8 byte (at offset 0)', + ); + }); }