From 949c2f6c77b6bae92e29a03fc452417b558010f0 Mon Sep 17 00:00:00 2001 From: Lukas Klingsbo Date: Fri, 24 Jan 2025 12:05:35 +0100 Subject: [PATCH] feat: Rollback on failed shared dependency resolution (#848) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixed: #847 ## Type of Change - [x] โœจ `feat` -- New feature (non-breaking change which adds functionality) - [ ] ๐Ÿ› ๏ธ `fix` -- Bug fix (non-breaking change which fixes an issue) - [ ] โŒ `!` -- Breaking change (fix or feature that would cause existing functionality to change) - [ ] ๐Ÿงน `refactor` -- Code refactor - [ ] โœ… `ci` -- Build configuration change - [ ] ๐Ÿ“ `docs` -- Documentation - [ ] ๐Ÿ—‘๏ธ `chore` -- Chore --- .../melos/lib/src/commands/bootstrap.dart | 37 +++-- .../melos/test/commands/bootstrap_test.dart | 133 ++++++++++++++++++ 2 files changed, 158 insertions(+), 12 deletions(-) diff --git a/packages/melos/lib/src/commands/bootstrap.dart b/packages/melos/lib/src/commands/bootstrap.dart index 8dd95be9..c0b1936a 100644 --- a/packages/melos/lib/src/commands/bootstrap.dart +++ b/packages/melos/lib/src/commands/bootstrap.dart @@ -37,6 +37,7 @@ mixin _BootstrapMixin on _CleanMixin { ..newLine(); final filteredPackages = workspace.filteredPackages.values; + final rollbackPubspecContent = {}; try { if (bootstrapCommandConfig.environment != null || @@ -44,18 +45,21 @@ mixin _BootstrapMixin on _CleanMixin { bootstrapCommandConfig.devDependencies != null) { logger.log('Updating common dependencies in workspace packages...'); - await Stream.fromIterable(filteredPackages).parallel((package) { + await Stream.fromIterable(filteredPackages) + .parallel((package) async { + final pubspecPath = utils.pubspecPathForDirectory(package.path); + final pubspecContent = await readTextFileAsync(pubspecPath); + rollbackPubspecContent[pubspecPath] = pubspecContent; + return _setSharedDependenciesForPackage( package, + pubspecPath: pubspecPath, + pubspecContent: pubspecContent, environment: bootstrapCommandConfig.environment, dependencies: bootstrapCommandConfig.dependencies, devDependencies: bootstrapCommandConfig.devDependencies, ); }).drain(); - - logger - ..child(successLabel, prefix: '> ') - ..newLine(); } logger.log( @@ -73,6 +77,18 @@ mixin _BootstrapMixin on _CleanMixin { ..child(successLabel, prefix: '> ') ..newLine(); } on BootstrapException catch (exception) { + if (rollbackPubspecContent.isNotEmpty) { + logger.log( + 'Dependency resolution failed, rolling back changes to ' + 'the pubspec.yaml files...', + ); + + await Stream.fromIterable(rollbackPubspecContent.entries) + .parallel((entry) async { + await writeTextFileAsync(entry.key, entry.value); + }).drain(); + } + _logBootstrapException(exception, workspace); rethrow; } @@ -175,13 +191,13 @@ mixin _BootstrapMixin on _CleanMixin { Future _setSharedDependenciesForPackage( Package package, { + required String pubspecPath, + required String pubspecContent, required Map? environment, required Map? dependencies, required Map? devDependencies, }) async { - final packagePubspecFile = utils.pubspecPathForDirectory(package.path); - final packagePubspecContents = await readTextFileAsync(packagePubspecFile); - final pubspecEditor = YamlEditor(packagePubspecContents); + final pubspecEditor = YamlEditor(pubspecContent); final updatedEnvironment = _updateEnvironment( pubspecEditor: pubspecEditor, @@ -204,10 +220,7 @@ mixin _BootstrapMixin on _CleanMixin { ); if (pubspecEditor.edits.isNotEmpty) { - await writeTextFileAsync( - packagePubspecFile, - pubspecEditor.toString(), - ); + await writeTextFileAsync(pubspecPath, pubspecEditor.toString()); final message = [ if (updatedEnvironment) 'Updated environment', diff --git a/packages/melos/test/commands/bootstrap_test.dart b/packages/melos/test/commands/bootstrap_test.dart index 8e0832c3..5c79075a 100644 --- a/packages/melos/test/commands/bootstrap_test.dart +++ b/packages/melos/test/commands/bootstrap_test.dart @@ -946,6 +946,139 @@ Generating IntelliJ IDE files... }); }); + test( + 'rollbacks applied shared dependencies on resolution failure', + () async { + final workspaceDir = await createTemporaryWorkspace( + workspacePackages: ['a', 'b'], + configBuilder: (path) => MelosWorkspaceConfig( + name: 'Melos', + packages: [ + createGlob('packages/**', currentDirectoryPath: path), + ], + commands: CommandConfigs( + bootstrap: BootstrapCommandConfigs( + environment: { + 'sdk': VersionConstraint.parse('>=3.6.0 <4.0.0'), + 'flutter': VersionConstraint.parse('>=2.18.0 <3.0.0'), + }, + dependencies: { + 'flame': HostedDependency( + version: VersionConstraint.compatibleWith( + // Should fail since the version is not compatible with + // the flutter version. + Version.parse('0.1.0'), + ), + ), + }, + devDependencies: { + 'flame_lint': HostedDependency( + version: VersionConstraint.compatibleWith( + Version.parse('1.2.1'), + ), + ), + }, + ), + ), + path: path, + ), + ); + + final pkgA = await createProject( + workspaceDir, + Pubspec( + 'a', + environment: {}, + dependencies: { + 'flame': HostedDependency( + version: + VersionConstraint.compatibleWith(Version.parse('1.23.0')), + ), + }, + devDependencies: { + 'flame_lint': HostedDependency( + version: VersionConstraint.compatibleWith(Version.parse('1.2.0')), + ), + }, + ), + ); + + final pkgB = await createProject( + workspaceDir, + Pubspec( + 'b', + environment: { + 'sdk': VersionConstraint.parse('>=2.12.0 <3.0.0'), + 'flutter': VersionConstraint.parse('>=2.12.0 <3.0.0'), + }, + dependencies: { + 'flame': HostedDependency( + version: + VersionConstraint.compatibleWith(Version.parse('1.23.0')), + ), + 'integral_isolates': HostedDependency( + version: VersionConstraint.compatibleWith(Version.parse('0.4.1')), + ), + 'intl': HostedDependency( + version: + VersionConstraint.compatibleWith(Version.parse('0.17.0')), + ), + 'path': HostedDependency(version: VersionConstraint.any), + }, + ), + ); + + final logger = TestLogger(); + final config = await MelosWorkspaceConfig.fromWorkspaceRoot(workspaceDir); + final melos = Melos( + logger: logger, + config: config, + ); + + final pubspecAPreBootstrap = pubspecFromYamlFile(directory: pkgA.path); + final pubspecBPreBootstrap = pubspecFromYamlFile(directory: pkgB.path); + + await expectLater( + () => runMelosBootstrap(melos, logger), + throwsA(isA()), + ); + + final pubspecA = pubspecFromYamlFile(directory: pkgA.path); + final pubspecB = pubspecFromYamlFile(directory: pkgB.path); + + expect( + pubspecAPreBootstrap.environment, + equals(defaultTestEnvironment), + ); + expect( + pubspecA.environment['sdk'], + equals(pubspecAPreBootstrap.environment['sdk']), + ); + expect( + pubspecA.dependencies, + equals(pubspecAPreBootstrap.dependencies), + ); + expect( + pubspecA.devDependencies, + equals(pubspecAPreBootstrap.devDependencies), + ); + + expect( + pubspecBPreBootstrap.environment['flutter'], + equals(VersionConstraint.parse('>=2.12.0 <3.0.0')), + ); + expect( + pubspecB.dependencies, + equals(pubspecBPreBootstrap.dependencies), + ); + expect( + pubspecB.devDependencies, + equals(pubspecBPreBootstrap.devDependencies), + ); + }, + timeout: const Timeout(Duration(minutes: 20)), + ); + group('melos bs --offline', () { test('should run pub get with --offline', () async { final workspaceDir = await createTemporaryWorkspace(