From c5238cd0eed8723e053717c21baa9361ccff7733 Mon Sep 17 00:00:00 2001 From: "Mateus Felipe C. C. Pinto" Date: Thu, 9 Jan 2025 17:14:14 -0300 Subject: [PATCH] fix: fix git dependencies comparison on bootstrap (#659) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixes comparison check of `GitReference` by also comparing the `path`. Fixes #658. ## Type of Change - [ ] โœจ `feat` -- New feature (non-breaking change which adds functionality) - [x] ๐Ÿ› ๏ธ `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 --------- Signed-off-by: Mateus Felipe C. C. Pinto Co-authored-by: Lukas Klingsbo --- .../melos/lib/src/commands/bootstrap.dart | 16 +- .../lib/src/common/extensions/dependency.dart | 22 +- .../melos/test/commands/bootstrap_test.dart | 214 +++++++++++++++++- packages/melos/test/pubspec_extension.dart | 42 +--- .../test/test_assets/add_to_app_json.json | 4 +- .../melos/test/test_assets/plugin_json.json | 2 +- packages/melos/test/utils.dart | 2 +- 7 files changed, 244 insertions(+), 58 deletions(-) diff --git a/packages/melos/lib/src/commands/bootstrap.dart b/packages/melos/lib/src/commands/bootstrap.dart index 40a3d572b..19c4dd75a 100644 --- a/packages/melos/lib/src/commands/bootstrap.dart +++ b/packages/melos/lib/src/commands/bootstrap.dart @@ -265,6 +265,14 @@ mixin _BootstrapMixin on _CleanMixin { return didUpdate; } + bool _areDependenciesEqual(Dependency? a, Dependency? b) { + if (a is GitDependency && b is GitDependency) { + return a == b && a.path == b.path; + } else { + return a == b; + } + } + int _updateDependencies({ required YamlEditor pubspecEditor, required Map? workspaceDependencies, @@ -276,7 +284,13 @@ mixin _BootstrapMixin on _CleanMixin { // dependencies that have a different version specified in the workspace. final dependenciesToUpdate = workspaceDependencies.entries.where((entry) { if (!packageDependencies.containsKey(entry.key)) return false; - if (packageDependencies[entry.key] == entry.value) return false; + // TODO: We may want to replace the `pubspec` dependency with something + // else that is actively maintained, so we don't have to provide our own + // equality logic. + // See: /~https://github.com/invertase/melos/discussions/663 + if (_areDependenciesEqual(packageDependencies[entry.key], entry.value)) { + return false; + } return true; }); diff --git a/packages/melos/lib/src/common/extensions/dependency.dart b/packages/melos/lib/src/common/extensions/dependency.dart index c5eb45f61..0b901fe43 100644 --- a/packages/melos/lib/src/common/extensions/dependency.dart +++ b/packages/melos/lib/src/common/extensions/dependency.dart @@ -50,12 +50,14 @@ extension HostedDependencyExtension on HostedDependency { return inlineVersion ? version.toString() : { - 'version': version.toString(), - if (hosted != null) - 'hosted': { - 'name': hosted!.declaredName, - 'url': hosted!.url?.toString(), - }, + 'hosted': { + 'version': version.toString(), + if (hosted != null) + 'hosted': { + 'name': hosted!.declaredName, + 'url': hosted!.url?.toString(), + }, + }, }; } } @@ -63,9 +65,11 @@ extension HostedDependencyExtension on HostedDependency { extension GitDependencyExtension on GitDependency { Map toJson() { return { - 'url': url.toString(), - if (ref != null) 'ref': ref, - if (path != null) 'path': path, + 'git': { + 'url': url.toString(), + if (ref != null) 'ref': ref, + if (path != null) 'path': path, + }, }; } } diff --git a/packages/melos/test/commands/bootstrap_test.dart b/packages/melos/test/commands/bootstrap_test.dart index c9dc31d06..81bbba1cf 100644 --- a/packages/melos/test/commands/bootstrap_test.dart +++ b/packages/melos/test/commands/bootstrap_test.dart @@ -1,4 +1,4 @@ -import 'dart:io' as io; +import 'dart:io'; import 'package:melos/melos.dart'; import 'package:melos/src/command_configs/command_configs.dart'; @@ -147,6 +147,213 @@ Generating IntelliJ IDE files... ); }); + test( + 'properly compares the path changes on git references', + () async { + final temporaryGitRepositoryPath = createTestTempDir().absolute.path; + + await Process.run( + 'git', + ['init'], + workingDirectory: temporaryGitRepositoryPath, + ); + + await createProject( + Directory('$temporaryGitRepositoryPath/dependency1'), + Pubspec('dependency'), + ); + + await createProject( + Directory('$temporaryGitRepositoryPath/dependency2'), + Pubspec('dependency'), + ); + + await Process.run( + 'git', + ['add', '-A'], + workingDirectory: temporaryGitRepositoryPath, + ); + + await Process.run( + 'git', + ['commit', '--message="Initial commit"'], + workingDirectory: temporaryGitRepositoryPath, + ); + + final workspaceDirectory = await createTemporaryWorkspace( + workspacePackages: [ + 'git_references', + ], + ); + + final initialReference = { + 'git': { + 'url': 'file://$temporaryGitRepositoryPath', + 'path': 'dependency1/packages/dependency', + }, + }; + + await createProject( + workspaceDirectory, + Pubspec( + 'git_references', + dependencies: { + 'dependency': GitDependency( + Uri.parse(initialReference['git']!['url']!), + path: initialReference['git']!['path'], + ), + }, + ), + ); + + final logger = TestLogger(); + final initialConfig = MelosWorkspaceConfig.fromYaml( + { + 'name': 'test', + 'packages': const ['packages/**'], + 'command': { + 'bootstrap': { + 'dependencies': { + 'dependency': initialReference, + }, + }, + }, + }, + path: workspaceDirectory.path, + ); + + final melosBeforeChangingPath = Melos( + logger: logger, + config: initialConfig, + ); + + await runMelosBootstrap(melosBeforeChangingPath, logger); + + final packageConfig = packageConfigForPackageAt(workspaceDirectory); + final dependencyPackage = packageConfig.packages.singleWhere( + (package) => package.name == 'dependency', + ); + + expect( + dependencyPackage.rootUri, + contains('dependency1/packages/dependency'), + ); + + final configWithChangedPath = MelosWorkspaceConfig.fromYaml( + { + 'name': 'test', + 'packages': const ['packages/**'], + 'command': { + 'bootstrap': { + 'dependencies': { + 'dependency': { + 'git': { + 'url': 'file://$temporaryGitRepositoryPath', + 'path': 'dependency2/packages/dependency', + }, + }, + }, + }, + }, + }, + path: workspaceDirectory.path, + ); + + final melosAfterChangingPath = Melos( + logger: logger, + config: configWithChangedPath, + ); + + await runMelosBootstrap(melosAfterChangingPath, logger); + + final alteredPackageConfig = + packageConfigForPackageAt(workspaceDirectory); + final alteredDependencyPackage = alteredPackageConfig.packages + .singleWhere((package) => package.name == 'dependency'); + + expect( + alteredDependencyPackage.rootUri, + contains('dependency2/packages/dependency'), + ); + }, + // This test works locally, but we can't create git repositories in CI. + skip: Platform.environment.containsKey('CI'), + ); + + test( + 'resolves workspace packages with path dependency', + () async { + final workspaceDir = await createTemporaryWorkspace( + workspacePackages: ['a', 'b', 'c', 'd'], + ); + + await createProject( + workspaceDir, + Pubspec( + 'a', + dependencies: { + 'b': HostedDependency(version: VersionConstraint.any), + }, + ), + ); + await createProject( + workspaceDir, + Pubspec('b'), + ); + + await createProject( + workspaceDir, + pubspecFromJsonFile(fileName: 'add_to_app_json.json'), + ); + + await createProject( + workspaceDir, + pubspecFromJsonFile(fileName: 'plugin_json.json'), + ); + + final logger = TestLogger(); + final config = + await MelosWorkspaceConfig.fromWorkspaceRoot(workspaceDir); + final melos = Melos( + logger: logger, + config: config, + ); + + await runMelosBootstrap(melos, logger); + + expect( + logger.output, + ignoringAnsii( + allOf( + [ + ''' +melos bootstrap + โ””> ${workspaceDir.path} + +Running "flutter pub get" in workspace...''', + ''' + > SUCCESS + +Generating IntelliJ IDE files... + > SUCCESS + + -> 4 packages bootstrapped +''', + ].map(contains).toList(), + ), + ), + ); + + final aConfig = packageConfigForPackageAt(workspaceDir); + + expect( + aConfig.packages.firstWhere((p) => p.name == 'b').rootUri, + '../packages/b', + ); + }, + timeout: Platform.isLinux ? const Timeout(Duration(seconds: 45)) : null, + ); + test('respects user dependency_overrides', () async { final workspaceDir = await createTemporaryWorkspace( workspacePackages: ['a'], @@ -191,7 +398,8 @@ Generating IntelliJ IDE files... test('bootstrap flutter example packages', () async { final workspaceDir = await createTemporaryWorkspace( - workspacePackages: ['a', 'a/example'], + workspacePackages: ['a'], + withExamples: true, ); await createProject( @@ -779,7 +987,7 @@ Future runMelosBootstrap( } } -YamlMap _pubspecContent(io.Directory directory) { +YamlMap _pubspecContent(Directory directory) { final source = readTextFile(pubspecPath(directory.path)); return loadYaml(source) as YamlMap; } diff --git a/packages/melos/test/pubspec_extension.dart b/packages/melos/test/pubspec_extension.dart index 18a8a1963..cc507dc6d 100644 --- a/packages/melos/test/pubspec_extension.dart +++ b/packages/melos/test/pubspec_extension.dart @@ -1,5 +1,6 @@ import 'dart:io'; +import 'package:melos/src/common/extensions/dependency.dart'; import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; @@ -100,44 +101,3 @@ extension on Screenshot { }; } } - -extension DependencyExtension on Dependency { - Map toJson() { - if (this is SdkDependency) { - final sdkDep = this as SdkDependency; - return { - 'sdk': sdkDep.sdk, - 'version': sdkDep.version.toString(), - }; - } else if (this is GitDependency) { - final gitDep = this as GitDependency; - return { - 'url': gitDep.url.toString(), - 'ref': gitDep.ref, - 'path': gitDep.path, - }; - } else if (this is PathDependency) { - final pathDep = this as PathDependency; - return { - 'path': pathDep.path, - }; - } else if (this is HostedDependency) { - final hostedDep = this as HostedDependency; - return { - 'version': hostedDep.version.toString(), - if (hostedDep.hosted != null) 'hosted': hostedDep.hosted?.toJson(), - }; - } else { - throw UnsupportedError('Unknown Dependency type'); - } - } -} - -extension HostedDetailsExtension on HostedDetails { - Map toJson() { - return { - 'name': declaredName, - 'url': url?.toString(), - }; - } -} diff --git a/packages/melos/test/test_assets/add_to_app_json.json b/packages/melos/test/test_assets/add_to_app_json.json index 7b21fdb98..27093d502 100644 --- a/packages/melos/test/test_assets/add_to_app_json.json +++ b/packages/melos/test/test_assets/add_to_app_json.json @@ -3,8 +3,8 @@ "description": "An example add to app module.", "version": "1.0.0+1", "environment": { - "sdk": ">=3.0.0 <4.0.0", - "flutter": ">=2.0.0 <=3.0.0" + "sdk": ">=3.6.0 <4.0.0", + "flutter": ">=3.27.1 <=4.0.0" }, "dependencies": { "flutter": { diff --git a/packages/melos/test/test_assets/plugin_json.json b/packages/melos/test/test_assets/plugin_json.json index 5b1dd9f25..6166de0b2 100644 --- a/packages/melos/test/test_assets/plugin_json.json +++ b/packages/melos/test/test_assets/plugin_json.json @@ -3,7 +3,7 @@ "description": "An example plugin", "version": "0.0.1", "environment": { - "sdk": ">=3.0.0 <4.0.0" + "sdk": ">=3.6.0 <4.0.0" }, "flutter": { "plugin": { diff --git a/packages/melos/test/utils.dart b/packages/melos/test/utils.dart index 4141d40bb..003e3538d 100644 --- a/packages/melos/test/utils.dart +++ b/packages/melos/test/utils.dart @@ -74,7 +74,7 @@ class TestLogger extends StandardLogger { } Directory createTestTempDir() { - final dir = createTempDir(Directory.systemTemp.path); + final dir = Directory.systemTemp.createTempSync('melos_test_').path; addTearDown(() => deleteEntry(dir)); return Directory(dir); }