Skip to content

Commit 53d859f

Browse files
derekxu16Commit Queue
authored and
Commit Queue
committed
Revert "[VM/CLI] Remove dartdev.dill"
This reverts commit c09f790. Reason for revert: CI failures Original change's description: > [VM/CLI] Remove dartdev.dill > > Incompatible VM flags will no longer break the CLI when running from an > AppJIT snapshot, so the fallback logic is no longer required. This CL > thus removes dartdev.dill and the fallback logic. > > Relevant past CLs: https://dart-review.googlesource.com/c/sdk/+/157601 > and https://dart-review.googlesource.com/c/sdk/+/178300 > > Fixes #50504 > > TEST=I tried running `out/ReleaseX64/dart --observe --sound-null-safety test.dart` > and `out/ReleaseX64/dart --observe --no-sound-null-safety test.dart` and > both worked. > > Change-Id: I5cdcfbccf71ec557964014fdb80733b4a7c76b4d > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274520 > Reviewed-by: Ben Konyi <[email protected]> > Commit-Queue: Derek Xu <[email protected]> [email protected],[email protected],[email protected] Change-Id: I5117f990dfabf93f5a9bae56098831280845e84e No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/275181 Reviewed-by: Ben Konyi <[email protected]> Commit-Queue: Derek Xu <[email protected]>
1 parent c430a3b commit 53d859f

File tree

6 files changed

+128
-16
lines changed

6 files changed

+128
-16
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:io';
6+
7+
import 'package:test/test.dart';
8+
9+
import 'utils.dart';
10+
11+
void main() {
12+
late TestProject p;
13+
14+
tearDown(() async => await p.dispose());
15+
16+
test("Fallback to dartdev.dill from dartdev.dart.snapshot for 'Hello World'",
17+
() async {
18+
p = project(mainSrc: "void main() { print('Hello World'); }");
19+
// The DartDev snapshot includes the --use_field_guards flag. If
20+
// --no-use-field-guards is passed, the VM will fail to load the
21+
// snapshot and should fall back to using the DartDev dill file.
22+
ProcessResult result =
23+
await p.run(['--no-use-field-guards', 'run', p.relativeFilePath]);
24+
25+
expect(result.stdout, contains('Hello World'));
26+
expect(result.stderr, isEmpty);
27+
expect(result.exitCode, 0);
28+
});
29+
}

runtime/bin/dartdev_isolate.cc

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,29 +59,37 @@ bool DartDevIsolate::ShouldParseCommand(const char* script_uri) {
5959
(strncmp(script_uri, "google3://", 10) != 0));
6060
}
6161

