Skip to content

Commit

Permalink
fix: fix git dependencies comparison on bootstrap (#659)
Browse files Browse the repository at this point in the history
## Description

Fixes comparison check of `GitReference` by also comparing the `path`.

Fixes #658.

## Type of Change

<!--- Put an `x` in all the boxes that apply: -->

- [ ] ✨ `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 <mateusfccp@gmail.com>
Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
  • Loading branch information
mateusfccp and spydon authored Jan 9, 2025
1 parent 4def209 commit c5238cd
Show file tree
Hide file tree
Showing 7 changed files with 244 additions and 58 deletions.
16 changes: 15 additions & 1 deletion packages/melos/lib/src/commands/bootstrap.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Dependency>? workspaceDependencies,
Expand All @@ -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;
});

Expand Down
22 changes: 13 additions & 9 deletions packages/melos/lib/src/common/extensions/dependency.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,26 @@ 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(),
},
},
};
}
}

extension GitDependencyExtension on GitDependency {
Map<String, dynamic> 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,
},
};
}
}
Expand Down
214 changes: 211 additions & 3 deletions packages/melos/test/commands/bootstrap_test.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -779,7 +987,7 @@ Future<void> runMelosBootstrap(
}
}

YamlMap _pubspecContent(io.Directory directory) {
YamlMap _pubspecContent(Directory directory) {
final source = readTextFile(pubspecPath(directory.path));
return loadYaml(source) as YamlMap;
}
42 changes: 1 addition & 41 deletions packages/melos/test/pubspec_extension.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -100,44 +101,3 @@ extension on Screenshot {
};
}
}

extension DependencyExtension on Dependency {
Map<String, dynamic> 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<String, dynamic> toJson() {
return {
'name': declaredName,
'url': url?.toString(),
};
}
}
4 changes: 2 additions & 2 deletions packages/melos/test/test_assets/add_to_app_json.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion packages/melos/test/test_assets/plugin_json.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion packages/melos/test/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit c5238cd

Please sign in to comment.