Skip to content

Don't precompile on pub get/upgrade by default #2277

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

Merged
merged 6 commits into from
Dec 12, 2019
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
2 changes: 1 addition & 1 deletion lib/src/command/get.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class GetCommand extends PubCommand {
help: "Report what dependencies would change but don't change any.");

argParser.addFlag('precompile',
defaultsTo: true,
defaultsTo: false,
help: "Precompile executables in immediate dependencies.");

argParser.addFlag('packages-dir', negatable: true, hide: true);
Expand Down
9 changes: 7 additions & 2 deletions lib/src/command/run.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,13 @@ class RunCommand extends PubCommand {

var exitCode = await runExecutable(entrypoint, package, executable, args,
checked: argResults['enable-asserts'] || argResults['checked'],
snapshotPath: useSnapshot ? snapshotPath : null,
recompile: entrypoint.precompileExecutables);
snapshotPath: useSnapshot ? snapshotPath : null, recompile: () {
final pkg = entrypoint.packageGraph.packages[package];
// The recompile function will only be called when [package] exists.
assert(pkg != null);
final executablePath = pkg.path(p.join("bin", executable));
return entrypoint.precompileExecutable(package, executablePath);
});
await flushThenExit(exitCode);
}
}
2 changes: 1 addition & 1 deletion lib/src/command/upgrade.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class UpgradeCommand extends PubCommand {
help: "Report what dependencies would change but don't change any.");

argParser.addFlag('precompile',
defaultsTo: true,
defaultsTo: false,
help: "Precompile executables in immediate dependencies.");

argParser.addFlag('packages-dir', negatable: true, hide: true);
Expand Down
30 changes: 21 additions & 9 deletions lib/src/entrypoint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class Entrypoint {
Future acquireDependencies(SolveType type,
{List<String> useLatest,
bool dryRun = false,
bool precompile = true}) async {
bool precompile = false}) async {
var result = await resolveVersions(type, cache, root,
lockFile: lockFile, useLatest: useLatest);

Expand Down Expand Up @@ -294,20 +294,32 @@ class Entrypoint {
}

//// Precompiles [executables] to snapshots from the filesystem.
Future _precompileExecutables(Map<String, List<String>> executables) {
Future<void> _precompileExecutables(Map<String, List<String>> executables) {
return waitAndPrintErrors(executables.keys.map((package) {
var dir = p.join(_snapshotPath, package);
cleanDir(dir);
return waitAndPrintErrors(executables[package].map((path) {
var url = p.toUri(p.join(packageGraph.packages[package].dir, path));
return dart.snapshot(
url, p.join(dir, p.basename(path) + '.snapshot.dart2'),
packagesFile: p.toUri(packagesFile),
name: '$package:${p.basenameWithoutExtension(path)}');
}));
return waitAndPrintErrors(executables[package]
.map((path) => _precompileExecutable(package, path)));
}));
}

/// Precompiles executable .dart file at [path] to a snapshot.
Future<void> precompileExecutable(String package, String path) async {
return await log.progress("Precompiling executable", () async {
var dir = p.join(_snapshotPath, package);
ensureDir(dir);
return waitAndPrintErrors([_precompileExecutable(package, path)]);
});
}

Future<void> _precompileExecutable(String package, String path) async {
var dir = p.join(_snapshotPath, package);
var url = p.toUri(p.join(packageGraph.packages[package].dir, path));
await dart.snapshot(url, p.join(dir, p.basename(path) + '.snapshot.dart2'),
packagesFile: p.toUri(packagesFile),
name: '$package:${p.basenameWithoutExtension(path)}');
}

/// Deletes outdated cached executable snapshots.
///
/// If [changed] is passed, only dependencies whose contents might be changed
Expand Down
6 changes: 4 additions & 2 deletions lib/src/executable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ Future<int> runExecutable(Entrypoint entrypoint, String package,
entrypoint.migrateCache();

// Unless the user overrides the verbosity, we want to filter out the
// normal pub output that may be shown when recompiling snapshots.
if (log.verbosity == log.Verbosity.NORMAL) {
// normal pub output that may be shown when recompiling snapshots if we are
// not attached to a terminal. This is to not pollute stdout when the output
// of `pub run` is piped somewhere.
if (log.verbosity == log.Verbosity.NORMAL && !stdout.hasTerminal) {
log.verbosity = log.Verbosity.WARNING;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/src/global_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class GlobalPackages {
var entrypoint = Entrypoint(path, cache);

// Get the package's dependencies.
await entrypoint.acquireDependencies(SolveType.GET);
await entrypoint.acquireDependencies(SolveType.GET, precompile: true);
var name = entrypoint.root.name;

// Call this just to log what the current active package is, if any.
Expand Down
4 changes: 3 additions & 1 deletion test/run/package_api_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ main() {
d.appPubspec({"foo": "any"})
]).create();

await pubGet(output: contains("Precompiled foo:script."));
await pubGet();

var pub = await pubRun(args: ["foo:script"]);

expect(pub.stdout, emits('Precompiling executable...'));
expect(pub.stdout, emits('Precompiled foo:script.'));
expect(pub.stdout, emits("null"));
expect(pub.stdout,
emits(p.toUri(p.join(d.sandbox, "myapp/.packages")).toString()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ main() {
d.appPubspec({"foo": null})
]).create();

await pubGet();
await pubGet(args: ['--precompile']);

var pub = await pubRun(args: ["foo:bar"]);
expect(pub.stdout, emits("foobar"));
Expand Down
37 changes: 23 additions & 14 deletions test/snapshot_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ main() {
await d.appDir({"foo": "1.2.3"}).create();

await pubGet(
args: ['--precompile'],
output: allOf([
contains("Precompiled foo:hello."),
contains("Precompiled foo:goodbye.")
]));
contains("Precompiled foo:hello."),
contains("Precompiled foo:goodbye.")
]));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin'), [
d.file('sdk-version', '0.1.2+3\n'),
Expand Down Expand Up @@ -67,10 +68,11 @@ main() {
await d.appDir({"foo": "1.2.3"}).create();

await pubGet(
args: ['--precompile'],
output: allOf([
contains("Precompiled foo:hello."),
contains("Precompiled foo:goodbye.")
]));
contains("Precompiled foo:hello."),
contains("Precompiled foo:goodbye.")
]));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin'), [
d.file('sdk-version', '0.1.2+3\n'),
Expand Down Expand Up @@ -102,7 +104,8 @@ main() {

await d.appDir({"foo": "any"}).create();

await pubGet(output: contains("Precompiled foo:hello."));
await pubGet(
args: ['--precompile'], output: contains("Precompiled foo:hello."));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin', 'foo'), [
d.file('hello.dart.snapshot.dart2', contains('hello!'))
Expand All @@ -115,7 +118,8 @@ main() {
]);
});

