From 1995da2c0cb54322201c6fe1c051daa269e6bdc1 Mon Sep 17 00:00:00 2001 From: Remi Rousselet Date: Mon, 19 Jul 2021 16:06:03 +0100 Subject: [PATCH] Disable the automatic "pub get" if the project is using a third-party tool for linking dependencies. (#86177) --- .../lib/src/commands/packages.dart | 1 + packages/flutter_tools/lib/src/dart/pub.dart | 26 +++ .../commands.shard/hermetic/drive_test.dart | 1 + .../commands.shard/hermetic/pub_get_test.dart | 1 + .../test/general.shard/cache_test.dart | 12 +- .../test/general.shard/dart/pub_get_test.dart | 180 ++++++++++++++++++ .../runner/flutter_command_test.dart | 1 + .../flutter_tools/test/src/throwing_pub.dart | 1 + 8 files changed, 222 insertions(+), 1 deletion(-) diff --git a/packages/flutter_tools/lib/src/commands/packages.dart b/packages/flutter_tools/lib/src/commands/packages.dart index 0ffd536125..557bc0acb5 100644 --- a/packages/flutter_tools/lib/src/commands/packages.dart +++ b/packages/flutter_tools/lib/src/commands/packages.dart @@ -139,6 +139,7 @@ class PackagesGetCommand extends FlutterCommand { context: PubContext.pubGet, directory: directory, upgrade: upgrade, + shouldSkipThirdPartyGenerator: false, offline: boolArg('offline'), generateSyntheticPackage: flutterProject.manifest.generateSyntheticPackage, ); diff --git a/packages/flutter_tools/lib/src/dart/pub.dart b/packages/flutter_tools/lib/src/dart/pub.dart index 423661ebbf..2a71ae5fd4 100644 --- a/packages/flutter_tools/lib/src/dart/pub.dart +++ b/packages/flutter_tools/lib/src/dart/pub.dart @@ -86,6 +86,10 @@ abstract class Pub { /// /// [context] provides extra information to package server requests to /// understand usage. + /// + /// If [shouldSkipThirdPartyGenerator] is true, the overall pub get will be + /// skipped if the package config file has a "generator" other than "pub". + /// Defaults to true. Future get({ required PubContext context, String directory, @@ -95,6 +99,7 @@ abstract class Pub { bool generateSyntheticPackage = false, String flutterRootOverride, bool checkUpToDate = false, + bool shouldSkipThirdPartyGenerator = true, }); /// Runs pub in 'batch' mode. @@ -169,6 +174,7 @@ class _DefaultPub implements Pub { bool generateSyntheticPackage = false, String? flutterRootOverride, bool checkUpToDate = false, + bool shouldSkipThirdPartyGenerator = true, }) async { directory ??= _fileSystem.currentDirectory.path; final File packageConfigFile = _fileSystem.file( @@ -185,6 +191,26 @@ class _DefaultPub implements Pub { _fileSystem.path.join(directory, 'pubspec.lock') ); + if (shouldSkipThirdPartyGenerator && packageConfigFile.existsSync()) { + Map packageConfigMap; + try { + packageConfigMap = jsonDecode( + packageConfigFile.readAsStringSync(), + ) as Map; + } on FormatException { + packageConfigMap = {}; + } + + final bool isPackageConfigGeneratedByThirdParty = + packageConfigMap.containsKey('generator') && + packageConfigMap['generator'] != 'pub'; + + if (isPackageConfigGeneratedByThirdParty) { + _logger.printTrace('Skipping pub get: generated by third-party.'); + return; + } + } + // If the pubspec.yaml is older than the package config file and the last // flutter version used is the same as the current version skip pub get. // This will incorrectly skip pub on the master branch if dependencies diff --git a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart index 095226bf81..e95079e7a9 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart @@ -118,5 +118,6 @@ class FakePub extends Fake implements Pub { bool generateSyntheticPackage = false, String flutterRootOverride, bool checkUpToDate = false, + bool shouldSkipThirdPartyGenerator = true, }) async { } } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/pub_get_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/pub_get_test.dart index 6f36128a52..79649d1a14 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/pub_get_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/pub_get_test.dart @@ -113,6 +113,7 @@ class FakePub extends Fake implements Pub { bool generateSyntheticPackage = false, String flutterRootOverride, bool checkUpToDate = false, + bool shouldSkipThirdPartyGenerator = true, }) async { fileSystem.currentDirectory .childDirectory('.dart_tool') diff --git a/packages/flutter_tools/test/general.shard/cache_test.dart b/packages/flutter_tools/test/general.shard/cache_test.dart index 0d73c5d542..2c6850b38c 100644 --- a/packages/flutter_tools/test/general.shard/cache_test.dart +++ b/packages/flutter_tools/test/general.shard/cache_test.dart @@ -1045,7 +1045,17 @@ class FakePub extends Fake implements Pub { int calledGet = 0; @override - Future get({PubContext context, String directory, bool skipIfAbsent = false, bool upgrade = false, bool offline = false, bool generateSyntheticPackage = false, String flutterRootOverride, bool checkUpToDate = false}) async { + Future get({ + PubContext context, + String directory, + bool skipIfAbsent = false, + bool upgrade = false, + bool offline = false, + bool generateSyntheticPackage = false, + String flutterRootOverride, + bool checkUpToDate = false, + bool shouldSkipThirdPartyGenerator = true, + }) async { calledGet += 1; } } diff --git a/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart b/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart index 821bed3e9a..7d0bfb6089 100644 --- a/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart +++ b/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart @@ -47,6 +47,186 @@ void main() { ), throwsToolExit(message: 'Your Flutter SDK download may be corrupt or missing permissions to run')); }); + group('shouldSkipThirdPartyGenerator', () { + testWithoutContext('does not skip pub get the parameter is false', () async { + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand(command: [ + 'bin/cache/dart-sdk/bin/pub', + '--verbosity=warning', + 'get', + '--no-precompile', + ]) + ]); + final BufferLogger logger = BufferLogger.test(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + + fileSystem.file('pubspec.yaml').createSync(); + fileSystem.file('pubspec.lock').createSync(); + fileSystem.file('version').writeAsStringSync('b'); + fileSystem.file('.dart_tool/package_config.json') + ..createSync(recursive: true) + ..writeAsStringSync(''' + { + "configVersion": 2, + "packages": [], + "generated": "2021-07-08T10:02:49.155589Z", + "generator": "third-party", + "generatorVersion": "2.14.0-276.0.dev" + } + '''); + + final Pub pub = Pub( + fileSystem: fileSystem, + logger: logger, + processManager: processManager, + usage: TestUsage(), + platform: FakePlatform( + environment: const {}, + ), + botDetector: const BotDetectorAlwaysNo(), + ); + + await pub.get( + context: PubContext.pubGet, + checkUpToDate: true, + shouldSkipThirdPartyGenerator: false, + ); + + expect(processManager, hasNoRemainingExpectations); + expect(fileSystem.file('.dart_tool/version').readAsStringSync(), 'b'); + }); + + testWithoutContext('does not skip pub get if package_config.json has "generator": "pub"', () async { + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand(command: [ + 'bin/cache/dart-sdk/bin/pub', + '--verbosity=warning', + 'get', + '--no-precompile', + ]) + ]); + final BufferLogger logger = BufferLogger.test(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + + fileSystem.file('pubspec.yaml').createSync(); + fileSystem.file('pubspec.lock').createSync(); + fileSystem.file('.dart_tool/package_config.json') + ..createSync(recursive: true) + ..writeAsStringSync(''' + { + "configVersion": 2, + "packages": [], + "generated": "2021-07-08T10:02:49.155589Z", + "generator": "pub", + "generatorVersion": "2.14.0-276.0.dev" + } + '''); + fileSystem.file('.dart_tool/version').writeAsStringSync('a'); + fileSystem.file('version').writeAsStringSync('b'); + + final Pub pub = Pub( + fileSystem: fileSystem, + logger: logger, + processManager: processManager, + usage: TestUsage(), + platform: FakePlatform( + environment: const {}, + ), + botDetector: const BotDetectorAlwaysNo(), + ); + + await pub.get( + context: PubContext.pubGet, + checkUpToDate: true, + ); + + expect(processManager, hasNoRemainingExpectations); + expect(fileSystem.file('.dart_tool/version').readAsStringSync(), 'b'); + }); + + testWithoutContext('does not skip pub get if package_config.json has "generator": "pub"', () async { + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand(command: [ + 'bin/cache/dart-sdk/bin/pub', + '--verbosity=warning', + 'get', + '--no-precompile', + ]) + ]); + final BufferLogger logger = BufferLogger.test(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + + fileSystem.file('pubspec.yaml').createSync(); + fileSystem.file('pubspec.lock').createSync(); + fileSystem.file('.dart_tool/package_config.json') + ..createSync(recursive: true) + ..writeAsStringSync(''' + { + "configVersion": 2, + "packages": [], + "generated": "2021-07-08T10:02:49.155589Z", + "generator": "pub", + "generatorVersion": "2.14.0-276.0.dev" + } + '''); + fileSystem.file('.dart_tool/version').writeAsStringSync('a'); + fileSystem.file('version').writeAsStringSync('b'); + + final Pub pub = Pub( + fileSystem: fileSystem, + logger: logger, + processManager: processManager, + usage: TestUsage(), + platform: FakePlatform( + environment: const {}, + ), + botDetector: const BotDetectorAlwaysNo(), + ); + + await pub.get( + context: PubContext.pubGet, + checkUpToDate: true, + ); + + expect(processManager, hasNoRemainingExpectations); + expect(fileSystem.file('.dart_tool/version').readAsStringSync(), 'b'); + }); + + testWithoutContext('skips pub get if the package config "generator" is ' + 'different than "pub"', () async { + final FakeProcessManager processManager = FakeProcessManager.empty(); + final BufferLogger logger = BufferLogger.test(); + final MemoryFileSystem fileSystem = MemoryFileSystem.test(); + + fileSystem.file('pubspec.yaml').createSync(); + fileSystem.file('pubspec.lock').createSync(); + fileSystem.file('.dart_tool/package_config.json') + ..createSync(recursive: true) + ..writeAsStringSync('{"generator": "third-party"}'); + + final Pub pub = Pub( + fileSystem: fileSystem, + logger: logger, + processManager: processManager, + usage: TestUsage(), + platform: FakePlatform( + environment: const {}, + ), + botDetector: const BotDetectorAlwaysNo(), + ); + + await pub.get( + context: PubContext.pubGet, + checkUpToDate: true, + ); + + expect( + logger.traceText, + contains('Skipping pub get: generated by third-party.'), + ); + }); + }); + testWithoutContext('checkUpToDate skips pub get if the package config is newer than the pubspec ' 'and the current framework version is the same as the last version', () async { final FakeProcessManager processManager = FakeProcessManager.empty(); diff --git a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart index 466dc58c8a..8f0d9d910c 100644 --- a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart @@ -722,5 +722,6 @@ class FakePub extends Fake implements Pub { bool generateSyntheticPackage = false, String flutterRootOverride, bool checkUpToDate = false, + bool shouldSkipThirdPartyGenerator = true, }) async { } } diff --git a/packages/flutter_tools/test/src/throwing_pub.dart b/packages/flutter_tools/test/src/throwing_pub.dart index b4ff8a12ec..f23caa1515 100644 --- a/packages/flutter_tools/test/src/throwing_pub.dart +++ b/packages/flutter_tools/test/src/throwing_pub.dart @@ -30,6 +30,7 @@ class ThrowingPub implements Pub { bool generateSyntheticPackage = false, String? flutterRootOverride, bool checkUpToDate = false, + bool shouldSkipThirdPartyGenerator = true, }) { throw UnsupportedError('Attempted to invoke pub during test.'); }