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.

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

Consider sdk packages immutable #4394

Merged
merged 1 commit into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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