62-
Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevSnapshotPath() {
63-
const char* snapshot_filename = "dartdev.dart.snapshot";
62+
Utils::CStringUniquePtr DartDevIsolate::TryResolveArtifactPath(
63+
const char* filename) {
6464
// |dir_prefix| includes the last path separator.
6565
auto dir_prefix = EXEUtils::GetDirectoryPrefixFromExeName();
6666

6767
// First assume we're in dart-sdk/bin.
6868
char* snapshot_path =
69-
Utils::SCreate("%ssnapshots/%s", dir_prefix.get(), snapshot_filename);
69+
Utils::SCreate("%ssnapshots/%s", dir_prefix.get(), filename);
7070
if (File::Exists(nullptr, snapshot_path)) {
7171
return Utils::CreateCStringUniquePtr(snapshot_path);
7272
}
7373
free(snapshot_path);
7474

7575
// If we're not in dart-sdk/bin, we might be in one of the $SDK/out/*
7676
// directories. Try to use a snapshot from a previously built SDK.
77-
snapshot_path = Utils::SCreate("%s%s", dir_prefix.get(), snapshot_filename);
77+
snapshot_path = Utils::SCreate("%s%s", dir_prefix.get(), filename);
7878
if (File::Exists(nullptr, snapshot_path)) {
7979
return Utils::CreateCStringUniquePtr(snapshot_path);
8080
}
8181
free(snapshot_path);
8282
return Utils::CreateCStringUniquePtr(nullptr);
8383
}
8484

85+
Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevSnapshotPath() {
86+
return TryResolveArtifactPath("dartdev.dart.snapshot");
87+
}
88+
89+
Utils::CStringUniquePtr DartDevIsolate::TryResolveDartDevKernelPath() {
90+
return TryResolveArtifactPath("dartdev.dill");
91+
}
92+
8593
void DartDevIsolate::DartDevRunner::Run(
8694
Dart_IsolateGroupCreateCallback create_isolate,
8795
char** packages_file,

runtime/bin/dartdev_isolate.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ class DartDevIsolate {
4646

4747
static bool should_run_dart_dev() { return should_run_dart_dev_; }
4848

49+
// Attempts to find the path of the DartDev kernel file.
50+
static Utils::CStringUniquePtr TryResolveDartDevKernelPath();
51+
4952
// Attempts to find the path of the DartDev snapshot.
5053
static Utils::CStringUniquePtr TryResolveDartDevSnapshotPath();
5154

@@ -96,6 +99,8 @@ class DartDevIsolate {
9699
};
97100

98101
private:
102+
static Utils::CStringUniquePtr TryResolveArtifactPath(const char* filename);
103+
99104
static DartDevRunner runner_;
100105
static bool should_run_dart_dev_;
101106
static bool print_usage_error_;

runtime/bin/main.cc

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -581,35 +581,75 @@ static Dart_Isolate CreateAndSetupDartDevIsolate(const char* script_uri,
581581
char** error,
582582
int* exit_code) {
583583
int64_t start = Dart_TimelineGetMicros();
584+
584585
auto dartdev_path = DartDevIsolate::TryResolveDartDevSnapshotPath();
585-
if (dartdev_path.get() == nullptr) {
586-
Syslog::PrintErr(
587-
"Failed to start the Dart CLI isolate. Could not resolve DartDev "
588-
"snapshot.\n");
589-
return nullptr;
590-
}
586+
587+
Dart_Isolate isolate = nullptr;
588+
const uint8_t* isolate_snapshot_data = core_isolate_snapshot_data;
589+
const uint8_t* isolate_snapshot_instructions =
590+
core_isolate_snapshot_instructions;
591+
IsolateGroupData* isolate_group_data = nullptr;
592+
IsolateData* isolate_data = nullptr;
591593

592594
if (error != nullptr) {
593595
*error = nullptr;
594596
}
595-
Dart_Isolate isolate = nullptr;
596597
AppSnapshot* app_snapshot = nullptr;
597-
const bool isolate_run_app_snapshot = true;
598+
bool isolate_run_app_snapshot = true;
598599
if (dartdev_path.get() != nullptr &&
599600
(app_snapshot = Snapshot::TryReadAppSnapshot(
600601
dartdev_path.get(), /*force_load_elf_from_memory=*/false,
601602
/*decode_uri=*/false)) != nullptr) {
602-
const uint8_t* isolate_snapshot_data = nullptr;
603-
const uint8_t* isolate_snapshot_instructions = nullptr;
603+
const uint8_t* isolate_snapshot_data = NULL;
604+
const uint8_t* isolate_snapshot_instructions = NULL;
604605
const uint8_t* ignore_vm_snapshot_data;
605606
const uint8_t* ignore_vm_snapshot_instructions;
606607
app_snapshot->SetBuffers(
607608
&ignore_vm_snapshot_data, &ignore_vm_snapshot_instructions,
608609
&isolate_snapshot_data, &isolate_snapshot_instructions);
609-
IsolateGroupData* isolate_group_data =
610+
isolate_group_data =
610611
new IsolateGroupData(DART_DEV_ISOLATE_NAME, packages_config,
611612
app_snapshot, isolate_run_app_snapshot);
612-
IsolateData* isolate_data = new IsolateData(isolate_group_data);
613+
isolate_data = new IsolateData(isolate_group_data);
614+
isolate = Dart_CreateIsolateGroup(
615+
DART_DEV_ISOLATE_NAME, DART_DEV_ISOLATE_NAME, isolate_snapshot_data,
616+
isolate_snapshot_instructions, flags, isolate_group_data, isolate_data,
617+
error);
618+
}
619+
620+
if (isolate == nullptr) {
621+
isolate_run_app_snapshot = false;
622+
dartdev_path = DartDevIsolate::TryResolveDartDevKernelPath();
623+
// Clear error from app snapshot and retry from kernel.
624+
if (error != nullptr && *error != nullptr) {
625+
free(*error);
626+
*error = nullptr;
627+
}
628+
629+
if (app_snapshot != nullptr) {
630+
delete app_snapshot;
631+
}
632+
633+
if (dartdev_path.get() == nullptr) {
634+
Syslog::PrintErr(
635+
"Failed to start the Dart CLI isolate. Could not resolve DartDev "
636+
"snapshot or kernel.\n");
637+
delete isolate_data;
638+
delete isolate_group_data;
639+
return nullptr;
640+
}
641+
642+
isolate_group_data =
643+
new IsolateGroupData(DART_DEV_ISOLATE_NAME, packages_config, nullptr,
644+
isolate_run_app_snapshot);
645+
uint8_t* application_kernel_buffer = NULL;
646+
intptr_t application_kernel_buffer_size = 0;
647+
dfe.ReadScript(dartdev_path.get(), &application_kernel_buffer,
648+
&application_kernel_buffer_size, /*decode_uri=*/false);
649+
isolate_group_data->SetKernelBufferNewlyOwned(
650+
application_kernel_buffer, application_kernel_buffer_size);
651+
652+
isolate_data = new IsolateData(isolate_group_data);
613653
isolate = Dart_CreateIsolateGroup(
614654
DART_DEV_ISOLATE_NAME, DART_DEV_ISOLATE_NAME, isolate_snapshot_data,
615655
isolate_snapshot_instructions, flags, isolate_group_data, isolate_data,

sdk/BUILD.gn

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ declare_args() {
4545
# ........dart2js.dart.snapshot
4646
# ........dart2wasm_product.snapshot (if not on ia32)
4747
# ........dartdev.dart.snapshot
48+
# ........dartdev.dill
4849
# ........dartdevc.dart.snapshot
4950
# ........dds.dart.snapshot
5051
# ........frontend_server.dart.snapshot
@@ -454,6 +455,17 @@ copy("copy_vm_dill_files") {
454455
[ "$root_out_dir/$dart_sdk_output/lib/_internal/{{source_file_part}}" ]
455456
}
456457

458+
copy("copy_dartdev_dill_files") {
459+
visibility = [ ":create_common_sdk" ]
460+
deps = [
461+
":copy_libraries",
462+
"../utils/dartdev:dartdev",
463+
]
464+
sources = [ "$root_out_dir/dartdev.dill" ]
465+
outputs =
466+
[ "$root_out_dir/$dart_sdk_output/bin/snapshots/{{source_file_part}}" ]
467+
}
468+
457469
copy("copy_dart2js_dill_files") {
458470
visibility = [ ":create_full_sdk" ]
459471
deps = [
@@ -753,6 +765,7 @@ group("create_common_sdk") {
753765
":copy_analysis_summaries",
754766
":copy_api_readme",
755767
":copy_dart",
768+
":copy_dartdev_dill_files",
756769
":copy_dartdoc_files",
757770
":copy_headers",
758771
":copy_libraries_dart",

utils/dartdev/BUILD.gn

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,28 @@ import("../application_snapshot.gni")
77

88
group("dartdev") {
99
public_deps = [
10+
":copy_dartdev_kernel",
1011
":copy_dartdev_snapshot",
1112
":copy_prebuilt_devtools",
1213
]
1314
}
1415

16+
copy("copy_dartdev_kernel") {
17+
visibility = [ ":dartdev" ]
18+
public_deps = [ ":generate_dartdev_kernel" ]
19+
sources = [ "$root_gen_dir/dartdev.dill" ]
20+
outputs = [ "$root_out_dir/dartdev.dill" ]
21+
}
22+
23+
application_snapshot("generate_dartdev_kernel") {
24+
dart_snapshot_kind = "kernel"
25+
main_dart = "../../pkg/dartdev/bin/dartdev.dart"
26+
training_args = []
27+
deps = [ "../dds:dds" ]
28+
vm_args = [ "--sound-null-safety" ]
29+
output = "$root_gen_dir/dartdev.dill"
30+
}
31+
1532
copy("copy_dartdev_snapshot") {
1633
visibility = [ ":dartdev" ]
1734
public_deps = [ ":generate_dartdev_snapshot" ]

0 commit comments

Comments
 (0)