Skip to content

Commit

Permalink
Consider sdk packages immutable (#4394)
Browse files Browse the repository at this point in the history
  • Loading branch information
sigurdm authored Sep 25, 2024
1 parent 3894534 commit 1892c46
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 9 deletions.
19 changes: 10 additions & 9 deletions lib/src/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:graphs/graphs.dart';
import 'package:path/path.dart' as p;

import 'entrypoint.dart';
import 'package.dart';
import 'solver.dart';
import 'source/cached.dart';
import 'source/sdk.dart';
import 'utils.dart';

/// A holistic view of the entire transitive dependency graph for an entrypoint.
Expand Down Expand Up @@ -78,12 +79,12 @@ class PackageGraph {
return _transitiveDependencies![package]!;
}

bool _isPackageCached(String package) {
// The root package is not included in the lock file, so we instead ask
// the entrypoint.
// TODO(sigurdm): there should be a way to get the id of any package
// including the root.
return p.isWithin(entrypoint.cache.rootDir, packages[package]!.dir);
bool _isPackageFromImmutableSource(String package) {
final id = entrypoint.lockFile.packages[package];
if (id == null) {
return false; // This is a root package.
}
return id.source is CachedSource || id.source is SdkSource;
}

/// Returns whether [package] is mutable.
Expand All @@ -93,9 +94,9 @@ class PackageGraph {
/// without modifying the pub cache. Information generated from mutable
/// packages is generally not safe to cache, since it may change frequently.
bool isPackageMutable(String package) {
if (!_isPackageCached(package)) return true;
if (!_isPackageFromImmutableSource(package)) return true;

return transitiveDependencies(package)
.any((dep) => !_isPackageCached(dep.name));
.any((dep) => !_isPackageFromImmutableSource(dep.name));
}
}
77 changes: 77 additions & 0 deletions test/embedding/embedding_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,83 @@ main() {
);
});

test(
'`embedding run` does not recompile executables '
'from packages depending on sdk packages', () async {
final server = await servePackages();
server.serve(
'hosted',
'1.0.0',
deps: {
'foo': {'sdk': 'flutter'},
},
contents: [
d.dir('bin', [d.file('hosted.dart', 'main() {print(42);}')]),
],
);
await d.dir('flutter', [
d.dir('bin', [
d.dir('cache', [
d.file(
'flutter.version.json',
'{"flutterVersion": "1.2.3"}',
),
]),
]),
d.dir('packages', [
d.dir('foo', [
d.libPubspec('foo', '1.2.3'),
]),
]),
]).create();

await d.dir(appPath, [
d.pubspec({
'name': 'myapp',
'dependencies': {
'hosted': '^1.0.0',
},
}),
]).create();

final buffer = StringBuffer();
await runEmbeddingToBuffer(
['run', 'hosted'],
buffer,
workingDirectory: d.path(appPath),
environment: {
'FLUTTER_ROOT': p.join(d.sandbox, 'flutter'),
EnvironmentKeys.forceTerminalOutput: '1',
},
);

expect(
buffer.toString(),
allOf(
contains('Built hosted:hosted'),
contains('42'),
),
);

final buffer2 = StringBuffer();
await runEmbeddingToBuffer(
['run', 'hosted'],
buffer2,
workingDirectory: d.path(appPath),
environment: {
'FLUTTER_ROOT': p.join(d.sandbox, 'flutter'),
EnvironmentKeys.forceTerminalOutput: '1',
},
);
expect(
buffer2.toString(),
allOf(
isNot(contains('Built hosted:hosted')),
contains('42'),
),
);
});

test('"pkg" and "packages" will trigger a suggestion of "pub"', () async {
await servePackages();
await d.appDir().create();
Expand Down

0 comments on commit 1892c46

Please sign in to comment.