Skip to content

Add missing await (and make downstream async) #3949

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

Merged
merged 2 commits into from
Jun 20, 2023
Merged

Add missing await (and make downstream async) #3949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a01a802
Add missing await (and make downstream async)
sigurdm Jun 20, 2023
0fe424c
Merge branch 'master' into fix_package_graph
sigurdm Jun 20, 2023
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
94 changes: 56 additions & 38 deletions lib/src/command/deps.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,13 @@ class DepsCommand extends PubCommand {
final visited = <String>[];
final toVisit = [entrypoint.root.name];
final packagesJson = <dynamic>[];
final graph = await entrypoint.packageGraph;
while (toVisit.isNotEmpty) {
final current = toVisit.removeLast();
if (visited.contains(current)) continue;
visited.add(current);
final currentPackage = entrypoint.packageGraph.packages[current]!;
final currentPackage =
(await entrypoint.packageGraph).packages[current]!;
final next = (current == entrypoint.root.name
? entrypoint.root.immediateDependencies
: currentPackage.dependencies)
Expand Down Expand Up @@ -126,7 +128,7 @@ class DepsCommand extends PubCommand {
for (final package in [
entrypoint.root,
...entrypoint.root.immediateDependencies.keys
.map((name) => entrypoint.packageGraph.packages[name]),
.map((name) => graph.packages[name]),
])
...package!.executableNames.map(
(name) => package == entrypoint.root
Expand All @@ -151,7 +153,7 @@ class DepsCommand extends PubCommand {
);
} else {
if (argResults.flag('executables')) {
_outputExecutables(buffer);
await _outputExecutables(buffer);
} else {
for (var sdk in sdks.values) {
if (!sdk.isAvailable) continue;
Expand All @@ -162,13 +164,13 @@ class DepsCommand extends PubCommand {

switch (argResults['style']) {
case 'compact':
_outputCompact(buffer);
await _outputCompact(buffer);
break;
case 'list':
_outputList(buffer);
await _outputList(buffer);
break;
case 'tree':
_outputTree(buffer);
await _outputTree(buffer);
break;
}
}
Expand All @@ -183,40 +185,44 @@ class DepsCommand extends PubCommand {
/// For each dependency listed, *that* package's immediate dependencies are
/// shown. Unlike [_outputList], this prints all of these dependencies on one
/// line.
void _outputCompact(
Future<void> _outputCompact(
StringBuffer buffer,
) {
) async {
var root = entrypoint.root;
_outputCompactPackages('dependencies', root.dependencies.keys, buffer);
await _outputCompactPackages(
'dependencies',
root.dependencies.keys,
buffer,
);
if (_includeDev) {
_outputCompactPackages(
await _outputCompactPackages(
'dev dependencies',
root.devDependencies.keys,
buffer,
);
}
_outputCompactPackages(
await _outputCompactPackages(
'dependency overrides',
root.dependencyOverrides.keys,
buffer,
);

var transitive = _getTransitiveDependencies();
_outputCompactPackages('transitive dependencies', transitive, buffer);
var transitive = await _getTransitiveDependencies();
await _outputCompactPackages('transitive dependencies', transitive, buffer);
}

/// Outputs one section of packages in the compact output.
void _outputCompactPackages(
Future<void> _outputCompactPackages(
String section,
Iterable<String> names,
StringBuffer buffer,
) {
) async {
if (names.isEmpty) return;

buffer.writeln();
buffer.writeln('$section:');
for (var name in ordered(names)) {
var package = _getPackage(name);
var package = await _getPackage(name);

buffer.write('- ${_labelPackage(package)}');
if (package.dependencies.isEmpty) {
Expand All @@ -234,37 +240,45 @@ class DepsCommand extends PubCommand {
///
/// For each dependency listed, *that* package's immediate dependencies are
/// shown.
void _outputList(StringBuffer buffer) {
Future<void> _outputList(StringBuffer buffer) async {
var root = entrypoint.root;
_outputListSection('dependencies', root.dependencies.keys, buffer);
await _outputListSection('dependencies', root.dependencies.keys, buffer);
if (_includeDev) {
_outputListSection('dev dependencies', root.devDependencies.keys, buffer);
await _outputListSection(
'dev dependencies',
root.devDependencies.keys,
buffer,
);
}
_outputListSection(
await _outputListSection(
'dependency overrides',
root.dependencyOverrides.keys,
buffer,
);

var transitive = _getTransitiveDependencies();
var transitive = await _getTransitiveDependencies();
if (transitive.isEmpty) return;

_outputListSection('transitive dependencies', ordered(transitive), buffer);
await _outputListSection(
'transitive dependencies',
ordered(transitive),
buffer,
);
}

/// Outputs one section of packages in the list output.
void _outputListSection(
Future<void> _outputListSection(
String name,
Iterable<String> deps,
StringBuffer buffer,
) {
) async {
if (deps.isEmpty) return;

buffer.writeln();
buffer.writeln('$name:');

for (var name in deps) {
var package = _getPackage(name);
var package = await _getPackage(name);
buffer.writeln('- ${_labelPackage(package)}');

for (var dep in package.dependencies.values) {
Expand All @@ -279,9 +293,9 @@ class DepsCommand extends PubCommand {
/// dependency), later ones are not traversed. This is done in breadth-first
/// fashion so that a package will always be expanded at the shallowest
/// depth that it appears at.
void _outputTree(
Future<void> _outputTree(
StringBuffer buffer,
) {
) async {
// The work list for the breadth-first traversal. It contains the package
// being added to the tree, and the parent map that will receive that
// package.
Expand All @@ -296,7 +310,7 @@ class DepsCommand extends PubCommand {
immediateDependencies.removeAll(entrypoint.root.devDependencies.keys);
}
for (var name in immediateDependencies) {
toWalk.add(Pair(_getPackage(name), packageTree));
toWalk.add(Pair(await _getPackage(name), packageTree));
}

// Do a breadth-first walk to the dependency graph.
Expand All @@ -317,7 +331,7 @@ class DepsCommand extends PubCommand {
map[_labelPackage(package)] = childMap;

for (var dep in package.dependencies.values) {
toWalk.add(Pair(_getPackage(dep.name), childMap));
toWalk.add(Pair(await _getPackage(dep.name), childMap));
}
}

Expand All @@ -328,8 +342,8 @@ class DepsCommand extends PubCommand {
'${log.bold(package.name)} ${package.version}';

/// Gets the names of the non-immediate dependencies of the root package.
Set<String> _getTransitiveDependencies() {
var transitive = _getAllDependencies();
Future<Set<String>> _getTransitiveDependencies() async {
var transitive = await _getAllDependencies();
var root = entrypoint.root;
transitive.remove(root.name);
transitive.removeAll(root.dependencies.keys);
Expand All @@ -340,13 +354,16 @@ class DepsCommand extends PubCommand {
return transitive;
}

Set<String> _getAllDependencies() {
if (_includeDev) return entrypoint.packageGraph.packages.keys.toSet();
Future<Set<String>> _getAllDependencies() async {
final graph = await entrypoint.packageGraph;
if (_includeDev) {
return graph.packages.keys.toSet();
}

var nonDevDependencies = entrypoint.root.dependencies.keys.toList()
..addAll(entrypoint.root.dependencyOverrides.keys);
return nonDevDependencies
.expand((name) => entrypoint.packageGraph.transitiveDependencies(name))
.expand(graph.transitiveDependencies)
.map((package) => package.name)
.toSet();
}
Expand All @@ -357,22 +374,23 @@ class DepsCommand extends PubCommand {
/// It's very unlikely that the lockfile won't be up-to-date with the pubspec,
/// but it's possible, since [Entrypoint.assertUpToDate]'s modification time
/// check can return a false negative. This fails gracefully if that happens.
Package _getPackage(String name) {
var package = entrypoint.packageGraph.packages[name];
Future<Package> _getPackage(String name) async {
var package = (await entrypoint.packageGraph).packages[name];
if (package != null) return package;
dataError('The pubspec.yaml file has changed since the pubspec.lock file '
'was generated, please run "$topLevelProgram pub get" again.');
}

/// Outputs all executables reachable from [entrypoint].
void _outputExecutables(StringBuffer buffer) {
Future<void> _outputExecutables(StringBuffer buffer) async {
final graph = await entrypoint.packageGraph;
var packages = [
entrypoint.root,
...(_includeDev
? entrypoint.root.immediateDependencies
: entrypoint.root.dependencies)
.keys
.map((name) => entrypoint.packageGraph.packages[name]!),
.map((name) => graph.packages[name]!),
];

for (var package in packages) {
Expand Down
44 changes: 25 additions & 19 deletions lib/src/entrypoint.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,14 @@ class Entrypoint {
///
/// Throws a [DataError] if the `.dart_tool/package_config.json` file isn't
/// up-to-date relative to the pubspec and the lockfile.
PackageGraph get packageGraph => _packageGraph ??= _createPackageGraph();

PackageGraph _createPackageGraph() {
ensureUpToDate(); // TODO, really?
Future<PackageGraph> get packageGraph =>
_packageGraph ??= _createPackageGraph();

Future<PackageGraph> _createPackageGraph() async {
// TODO(sigurdm): consider having [ensureUptoDate] and [AcquireDependencies]
// return the package-graph, such it by construction will always made from an
// up-to-date package-config.
await ensureUpToDate();
var packages = {
for (var packageEntry in packageConfig.nonInjectedPackages)
packageEntry.name: Package.load(
Expand All @@ -184,7 +188,7 @@ class Entrypoint {
return PackageGraph(this, packages);
}

PackageGraph? _packageGraph;
Future<PackageGraph>? _packageGraph;

/// Where the lock file and package configurations are to be found.
///
Expand Down Expand Up @@ -275,7 +279,8 @@ class Entrypoint {
SolveResult? solveResult,
}) : rootDir = _root.dir {
if (solveResult != null) {
_packageGraph = PackageGraph.fromSolveResult(this, solveResult);
_packageGraph =
Future.value(PackageGraph.fromSolveResult(this, solveResult));
}
}

Expand Down Expand Up @@ -421,15 +426,15 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without

/// Build a package graph from the version solver results so we don't
/// have to reload and reparse all the pubspecs.
_packageGraph = PackageGraph.fromSolveResult(this, result);
_packageGraph = Future.value(PackageGraph.fromSolveResult(this, result));

await writePackageConfigFile();

try {
if (precompile) {
await precompileExecutables();
} else {
_deleteExecutableSnapshots(changed: result.changedPackages);
await _deleteExecutableSnapshots(changed: result.changedPackages);
}
} catch (error, stackTrace) {
// Just log exceptions here. Since the method is just about acquiring
Expand All @@ -446,7 +451,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
///
/// Except globally activated packages they should precompile executables from
/// the package itself if they are immutable.
List<Executable> get _builtExecutables {
Future<List<Executable>> get _builtExecutables async {
if (isGlobal) {
if (isCached) {
return root.executablePaths
Expand All @@ -456,8 +461,9 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
return <Executable>[];
}
}
final graph = await packageGraph;
final r = root.immediateDependencies.keys.expand((packageName) {
final package = packageGraph.packages[packageName]!;
final package = graph.packages[packageName]!;
return package.executablePaths
.map((path) => Executable(packageName, path));
}).toList();
Expand All @@ -466,7 +472,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without

/// Precompiles all [_builtExecutables].
Future<void> precompileExecutables() async {
final executables = _builtExecutables;
final executables = await _builtExecutables;

if (executables.isEmpty) return;

Expand Down Expand Up @@ -524,7 +530,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
final package = executable.package;

await dart.precompile(
executablePath: resolveExecutable(executable),
executablePath: await resolveExecutable(executable),
outputPath: pathOfExecutable(executable),
incrementalDillPath: incrementalDillPathOfExecutable(executable),
packageConfigPath: packageConfigPath,
Expand Down Expand Up @@ -572,9 +578,9 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
}

/// The absolute path of [executable] resolved relative to [this].
String resolveExecutable(Executable executable) {
Future<String> resolveExecutable(Executable executable) async {
return p.join(
packageGraph.packages[executable.package]!.dir,
(await packageGraph).packages[executable.package]!.dir,
executable.relativePath,
);
}
Expand All @@ -583,7 +589,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
///
/// If [changed] is passed, only dependencies whose contents might be changed
/// if one of the given packages changes will have their executables deleted.
void _deleteExecutableSnapshots({Iterable<String>? changed}) {
Future<void> _deleteExecutableSnapshots({Iterable<String>? changed}) async {
if (!dirExists(_snapshotPath)) return;

// If we don't know what changed, we can't safely re-use any snapshots.
Expand All @@ -603,15 +609,15 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without
deleteEntry(_snapshotPath);
return;
}
final graph = await packageGraph;

// Clean out any outdated snapshots.
for (var entry in listDir(_snapshotPath)) {
if (!dirExists(entry)) continue;

var package = p.basename(entry);
if (!packageGraph.packages.containsKey(package) ||
packageGraph.isPackageMutable(package) ||
packageGraph
if (!graph.packages.containsKey(package) ||
graph.isPackageMutable(package) ||
graph
.transitiveDependencies(package)
.any((dep) => changedDeps.contains(dep.name))) {
deleteEntry(entry);
Expand Down
Loading