await pubUpgrade(output: contains("Precompiled foo:hello."));
await pubUpgrade(
args: ['--precompile'], output: contains("Precompiled foo:hello."));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin', 'foo'), [
d.file('hello.dart.snapshot.dart2', contains('hello 2!'))
Expand Down Expand Up @@ -146,7 +150,8 @@ main() {

await d.appDir({"foo": "any"}).create();

await pubGet(output: contains("Precompiled foo:hello."));
await pubGet(
args: ['--precompile'], output: contains("Precompiled foo:hello."));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin', 'foo'), [
d.file('hello.dart.snapshot.dart2', contains('hello!'))
Expand All @@ -158,7 +163,8 @@ main() {
]);
});

await pubUpgrade(output: contains("Precompiled foo:hello."));
await pubUpgrade(
args: ['--precompile'], output: contains("Precompiled foo:hello."));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin', 'foo'), [
d.file('hello.dart.snapshot.dart2', contains('hello 2!'))
Expand All @@ -182,7 +188,8 @@ main() {
"foo": {"git": "../foo.git"}
}).create();

await pubGet(output: contains("Precompiled foo:hello."));
await pubGet(
args: ['--precompile'], output: contains("Precompiled foo:hello."));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin', 'foo'), [
d.file('hello.dart.snapshot.dart2', contains('Hello!'))
Expand All @@ -193,7 +200,8 @@ main() {
[d.file("hello.dart", "void main() => print('Goodbye!');")])
]).commit();

await pubUpgrade(output: contains("Precompiled foo:hello."));
await pubUpgrade(
args: ['--precompile'], output: contains("Precompiled foo:hello."));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin', 'foo'), [
d.file('hello.dart.snapshot.dart2', contains('Goodbye!'))
Expand All @@ -214,7 +222,8 @@ main() {

await d.appDir({"foo": "5.6.7"}).create();

await pubGet(output: contains("Precompiled foo:hello."));
await pubGet(
args: ['--precompile'], output: contains("Precompiled foo:hello."));

await d.dir(p.join(appPath, '.dart_tool', 'pub', 'bin'), [
d.dir('foo', [d.outOfDateSnapshot('hello.dart.snapshot.dart2')])
Expand All @@ -224,7 +233,7 @@ main() {

// In the real world this would just print "hello!", but since we collect
// all output we see the precompilation messages as well.
expect(process.stdout, emits("Precompiling executables..."));
expect(process.stdout, emits("Precompiling executable..."));
expect(process.stdout, emitsThrough("hello!"));
await process.shouldExit();

Expand Down