-
Notifications
You must be signed in to change notification settings - Fork 33
wip - add-overrides command #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file | ||
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
import 'dart:io'; | ||
|
||
import 'package:path/path.dart' as p; | ||
import 'package:pubspec_parse/pubspec_parse.dart'; | ||
import 'package:yaml/yaml.dart'; | ||
import 'package:yaml_edit/yaml_edit.dart'; | ||
|
||
import '../../mono_repo.dart'; | ||
import '../root_config.dart'; | ||
import 'mono_repo_command.dart'; | ||
|
||
class AddOverridesCommand extends MonoRepoCommand { | ||
@override | ||
String get name => 'add-overrides'; | ||
|
||
@override | ||
String get description => | ||
'Generates the CI configuration for child packages.'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update :) |
||
|
||
@override | ||
void run() => addOverrides(rootConfig()); | ||
} | ||
|
||
void addOverrides(RootConfig config) { | ||
final dependencyOverrides = config.monoConfig.dependencyOverrides; | ||
if (dependencyOverrides.isEmpty) { | ||
throw UserException('No dependency_overrides defined!'); | ||
} | ||
|
||
final updateQueue = <String, String>{}; | ||
|
||
for (var package in config) { | ||
print(package.relativePath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cruft? |
||
final pubspecPath = p.join(package.relativePath, 'pubspec.yaml'); | ||
final lockPath = p.join(package.relativePath, 'pubspec.lock'); | ||
final lockFile = File(lockPath); | ||
|
||
final lockYaml = | ||
loadYaml(lockFile.readAsStringSync(), sourceUrl: Uri.parse(lockPath)) | ||
as YamlMap; | ||
final packages = lockYaml['packages'] as YamlMap; | ||
|
||
final toOverride = packages.keys | ||
.cast<String>() | ||
.where((element) => dependencyOverrides.keys.contains(element)) | ||
.toSet(); | ||
|
||
if (toOverride.isEmpty) { | ||
print('Nothing to update in "$pubspecPath".'); | ||
continue; | ||
} | ||
|
||
final pubspecFile = File(pubspecPath); | ||
final pubspecYaml = YamlEditor(pubspecFile.readAsStringSync()); | ||
|
||
final pubspecOverrides = pubspecYaml | ||
.parseAt(['dependency_overrides'], orElse: () => wrapAsYamlNode(null)); | ||
|
||
if (pubspecOverrides is YamlScalar && pubspecOverrides.value == null) { | ||
// no overrides! | ||
pubspecYaml.update(['dependency_overrides'], {}); | ||
} else if (pubspecOverrides is! YamlMap) { | ||
throw UserException( | ||
'"${pubspecFile.path}" has a `dependency_overrides` value, but it is ' | ||
'not a Map. Not sure what to do!', | ||
); | ||
} | ||
|
||
// We're ready to start adding dependency overrides! | ||
for (var newOverride in toOverride) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: pkgToOverride, or just pkg or package? |
||
final newDep = dependencyOverrides[newOverride]!; | ||
|
||
Map<String, dynamic> newValue; | ||
|
||
if (newDep is PathDependency) { | ||
final path = p.isAbsolute(newDep.path) | ||
? newDep.path | ||
: p.relative(p.absolute(newDep.path), from: package.relativePath); | ||
newValue = {'path': path}; | ||
} else if (newDep is GitDependency) { | ||
newValue = { | ||
'git': <String, String>{ | ||
'url': newDep.url.toString(), | ||
if (newDep.path != null) 'path': newDep.path!, | ||
if (newDep.ref != null) 'ref': newDep.ref!, | ||
} | ||
}; | ||
} else { | ||
// TODO: support "normal" dependencies | ||
throw UserException('Not sure how to write `$newDep`.'); | ||
} | ||
|
||
// TODO: if there is already a value at the target and it doesn't equal | ||
// the value we're adding – throw! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think maybe just log? If I change the overrides in my mono_repo.yaml, I want that to overwrite previously written things. |
||
|
||
pubspecYaml.update( | ||
['dependency_overrides', newOverride], | ||
newValue, | ||
); | ||
} | ||
|
||
updateQueue[pubspecPath] = pubspecYaml.toString(); | ||
} | ||
|
||
assert(updateQueue.isNotEmpty); | ||
for (var entry in updateQueue.entries) { | ||
File(entry.key).writeAsStringSync(entry.value); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
|
||
import 'package:json_annotation/json_annotation.dart'; | ||
import 'package:path/path.dart' as p; | ||
// ignore: implementation_imports | ||
import 'package:pubspec_parse/src/dependency.dart'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to expose this? We could also do something funky where we create a fake pubspec and parse that to avoid the impl import? |
||
|
||
import 'github_config.dart'; | ||
import 'yaml.dart'; | ||
|
@@ -16,6 +18,7 @@ const _monoConfigFileName = 'mono_repo.yaml'; | |
const _reservedTravisKeys = {'cache', 'jobs', 'language'}; | ||
|
||
const _allowedMonoConfigKeys = { | ||
'dependency_overrides', | ||
'github', | ||
'merge_stages', | ||
'pretty_ansi', | ||
|
@@ -41,6 +44,7 @@ class MonoConfig { | |
final String? selfValidateStage; | ||
final Map<String, dynamic> travis; | ||
final GitHubConfig github; | ||
final Map<String, Dependency> dependencyOverrides; | ||
|
||
MonoConfig._({ | ||
required Set<CI>? ci, | ||
|
@@ -52,6 +56,7 @@ class MonoConfig { | |
required this.selfValidateStage, | ||
required this.travis, | ||
required this.github, | ||
required this.dependencyOverrides, | ||
}) : ci = ci ?? const {CI.travis}; | ||
|
||
factory MonoConfig({ | ||
|
@@ -62,6 +67,7 @@ class MonoConfig { | |
required String? selfValidateStage, | ||
required Map travis, | ||
required Map github, | ||
required Map dependencyOverrides, | ||
}) { | ||
final overlappingKeys = | ||
travis.keys.where(_reservedTravisKeys.contains).toList(); | ||
|
@@ -87,6 +93,7 @@ class MonoConfig { | |
selfValidateStage: selfValidateStage, | ||
travis: travis.map((k, v) => MapEntry(k as String, v))..remove('stages'), | ||
github: GitHubConfig.fromJson(github), | ||
dependencyOverrides: parseDeps(dependencyOverrides), | ||
); | ||
} | ||
|
||
|
@@ -184,6 +191,7 @@ class MonoConfig { | |
selfValidateStage: _selfValidateFromValue(selfValidate), | ||
travis: travis, | ||
github: github, | ||
dependencyOverrides: json['dependency_overrides'] as Map? ?? {}, | ||
); | ||
} else { | ||
throw CheckedFromJsonException( | ||
|
@@ -208,6 +216,7 @@ class MonoConfig { | |
selfValidateStage: null, | ||
travis: {}, | ||
github: {}, | ||
dependencyOverrides: {}, | ||
); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably we also want a
RemoveOverridesCommand
(for publishing?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely. Or one should just revert?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is common to leave overrides in though as well (see the test repo). But then you need to remove them before publishing.
A bit of a tangent but now that I think about it this could also be useful for setting dependency constraints more generally as well? We could support a
dependencies
(anddev_dependencies
) section where you could set the versions of all deps that you want to be consistent across packages.