Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: 'auto' option for runPubGetInParallel #731

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: 'auto' option for runPubGetInParallel #731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be17cdf
feat: 'auto' option for runPubGetInParallel
tomassasovsky Jun 19, 2024
8ab74d9
docs: update API documentation to reflect new changes from PR
tomassasovsky Jun 19, 2024
e226196
refactor: use a switch case for readability when processing ParallelP…
tomassasovsky Jun 19, 2024
6de71d8
chore: change ArgumentError name field from 'name' to 'value'
tomassasovsky Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/configuration/overview.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ dependencyOverridePaths:

Whether to run `pub get` in parallel during bootstrapping.

The default is `true`.
The default is `auto`, which fallbacks to `false` if the machine is a CI instance.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this change will make into main, I would suggest adding explanation to this line. Why is this the default, what is the issue it resolves, and what could someone who changes this to true expect.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you look into that @tomassasovsky?


### runPubGetOffline

Expand Down
73 changes: 59 additions & 14 deletions packages/melos/lib/src/command_configs/bootstrap.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import '../lifecycle_hooks/lifecycle_hooks.dart';
@immutable
class BootstrapCommandConfigs {
const BootstrapCommandConfigs({
this.runPubGetInParallel = true,
this.parallelPubGetMode = ParallelPubGetMode.auto,
this.runPubGetOffline = false,
this.enforceLockfile = false,
this.environment,
Expand All @@ -27,12 +27,31 @@ class BootstrapCommandConfigs {
Map<Object?, Object?> yaml, {
required String workspacePath,
}) {
final runPubGetInParallel = assertKeyIsA<bool?>(
key: 'runPubGetInParallel',
map: yaml,
path: 'command/bootstrap',
) ??
true;
ParallelPubGetMode parallelPubGetMode;
try {
final runPubGetInParallel = assertKeyIsA<bool?>(
key: 'runPubGetInParallel',
map: yaml,
path: 'command/bootstrap',
);
if (runPubGetInParallel != null) {
parallelPubGetMode = runPubGetInParallel
? ParallelPubGetMode.enabled
: ParallelPubGetMode.disabled;
} else {
parallelPubGetMode = ParallelPubGetMode.auto;
}
} on MelosConfigException {
final runPubGetInParallel = assertKeyIsA<String?>(
key: 'runPubGetInParallel',
map: yaml,
path: 'command/bootstrap',
) ??
'auto';
parallelPubGetMode = ParallelPubGetMode.fromValue(runPubGetInParallel);
} catch (_) {
rethrow;
}

final runPubGetOffline = assertKeyIsA<bool?>(
key: 'runPubGetOffline',
Expand Down Expand Up @@ -94,7 +113,7 @@ class BootstrapCommandConfigs {
: LifecycleHooks.empty;

return BootstrapCommandConfigs(
runPubGetInParallel: runPubGetInParallel,
parallelPubGetMode: parallelPubGetMode,
runPubGetOffline: runPubGetOffline,
enforceLockfile: enforceLockfile,
environment: environment,
Expand All @@ -114,8 +133,9 @@ class BootstrapCommandConfigs {

/// Whether to run `pub get` in parallel during bootstrapping.
///
/// The default is `true`.
final bool runPubGetInParallel;
/// The default is `auto`, which will run `pub get` in parallel if the `CI`
/// environment variable is not set.
final ParallelPubGetMode parallelPubGetMode;

/// Whether to attempt to run `pub get` in offline mode during bootstrapping.
/// Useful in closed network environments with pre-populated pubcaches.
Expand Down Expand Up @@ -148,7 +168,7 @@ class BootstrapCommandConfigs {

Map<String, Object?> toJson() {
return {
'runPubGetInParallel': runPubGetInParallel,
'runPubGetInParallel': parallelPubGetMode.name,
'runPubGetOffline': runPubGetOffline,
'enforceLockfile': enforceLockfile,
if (environment != null) 'environment': environment!.toJson(),
Expand All @@ -171,7 +191,7 @@ class BootstrapCommandConfigs {
bool operator ==(Object other) =>
other is BootstrapCommandConfigs &&
runtimeType == other.runtimeType &&
other.runPubGetInParallel == runPubGetInParallel &&
other.parallelPubGetMode == parallelPubGetMode &&
other.runPubGetOffline == runPubGetOffline &&
other.enforceLockfile == enforceLockfile &&
// Extracting equality from environment here as it does not implement ==
Expand All @@ -190,7 +210,7 @@ class BootstrapCommandConfigs {
@override
int get hashCode =>
runtimeType.hashCode ^
runPubGetInParallel.hashCode ^
parallelPubGetMode.hashCode ^
runPubGetOffline.hashCode ^
enforceLockfile.hashCode ^
// Extracting hashCode from environment here as it does not implement
Expand All @@ -209,7 +229,7 @@ class BootstrapCommandConfigs {
String toString() {
return '''
BootstrapCommandConfigs(
runPubGetInParallel: $runPubGetInParallel,
runPubGetInParallel: $parallelPubGetMode,
runPubGetOffline: $runPubGetOffline,
enforceLockfile: $enforceLockfile,
environment: $environment,
Expand All @@ -220,3 +240,28 @@ BootstrapCommandConfigs(
)''';
}
}

enum ParallelPubGetMode {
auto,
enabled,
disabled;

factory ParallelPubGetMode.fromValue(String value) {
switch (value) {
case 'auto':
return ParallelPubGetMode.auto;
case 'true':
case 'enabled':
return ParallelPubGetMode.enabled;
case 'false':
case 'disabled':
return ParallelPubGetMode.disabled;
default:
throw ArgumentError.value(
value,
'value',
'Invalid value for ParallelPubGetMode',
);
}
}
}
14 changes: 12 additions & 2 deletions packages/melos/lib/src/commands/bootstrap.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,17 @@ mixin _BootstrapMixin on _CleanMixin {
required bool noExample,
}) async {
final filteredPackages = workspace.filteredPackages.values;
final isCI = utils.isCI;

bool parallelism;
tomassasovsky marked this conversation as resolved.
Show resolved Hide resolved
switch (workspace.config.commands.bootstrap.parallelPubGetMode) {
case ParallelPubGetMode.auto:
parallelism = !isCI;
case ParallelPubGetMode.enabled:
parallelism = true;
case ParallelPubGetMode.disabled:
parallelism = false;
}

await Stream.fromIterable(filteredPackages).parallel(
(package) async {
Expand Down Expand Up @@ -143,8 +154,7 @@ mixin _BootstrapMixin on _CleanMixin {

bootstrappedPackages.forEach(_logBootstrapSuccess);
},
parallelism:
workspace.config.commands.bootstrap.runPubGetInParallel ? null : 1,
parallelism: parallelism ? null : 1,
).drain<void>();
}

Expand Down
6 changes: 3 additions & 3 deletions packages/melos/test/workspace_config_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ void main() {
// ignore: use_named_constants
const value = BootstrapCommandConfigs();

expect(value.runPubGetInParallel, true);
expect(value.parallelPubGetMode, ParallelPubGetMode.auto);
});

group('fromYaml', () {
Expand Down Expand Up @@ -49,7 +49,7 @@ void main() {
workspacePath: '.',
),
BootstrapCommandConfigs(
runPubGetInParallel: false,
parallelPubGetMode: ParallelPubGetMode.disabled,
runPubGetOffline: true,
enforceLockfile: true,
dependencyOverridePaths: [
Expand Down Expand Up @@ -243,7 +243,7 @@ void main() {
// ignore: avoid_redundant_argument_values, use_named_constants
bootstrap: BootstrapCommandConfigs(
// ignore: avoid_redundant_argument_values
runPubGetInParallel: true,
parallelPubGetMode: ParallelPubGetMode.enabled,
),
),
);
Expand Down
Loading