From 8403930d4144a3501c6f3f3b5ab6b628cebbc786 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 27 Mar 2020 22:11:01 -0700 Subject: [PATCH] [flutter_tools] unpin SDK deps when upgrading packages (#53429) --- .../lib/src/commands/update_packages.dart | 95 ++++++++++++++++++- packages/flutter_tools/lib/src/dart/pub.dart | 10 +- .../general.shard/update_packages_test.dart | 91 ++++++++++++++++++ .../flutter_tools/test/src/throwing_pub.dart | 1 + 4 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 packages/flutter_tools/test/general.shard/update_packages_test.dart diff --git a/packages/flutter_tools/lib/src/commands/update_packages.dart b/packages/flutter_tools/lib/src/commands/update_packages.dart index 246e2b81d20..a2312f9b5c1 100644 --- a/packages/flutter_tools/lib/src/commands/update_packages.dart +++ b/packages/flutter_tools/lib/src/commands/update_packages.dart @@ -276,14 +276,30 @@ class UpdatePackagesCommand extends FlutterCommand { final File fakePackage = _pubspecFor(tempDir); fakePackage.createSync(); fakePackage.writeAsStringSync(_generateFakePubspec(dependencies.values)); - // First we run "pub upgrade" on this generated package: + // First, we create a synthetic flutter SDK so that transitive flutter SDK + // constraints are not affected by this upgrade. + final Directory temporaryFlutterSdk = createTemporaryFlutterSdk( + globals.fs, + globals.fs.directory(Cache.flutterRoot), + pubspecs, + ); + + // Next we run "pub upgrade" on this generated package: await pub.get( context: PubContext.updatePackages, directory: tempDir.path, upgrade: true, checkLastModified: false, offline: offline, + flutterRootOverride: temporaryFlutterSdk.path, ); + // Cleanup the temporary SDK + try { + temporaryFlutterSdk.deleteSync(recursive: true); + } on FileSystemException { + // So sad... + } + // Then we run "pub deps --style=compact" on the result. We pipe all the // output to tree.fill(), which parses it so that it can create a graph // of all the dependencies so that we can figure out the transitive @@ -1170,7 +1186,8 @@ class PubspecDependency extends PubspecLine { /// Generates the File object for the pubspec.yaml file of a given Directory. File _pubspecFor(Directory directory) { - return globals.fs.file(globals.fs.path.join(directory.path, 'pubspec.yaml')); + return directory.fileSystem.file( + directory.fileSystem.path.join(directory.path, 'pubspec.yaml')); } /// Generates the source of a fake pubspec.yaml file given a list of @@ -1334,3 +1351,77 @@ String _computeChecksum(Iterable names, String getVersion(String name)) } return ((upperCheck << 8) | lowerCheck).toRadixString(16).padLeft(4, '0'); } + +/// Create a synthetic Flutter SDK so that pub version solving does not get +/// stuck on the old versions. +Directory createTemporaryFlutterSdk(FileSystem fileSystem, Directory realFlutter, List pubspecs) { + final Set currentPackages = realFlutter + .childDirectory('packages') + .listSync() + .whereType() + .map((Directory directory) => fileSystem.path.basename(directory.path)) + .toSet(); + + final Map pubspecsByName = {}; + for (final PubspecYaml pubspec in pubspecs) { + pubspecsByName[pubspec.name] = pubspec; + } + + final Directory directory = fileSystem.systemTempDirectory + .createTempSync('flutter_upgrade_sdk.') + ..createSync(); + // Fill in version info. + realFlutter.childFile('version') + .copySync(directory.childFile('version').path); + + // Directory structure should mirror the current Flutter SDK + final Directory packages = directory.childDirectory('packages'); + for (final String flutterPackage in currentPackages) { + final File pubspecFile = packages + .childDirectory(flutterPackage) + .childFile('pubspec.yaml') + ..createSync(recursive: true); + final PubspecYaml pubspecYaml = pubspecsByName[flutterPackage]; + final StringBuffer output = StringBuffer('name: $flutterPackage\n'); + + // Fill in SDK dependency constraint. + output.write(''' +environment: + sdk: ">=2.7.0 <3.0.0" +'''); + + output.writeln('dependencies:'); + for (final PubspecDependency dependency in pubspecYaml.dependencies) { + if (dependency.isTransitive || dependency.isDevDependency) { + continue; + } + if (dependency.kind == DependencyKind.sdk) { + output.writeln(' ${dependency.name}:\n sdk: flutter'); + continue; + } + output.writeln(' ${dependency.name}: any'); + } + pubspecFile.writeAsStringSync(output.toString()); + } + + // Create the sky engine pubspec.yaml + directory + .childDirectory('bin') + .childDirectory('cache') + .childDirectory('pkg') + .childDirectory('sky_engine') + .childFile('pubspec.yaml') + ..createSync(recursive: true) + ..writeAsStringSync(''' +name: sky_engine +version: 0.0.99 +author: Flutter Authors +description: Dart SDK extensions for dart:ui +homepage: http://flutter.io +# sky_engine requires sdk_ext support in the analyzer which was added in 1.11.x +environment: + sdk: '>=1.11.0 <3.0.0' +'''); + + return directory; +} diff --git a/packages/flutter_tools/lib/src/dart/pub.dart b/packages/flutter_tools/lib/src/dart/pub.dart index 10ae1b92056..aa5e8732e40 100644 --- a/packages/flutter_tools/lib/src/dart/pub.dart +++ b/packages/flutter_tools/lib/src/dart/pub.dart @@ -96,6 +96,7 @@ abstract class Pub { bool offline = false, bool checkLastModified = true, bool skipPubspecYamlCheck = false, + String flutterRootOverride, }); /// Runs pub in 'batch' mode. @@ -143,6 +144,7 @@ class _DefaultPub implements Pub { bool offline = false, bool checkLastModified = true, bool skipPubspecYamlCheck = false, + String flutterRootOverride, }) async { directory ??= globals.fs.currentDirectory.path; @@ -178,6 +180,7 @@ class _DefaultPub implements Pub { filter: _filterOverrideWarnings, failureMessage: 'pub $command failed', retry: true, + flutterRootOverride: flutterRootOverride, ); status.stop(); // The exception is rethrown, so don't catch only Exceptions. @@ -231,6 +234,7 @@ class _DefaultPub implements Pub { String failureMessage = 'pub failed', @required bool retry, bool showTraceForErrors, + String flutterRootOverride, }) async { showTraceForErrors ??= await globals.isRunningOnBot; @@ -259,7 +263,7 @@ class _DefaultPub implements Pub { _pubCommand(arguments), workingDirectory: directory, mapFunction: filterWrapper, // may set versionSolvingFailed, lastPubMessage - environment: await _createPubEnvironment(context), + environment: await _createPubEnvironment(context, flutterRootOverride), ); String message; switch (code) { @@ -348,9 +352,9 @@ typedef MessageFilter = String Function(String message); /// /// [context] provides extra information to package server requests to /// understand usage. -Future> _createPubEnvironment(PubContext context) async { +Future> _createPubEnvironment(PubContext context, [ String flutterRootOverride ]) async { final Map environment = { - 'FLUTTER_ROOT': Cache.flutterRoot, + 'FLUTTER_ROOT': flutterRootOverride ?? Cache.flutterRoot, _pubEnvironmentKey: await _getPubEnvironmentValue(context), }; final String pubCache = _getRootPubCacheIfAvailable(); diff --git a/packages/flutter_tools/test/general.shard/update_packages_test.dart b/packages/flutter_tools/test/general.shard/update_packages_test.dart new file mode 100644 index 00000000000..8bf33a91b11 --- /dev/null +++ b/packages/flutter_tools/test/general.shard/update_packages_test.dart @@ -0,0 +1,91 @@ +// 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:file/memory.dart'; +import 'package:file_testing/file_testing.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/commands/update_packages.dart'; + +import '../src/common.dart'; + +// An example pubspec.yaml from flutter, not necessary for it to be up to date. +const String kFlutterPubspecYaml = r''' +name: flutter +author: Flutter Authors +description: A framework for writing Flutter applications +homepage: http://flutter.dev + +environment: + # The pub client defaults to an <2.0.0 sdk constraint which we need to explicitly overwrite. + sdk: ">=2.2.2 <3.0.0" + +dependencies: + # To update these, use "flutter update-packages --force-upgrade". + collection: 1.14.11 + meta: 1.1.8 + typed_data: 1.1.6 + vector_math: 2.0.8 + + sky_engine: + sdk: flutter + +dev_dependencies: + flutter_test: + sdk: flutter + flutter_goldens: + sdk: flutter + mockito: 4.1.1 + + archive: 2.0.11 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + +# PUBSPEC CHECKSUM: 1437 +'''; + +void main() { + testWithoutContext('createTemporaryFlutterSdk creates an unpinned flutter SDK', () { + final FileSystem fileSystem = MemoryFileSystem.test(); + + // Setup simplified FLutter SDK. + final Directory flutterSdk = fileSystem.directory('flutter') + ..createSync(); + // Create version file + flutterSdk.childFile('version').writeAsStringSync('1.2.3'); + // Create a pubspec file + final Directory flutter = flutterSdk + .childDirectory('packages') + .childDirectory('flutter') + ..createSync(recursive: true); + flutter + .childFile('pubspec.yaml') + .writeAsStringSync(kFlutterPubspecYaml); + + // Create already parsed pubspecs. + final PubspecYaml flutterPubspec = PubspecYaml(flutter); + + final Directory result = createTemporaryFlutterSdk( + fileSystem, + flutterSdk, + [flutterPubspec], + ); + + expect(result, exists); + + // The version file exists. + expect(result.childFile('version'), exists); + expect(result.childFile('version').readAsStringSync(), '1.2.3'); + + // The sky_engine package exists + expect(fileSystem.directory('${result.path}/bin/cache/pkg/sky_engine'), exists); + + // The flutter pubspec exists + final File pubspecFile = fileSystem.file('${result.path}/packages/flutter/pubspec.yaml'); + expect(pubspecFile, exists); + + // The flutter pubspec contains `any` dependencies. + final PubspecYaml outputPubspec = PubspecYaml(pubspecFile.parent); + expect(outputPubspec.name, 'flutter'); + expect(outputPubspec.dependencies.first.name, 'collection'); + expect(outputPubspec.dependencies.first.version, 'any'); + }); +} diff --git a/packages/flutter_tools/test/src/throwing_pub.dart b/packages/flutter_tools/test/src/throwing_pub.dart index 8243c73a080..e2d4c61da58 100644 --- a/packages/flutter_tools/test/src/throwing_pub.dart +++ b/packages/flutter_tools/test/src/throwing_pub.dart @@ -28,6 +28,7 @@ class ThrowingPub implements Pub { bool offline = false, bool checkLastModified = true, bool skipPubspecYamlCheck = false, + String flutterRootOverride, }) { throw UnsupportedError('Attempted to invoke pub during test.'); }