From b303a3d5cfe130cf7ffd81ba13029bd2a51dd090 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Sat, 30 Apr 2022 14:07:14 +0200 Subject: [PATCH 1/2] fix: remove dependency overrides `pubspec_overrides.yaml` in `melos clean` --- packages/melos/lib/src/commands/clean.dart | 18 +- packages/melos/test/commands/clean_test.dart | 65 ++++++++ packages/melos/test/matchers.dart | 163 +++++++++++++++++++ 3 files changed, 243 insertions(+), 3 deletions(-) create mode 100644 packages/melos/test/commands/clean_test.dart diff --git a/packages/melos/lib/src/commands/clean.dart b/packages/melos/lib/src/commands/clean.dart index 8a7db2136..193d2497b 100644 --- a/packages/melos/lib/src/commands/clean.dart +++ b/packages/melos/lib/src/commands/clean.dart @@ -13,7 +13,7 @@ mixin _CleanMixin on _Melos { /// Cleans the workspace of all files generated by Melos. cleanWorkspace(workspace); - workspace.filteredPackages.values.forEach(_cleanPackage); + await Future.wait(workspace.filteredPackages.values.map(_cleanPackage)); await cleanIntelliJ(workspace); @@ -31,7 +31,7 @@ mixin _CleanMixin on _Melos { } } - void _cleanPackage(Package package) { + Future _cleanPackage(Package package) async { final pathsToClean = [ ...cleanablePubFilePaths, '.dart_tool', @@ -40,7 +40,19 @@ mixin _CleanMixin on _Melos { for (final generatedPubFilePath in pathsToClean) { final file = File(join(package.path, generatedPubFilePath)); if (file.existsSync()) { - file.deleteSync(recursive: true); + await file.delete(recursive: true); + } + } + + // Remove any Melos generated dependency overrides from + // `pubspec_overrides.yaml`. + final pubspecOverridesFile = + File(join(package.path, 'pubspec_overrides.yaml')); + if (pubspecOverridesFile.existsSync()) { + final contents = await pubspecOverridesFile.readAsString(); + final updatedContents = mergeMelosPubspecOverrides({}, contents); + if (updatedContents != null) { + await pubspecOverridesFile.writeAsString(updatedContents); } } } diff --git a/packages/melos/test/commands/clean_test.dart b/packages/melos/test/commands/clean_test.dart new file mode 100644 index 000000000..4621099b4 --- /dev/null +++ b/packages/melos/test/commands/clean_test.dart @@ -0,0 +1,65 @@ +import 'package:glob/glob.dart'; +import 'package:melos/melos.dart'; +import 'package:melos/src/common/utils.dart'; +import 'package:path/path.dart' as p; +import 'package:pub_semver/pub_semver.dart'; +import 'package:pubspec/pubspec.dart'; +import 'package:test/test.dart'; + +import '../matchers.dart'; +import '../utils.dart'; + +void main() { + group('clean', () { + test( + 'removes dependency overrides from pubspec_overrides.yaml', + () async { + final workspaceDir = createTemporaryWorkspaceDirectory( + configBuilder: (path) => MelosWorkspaceConfig( + path: path, + name: 'test_workspace', + packages: [Glob('packages/**')], + commands: const CommandConfigs( + bootstrap: BootstrapCommandConfigs( + usePubspecOverrides: true, + ), + ), + ), + ); + + await createProject(workspaceDir, const PubSpec(name: 'a')); + final packageBDir = await createProject( + workspaceDir, + PubSpec( + name: 'b', + dependencies: {'a': HostedReference(VersionConstraint.any)}, + ), + ); + final pubspecOverrides = + p.join(packageBDir.path, 'pubspec_overrides.yaml'); + + final config = await MelosWorkspaceConfig.fromDirectory(workspaceDir); + final logger = TestLogger(); + final melos = Melos(config: config, logger: logger); + await melos.bootstrap(); + + expect( + pubspecOverrides, + yamlFile({ + 'dependency_overrides': { + 'a': {'path': '../a'} + } + }), + ); + + await melos.clean(); + + expect( + pubspecOverrides, + yamlFile({'dependency_overrides': null}), + ); + }, + skip: !isPubspecOverridesSupported(), + ); + }); +} diff --git a/packages/melos/test/matchers.dart b/packages/melos/test/matchers.dart index 527fc7f16..5a6c600c5 100644 --- a/packages/melos/test/matchers.dart +++ b/packages/melos/test/matchers.dart @@ -14,9 +14,14 @@ * limitations under the License. */ +import 'dart:io' as io; + +import 'package:file/file.dart'; import 'package:melos/src/common/validation.dart'; import 'package:melos/src/package.dart'; +import 'package:path/path.dart' as p; import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; Matcher packageNamed(dynamic matcher) => _PackageNameMatcher(matcher); @@ -83,3 +88,161 @@ TypeMatcher isMelosConfigException({ Matcher throwsMelosConfigException({Object? message}) { return throwsA(isMelosConfigException(message: message)); } + +Matcher fileContents(Object? matcher) => _FileContents(wrapMatcher(matcher)); + +class _FileContents extends Matcher { + _FileContents(this._matcher); + + final Matcher _matcher; + + @override + bool matches(Object? item, Map matchState) { + final io.File file; + if (item is File) { + file = item; + } else if (item is String) { + file = io.File(p.normalize(item)); + } else { + matchState['fileContents.invalidItem'] = true; + return false; + } + + if (!file.existsSync()) { + matchState['fileContents.fileMissing'] = true; + return false; + } + + final contents = file.readAsStringSync(); + if (_matcher.matches(contents, matchState)) { + return true; + } + addStateInfo( + matchState, + {'fileContents.contents': contents}, + ); + return false; + } + + @override + Description describe(Description description) => description + ..add('file with contents that ') + ..addDescriptionOf(_matcher); + + @override + Description describeMismatch( + Object? item, + Description mismatchDescription, + Map matchState, + bool verbose, + ) { + if (matchState['fileContents.invalidItem'] == true) { + return mismatchDescription.add('is not a reference to a file'); + } + + if (matchState['fileContents.fileMissing'] == true) { + return mismatchDescription.add('does not exist'); + } + + final contents = matchState['fileContents.contents'] as String; + mismatchDescription + ..add('contains ') + ..addDescriptionOf(contents); + + final innerDescription = StringDescription(); + _matcher.describeMismatch( + contents, + innerDescription, + matchState['state'] as Map, + verbose, + ); + if (innerDescription.length > 0) { + mismatchDescription.add(' which ').add(innerDescription.toString()); + } + + return mismatchDescription; + } +} + +Matcher yaml(Object? matcher) => _Yaml(wrapMatcher(matcher)); + +class _Yaml extends Matcher { + _Yaml(this._matcher); + + final Matcher _matcher; + + @override + bool matches(Object? item, Map matchState) { + final String yamlString; + if (item is String) { + yamlString = item; + } else { + matchState['yaml.invalidItem'] = true; + return false; + } + + Object? value; + try { + value = loadYaml(yamlString); + } catch (e, s) { + matchState['yaml.error'] = e; + matchState['yaml.stack'] = s; + return false; + } + + if (_matcher.matches(value, matchState)) { + return true; + } + addStateInfo( + matchState, + {'yaml.value': value}, + ); + return false; + } + + @override + Description describe(Description description) => description + ..add('is valid Yaml string with parsed value that ') + ..addDescriptionOf(_matcher); + + @override + Description describeMismatch( + Object? item, + Description mismatchDescription, + Map matchState, + bool verbose, + ) { + if (matchState['yaml.invalidItem'] == true) { + return mismatchDescription.add(' must be a String'); + } + + final error = matchState['yaml.error'] as Object?; + final stack = matchState['yaml.stack'] as StackTrace?; + if (error != null) { + return mismatchDescription.add('could not be parsed: \n') + ..addDescriptionOf(error) + ..add('\n') + ..add(stack.toString()); + } + + final value = matchState['yaml.value'] as Object?; + mismatchDescription + ..add('has the parsed value ') + ..addDescriptionOf(value); + + final innerDescription = StringDescription(); + _matcher.describeMismatch( + value, + innerDescription, + matchState['state'] as Map, + verbose, + ); + if (innerDescription.length > 0) { + mismatchDescription.add(' which ').add(innerDescription.toString()); + } + + return mismatchDescription; + } +} + +Matcher yamlFile(Object? matcher) => fileContents(yaml(matcher)); From d66d3a3d0d0e1a1e713b6b57ff9339e59433b968 Mon Sep 17 00:00:00 2001 From: Gabriel Terwesten Date: Mon, 9 May 2022 14:16:26 +0200 Subject: [PATCH 2/2] Remove `pubspec_overrides.yaml` when empty --- .../melos/lib/src/commands/bootstrap.dart | 19 +++-- packages/melos/lib/src/commands/clean.dart | 6 +- .../melos/test/commands/bootstrap_test.dart | 4 +- packages/melos/test/commands/clean_test.dart | 2 +- packages/melos/test/matchers.dart | 69 +++++++++++++++---- 5 files changed, 76 insertions(+), 24 deletions(-) diff --git a/packages/melos/lib/src/commands/bootstrap.dart b/packages/melos/lib/src/commands/bootstrap.dart index 35e272fa4..ee87d008c 100644 --- a/packages/melos/lib/src/commands/bootstrap.dart +++ b/packages/melos/lib/src/commands/bootstrap.dart @@ -120,7 +120,7 @@ mixin _BootstrapMixin on _CleanMixin { final pubspecOverridesFile = File(utils.pubspecOverridesPathForDirectory(Directory(package.path))); final pubspecOverridesContents = pubspecOverridesFile.existsSync() - ? pubspecOverridesFile.readAsStringSync() + ? await pubspecOverridesFile.readAsString() : null; // Write new version of pubspec_overrides.yaml if it has changed. @@ -129,8 +129,13 @@ mixin _BootstrapMixin on _CleanMixin { pubspecOverridesContents, ); if (updatedPubspecOverridesContents != null) { - pubspecOverridesFile.createSync(recursive: true); - pubspecOverridesFile.writeAsStringSync(updatedPubspecOverridesContents); + if (updatedPubspecOverridesContents.isEmpty) { + await pubspecOverridesFile.delete(); + } else { + await pubspecOverridesFile.create(recursive: true); + await pubspecOverridesFile + .writeAsString(updatedPubspecOverridesContents); + } } } @@ -564,7 +569,7 @@ String? mergeMelosPubspecOverrides( // This means it is possible that dependency_overrides and/or // melos_managed_dependency_overrides are now empty. if (dependencyOverrides?.isEmpty ?? false) { - pubspecOverridesEditor.update(['dependency_overrides'], null); + pubspecOverridesEditor.remove(['dependency_overrides']); } } @@ -598,6 +603,12 @@ String? mergeMelosPubspecOverrides( } } + if (result.trim() == '{}') { + // YamlEditor uses an empty dictionary ({}) when all properties have been + // removed and the file is essentially empty. + return ''; + } + // Make sure the `pubspec_overrides.yaml` file always ends with a newline. if (result.isEmpty || !result.endsWith('\n')) { result += '\n'; diff --git a/packages/melos/lib/src/commands/clean.dart b/packages/melos/lib/src/commands/clean.dart index 193d2497b..b7007116d 100644 --- a/packages/melos/lib/src/commands/clean.dart +++ b/packages/melos/lib/src/commands/clean.dart @@ -52,7 +52,11 @@ mixin _CleanMixin on _Melos { final contents = await pubspecOverridesFile.readAsString(); final updatedContents = mergeMelosPubspecOverrides({}, contents); if (updatedContents != null) { - await pubspecOverridesFile.writeAsString(updatedContents); + if (updatedContents.isEmpty) { + await pubspecOverridesFile.delete(); + } else { + await pubspecOverridesFile.writeAsString(updatedContents); + } } } } diff --git a/packages/melos/test/commands/bootstrap_test.dart b/packages/melos/test/commands/bootstrap_test.dart index c0a386b2c..65f817855 100644 --- a/packages/melos/test/commands/bootstrap_test.dart +++ b/packages/melos/test/commands/bootstrap_test.dart @@ -381,9 +381,7 @@ dependency_overrides: a: path: ../a ''', - updatedPubspecOverrides: ''' -dependency_overrides: null -''', + updatedPubspecOverrides: '', ); }); diff --git a/packages/melos/test/commands/clean_test.dart b/packages/melos/test/commands/clean_test.dart index 4621099b4..15592da1c 100644 --- a/packages/melos/test/commands/clean_test.dart +++ b/packages/melos/test/commands/clean_test.dart @@ -56,7 +56,7 @@ void main() { expect( pubspecOverrides, - yamlFile({'dependency_overrides': null}), + isNot(fileExists), ); }, skip: !isPubspecOverridesSupported(), diff --git a/packages/melos/test/matchers.dart b/packages/melos/test/matchers.dart index 5a6c600c5..73c75760e 100644 --- a/packages/melos/test/matchers.dart +++ b/packages/melos/test/matchers.dart @@ -89,6 +89,49 @@ Matcher throwsMelosConfigException({Object? message}) { return throwsA(isMelosConfigException(message: message)); } +final Matcher fileExists = _FileExists(); + +class _FileExists extends Matcher { + _FileExists(); + + @override + bool matches(Object? item, Map matchState) { + final io.File file; + if (item is File) { + file = item; + } else if (item is String) { + file = io.File(p.normalize(item)); + } else { + matchState['fileExists.invalidItem'] = true; + return false; + } + + if (file.existsSync()) { + return true; + } + + return false; + } + + @override + Description describe(Description description) => + description.add('file exists'); + + @override + Description describeMismatch( + Object? item, + Description mismatchDescription, + Map matchState, + bool verbose, + ) { + if (matchState['fileExists.invalidItem'] == true) { + return mismatchDescription.add('is not a reference to a file'); + } + + return mismatchDescription.add('does not exist'); + } +} + Matcher fileContents(Object? matcher) => _FileContents(wrapMatcher(matcher)); class _FileContents extends Matcher { @@ -125,9 +168,8 @@ class _FileContents extends Matcher { } @override - Description describe(Description description) => description - ..add('file with contents that ') - ..addDescriptionOf(_matcher); + Description describe(Description description) => + description.add('file with contents that ').addDescriptionOf(_matcher); @override Description describeMismatch( @@ -145,9 +187,7 @@ class _FileContents extends Matcher { } final contents = matchState['fileContents.contents'] as String; - mismatchDescription - ..add('contains ') - ..addDescriptionOf(contents); + mismatchDescription.add('contains ').addDescriptionOf(contents); final innerDescription = StringDescription(); _matcher.describeMismatch( @@ -202,8 +242,8 @@ class _Yaml extends Matcher { @override Description describe(Description description) => description - ..add('is valid Yaml string with parsed value that ') - ..addDescriptionOf(_matcher); + .add('is valid Yaml string with parsed value that ') + .addDescriptionOf(_matcher); @override Description describeMismatch( @@ -219,16 +259,15 @@ class _Yaml extends Matcher { final error = matchState['yaml.error'] as Object?; final stack = matchState['yaml.stack'] as StackTrace?; if (error != null) { - return mismatchDescription.add('could not be parsed: \n') - ..addDescriptionOf(error) - ..add('\n') - ..add(stack.toString()); + return mismatchDescription + .add('could not be parsed: \n') + .addDescriptionOf(error) + .add('\n') + .add(stack.toString()); } final value = matchState['yaml.value'] as Object?; - mismatchDescription - ..add('has the parsed value ') - ..addDescriptionOf(value); + mismatchDescription.add('has the parsed value ').addDescriptionOf(value); final innerDescription = StringDescription(); _matcher.describeMismatch(