Skip to content

Commit 29e93bc

Browse files
a-sivacommit-bot@chromium.org
a-siva
authored and
[email protected]
committed
[VM/Runtime] Cleanup package config initialization code
- Use _Init function for initialization of package config - Remove DartUtils::SetWorkingDirectory, SingleArgDart_Invoke, DartUtils::SetupPackageConfig - Rename Loader::InitForSnapshot to Loader::Init - Remove _setPackagesConfig, _setWorkingDirectory from builtin library - Rename _setPackagesMap to _setPackagesConfig Change-Id: I0b83574593fa95e1b948134fcd4a7b4858ea19c2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151045 Commit-Queue: Siva Annamalai <[email protected]> Reviewed-by: Alexander Aprelev <[email protected]>
1 parent 07e2921 commit 29e93bc

File tree

17 files changed

+170
-330
lines changed

17 files changed

+170
-330
lines changed

‎pkg/vm/bin/kernel_service.dart‎

Lines changed: 56 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,26 @@ CompilerOptions setupCompilerOptions(
100100
int nullSafety,
101101
List<String> experimentalFlags,
102102
bool bytecode,
103-
Uri packagesUri,
103+
String packageConfig,
104+
String workingDirectory,
104105
List<String> errors) {
105106
final expFlags = <String>[];
106107
if (experimentalFlags != null) {
107108
for (String flag in experimentalFlags) {
108109
expFlags.addAll(flag.split(","));
109110
}
110111
}
111-
112+
Uri packagesUri = null;
113+
if (packageConfig != null) {
114+
packagesUri = Uri.parse(packageConfig);
115+
if (packagesUri.scheme == '') {
116+
// Script does not have a scheme, assume that it is a path,
117+
// resolve it against the working directory.
118+
packagesUri = Uri.directory(workingDirectory).resolveUri(packagesUri);
119+
}
120+
} else if (Platform.packageConfig != null) {
121+
packagesUri = Uri.parse(Platform.packageConfig);
122+
}
112123
return new CompilerOptions()
113124
..fileSystem = fileSystem
114125
..target = new VmTarget(new TargetFlags(
@@ -158,7 +169,7 @@ abstract class Compiler {
158169
final List<String> experimentalFlags;
159170
final bool bytecode;
160171
final String packageConfig;
161-
172+
final String workingDirectory;
162173
// Code coverage and hot reload are only supported by incremental compiler,
163174
// which is used if vm-service is enabled.
164175
final bool supportCodeCoverage;
@@ -176,21 +187,8 @@ abstract class Compiler {
176187
this.bytecode: false,
177188
this.supportCodeCoverage: false,
178189
this.supportHotReload: false,
179-
this.packageConfig: null}) {
180-
Uri packagesUri = null;
181-
if (packageConfig != null) {
182-
packagesUri = Uri.parse(packageConfig);
183-
} else if (Platform.packageConfig != null) {
184-
packagesUri = Uri.parse(Platform.packageConfig);
185-
}
186-
187-
if (verbose) {
188-
print("DFE: Platform.packageConfig: ${Platform.packageConfig}");
189-
print("DFE: packagesUri: ${packagesUri}");
190-
print("DFE: Platform.resolvedExecutable: ${Platform.resolvedExecutable}");
191-
print("DFE: platformKernelPath: ${platformKernelPath}");
192-
}
193-
190+
this.packageConfig: null,
191+
this.workingDirectory: null}) {
194192
options = setupCompilerOptions(
195193
fileSystem,
196194
platformKernelPath,
@@ -199,8 +197,16 @@ abstract class Compiler {
199197
nullSafety,
200198
experimentalFlags,
201199
bytecode,
202-
packagesUri,
200+
packageConfig,
201+
workingDirectory,
203202
errors);
203+
204+
if (verbose) {
205+
print("DFE: Platform.packageConfig: ${Platform.packageConfig}");
206+
print("DFE: packagesUri: ${options.packagesFileUri}");
207+
print("DFE: Platform.resolvedExecutable: ${Platform.resolvedExecutable}");
208+
print("DFE: platformKernelPath: ${platformKernelPath}");
209+
}
204210
}
205211

206212
Future<CompilerResult> compile(Uri script) {
@@ -337,7 +343,8 @@ class IncrementalCompilerWrapper extends Compiler {
337343
int nullSafety: kNullSafetyOptionUnspecified,
338344
List<String> experimentalFlags: null,
339345
bool bytecode: false,
340-
String packageConfig: null})
346+
String packageConfig: null,
347+
String workingDirectory: null})
341348
: super(isolateId, fileSystem, platformKernelPath,
342349
suppressWarnings: suppressWarnings,
343350
enableAsserts: enableAsserts,
@@ -346,7 +353,8 @@ class IncrementalCompilerWrapper extends Compiler {
346353
bytecode: bytecode,
347354
supportHotReload: true,
348355
supportCodeCoverage: true,
349-
packageConfig: packageConfig);
356+
packageConfig: packageConfig,
357+
workingDirectory: workingDirectory);
350358

351359
factory IncrementalCompilerWrapper.forExpressionCompilationOnly(
352360
Component component,
@@ -357,7 +365,8 @@ class IncrementalCompilerWrapper extends Compiler {
357365
bool enableAsserts: false,
358366
List<String> experimentalFlags: null,
359367
bool bytecode: false,
360-
String packageConfig: null}) {
368+
String packageConfig: null,
369+
String workingDirectory: null}) {
361370
IncrementalCompilerWrapper result = IncrementalCompilerWrapper(
362371
isolateId, fileSystem, platformKernelPath,
363372
suppressWarnings: suppressWarnings,
@@ -393,7 +402,8 @@ class IncrementalCompilerWrapper extends Compiler {
393402
nullSafety: nullSafety,
394403
experimentalFlags: experimentalFlags,
395404
bytecode: bytecode,
396-
packageConfig: packageConfig);
405+
packageConfig: packageConfig,
406+
workingDirectory: workingDirectory);
397407

398408
generator.resetDeltaState();
399409
Component fullComponent = await generator.compile();
@@ -424,14 +434,16 @@ class SingleShotCompilerWrapper extends Compiler {
424434
int nullSafety: kNullSafetyOptionUnspecified,
425435
List<String> experimentalFlags: null,
426436
bool bytecode: false,
427-
String packageConfig: null})
437+
String packageConfig: null,
438+
String workingDirectory: null})
428439
: super(isolateId, fileSystem, platformKernelPath,
429440
suppressWarnings: suppressWarnings,
430441
enableAsserts: enableAsserts,
431442
nullSafety: nullSafety,
432443
experimentalFlags: experimentalFlags,
433444
bytecode: bytecode,
434-
packageConfig: packageConfig);
445+
packageConfig: packageConfig,
446+
workingDirectory: workingDirectory);
435447

436448
@override
437449
Future<CompilerResult> compileInternal(Uri script) async {
@@ -467,6 +479,7 @@ Future<Compiler> lookupOrBuildNewIncrementalCompiler(int isolateId,
467479
List<String> experimentalFlags: null,
468480
bool bytecode: false,
469481
String packageConfig: null,
482+
String workingDirectory: null,
470483
String multirootFilepaths,
471484
String multirootScheme}) async {
472485
IncrementalCompilerWrapper compiler = lookupIncrementalCompiler(isolateId);
@@ -480,7 +493,8 @@ Future<Compiler> lookupOrBuildNewIncrementalCompiler(int isolateId,
480493
if (sourceFiles != null &&
481494
sourceFiles.length > 0 &&
482495
sourceFiles[1] == null) {
483-
// Just use first compiler that should represent main isolate as a source for cloning.
496+
// Just use first compiler that should represent main isolate as a source
497+
// for cloning.
484498
var source = isolateCompilers.entries.first;
485499
compiler = await source.value.clone(isolateId);
486500
} else {
@@ -498,7 +512,8 @@ Future<Compiler> lookupOrBuildNewIncrementalCompiler(int isolateId,
498512
nullSafety: nullSafety,
499513
experimentalFlags: experimentalFlags,
500514
bytecode: bytecode,
501-
packageConfig: packageConfig);
515+
packageConfig: packageConfig,
516+
workingDirectory: workingDirectory);
502517
}
503518
isolateCompilers[isolateId] = compiler;
504519
}
@@ -822,20 +837,18 @@ Future _processLoadRequest(request) async {
822837
} else if (tag == kDetectNullabilityTag) {
823838
FileSystem fileSystem = _buildFileSystem(
824839
sourceFiles, platformKernel, multirootFilepaths, multirootScheme);
825-
Uri packagesUri = null;
826-
if (packageConfig != null) {
827-
packagesUri = Uri.parse(packageConfig);
828-
} else if (Platform.packageConfig != null) {
829-
packagesUri = Uri.parse(Platform.packageConfig);
830-
}
831-
if (packagesUri != null && packagesUri.scheme == '') {
832-
// Script does not have a scheme, assume that it is a path,
833-
// resolve it against the working directory.
834-
packagesUri = Uri.directory(workingDirectory).resolveUri(packagesUri);
835-
}
836840
final List<String> errors = <String>[];
837-
var options = setupCompilerOptions(fileSystem, platformKernelPath, false,
838-
false, nullSafety, experimentalFlags, false, packagesUri, errors);
841+
var options = setupCompilerOptions(
842+
fileSystem,
843+
platformKernelPath,
844+
false,
845+
false,
846+
nullSafety,
847+
experimentalFlags,
848+
false,
849+
packageConfig,
850+
workingDirectory,
851+
errors);
839852

840853
// script should only be null for kUpdateSourcesTag.
841854
assert(script != null);
@@ -861,6 +874,7 @@ Future _processLoadRequest(request) async {
861874
experimentalFlags: experimentalFlags,
862875
bytecode: bytecode,
863876
packageConfig: packageConfig,
877+
workingDirectory: workingDirectory,
864878
multirootFilepaths: multirootFilepaths,
865879
multirootScheme: multirootScheme);
866880
} else {
@@ -874,7 +888,8 @@ Future _processLoadRequest(request) async {
874888
nullSafety: nullSafety,
875889
experimentalFlags: experimentalFlags,
876890
bytecode: bytecode,
877-
packageConfig: packageConfig);
891+
packageConfig: packageConfig,
892+
workingDirectory: workingDirectory);
878893
}
879894

880895
CompilationResult result;

‎runtime/bin/dartutils.cc‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -302,15 +302,6 @@ bool DartUtils::EntropySource(uint8_t* buffer, intptr_t length) {
302302
return Crypto::GetRandomBytes(length, buffer);
303303
}
304304

305-
static Dart_Handle SingleArgDart_Invoke(Dart_Handle lib,
306-
const char* method,
307-
Dart_Handle arg) {
308-
const int kNumArgs = 1;
309-
Dart_Handle dart_args[kNumArgs];
310-
dart_args[0] = arg;
311-
return Dart_Invoke(lib, DartUtils::NewString(method), kNumArgs, dart_args);
312-
}
313-
314305
// TODO(iposva): Allocate from the zone instead of leaking error string
315306
// here. On the other hand the binary is about to exit anyway.
316307
#define SET_ERROR_MSG(error_msg, format, ...) \
@@ -370,20 +361,6 @@ Dart_Handle DartUtils::MakeUint8Array(const uint8_t* buffer, intptr_t len) {
370361
return array;
371362
}
372363

373-
Dart_Handle DartUtils::SetWorkingDirectory() {
374-
Dart_Handle directory = NewString(original_working_directory);
375-
return SingleArgDart_Invoke(LookupBuiltinLib(), "_setWorkingDirectory",
376-
directory);
377-
}
378-
379-
Dart_Handle DartUtils::ResolveScript(Dart_Handle url) {
380-
const int kNumArgs = 1;
381-
Dart_Handle dart_args[kNumArgs];
382-
dart_args[0] = url;
383-
return Dart_Invoke(DartUtils::LookupBuiltinLib(),
384-
NewString("_resolveScriptUri"), kNumArgs, dart_args);
385-
}
386-
387364
static bool CheckMagicNumber(const uint8_t* buffer,
388365
intptr_t buffer_length,
389366
const MagicNumberData& magic_number) {
@@ -465,9 +442,6 @@ Dart_Handle DartUtils::PrepareBuiltinLibrary(Dart_Handle builtin_lib,
465442
Dart_SetField(builtin_lib, NewString("_traceLoading"), Dart_True());
466443
RETURN_IF_ERROR(result);
467444
}
468-
// Set current working directory.
469-
result = SetWorkingDirectory();
470-
RETURN_IF_ERROR(result);
471445
}
472446
return Dart_True();
473447
}
@@ -514,21 +488,6 @@ Dart_Handle DartUtils::PrepareCLILibrary(Dart_Handle cli_lib) {
514488
wait_for_event_handle);
515489
}
516490

517-
Dart_Handle DartUtils::SetupPackageConfig(const char* packages_config) {
518-
Dart_Handle result = Dart_Null();
519-
520-
if (packages_config != NULL) {
521-
result = NewString(packages_config);
522-
RETURN_IF_ERROR(result);
523-
const int kNumArgs = 1;
524-
Dart_Handle dart_args[kNumArgs];
525-
dart_args[0] = result;
526-
result = Dart_Invoke(DartUtils::LookupBuiltinLib(),
527-
NewString("_setPackagesMap"), kNumArgs, dart_args);
528-
}
529-
return result;
530-
}
531-
532491
Dart_Handle DartUtils::PrepareForScriptLoading(bool is_service_isolate,
533492
bool trace_loading) {
534493
// First ensure all required libraries are available.

‎runtime/bin/dartutils.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,6 @@ class DartUtils {
161161
static Dart_Handle MakeUint8Array(const uint8_t* buffer, intptr_t length);
162162
static Dart_Handle PrepareForScriptLoading(bool is_service_isolate,
163163
bool trace_loading);
164-
static Dart_Handle SetupPackageConfig(const char* packages_file);
165-
166164
static Dart_Handle SetupIOLibrary(const char* namespc_path,
167165
const char* script_uri,
168166
bool disable_exit);
@@ -225,8 +223,6 @@ class DartUtils {
225223

226224
static bool SetOriginalWorkingDirectory();
227225

228-
static Dart_Handle ResolveScript(Dart_Handle url);
229-
230226
enum MagicNumber {
231227
kAppJITMagicNumber,
232228
kKernelMagicNumber,
@@ -265,7 +261,6 @@ class DartUtils {
265261
static Dart_Handle EnvironmentCallback(Dart_Handle name);
266262

267263
private:
268-
static Dart_Handle SetWorkingDirectory();
269264
static Dart_Handle PrepareBuiltinLibrary(Dart_Handle builtin_lib,
270265
Dart_Handle internal_lib,
271266
bool is_service_isolate,

‎runtime/bin/dfe.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ Dart_KernelCompilationResult DFE::CompileScript(const char* script_uri,
240240

241241
return Dart_CompileToKernel(
242242
sanitized_uri, platform_strong_dill_for_compilation_,
243-
platform_strong_dill_for_compilation_size_, incremental, package_config);
243+
platform_strong_dill_for_compilation_size_, incremental, package_config,
244+
DartUtils::original_working_directory);
244245
}
245246

246247
void DFE::CompileAndReadScript(const char* script_uri,

‎runtime/bin/isolate_data.cc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ IsolateGroupData::IsolateGroupData(const char* url,
1515
bool isolate_run_app_snapshot)
1616
: script_url((url != NULL) ? strdup(url) : NULL),
1717
app_snapshot_(app_snapshot),
18-
resolved_packages_config_(NULL),
1918
kernel_buffer_(NULL),
2019
kernel_buffer_size_(0),
2120
isolate_run_app_snapshot_(isolate_run_app_snapshot) {
@@ -29,8 +28,6 @@ IsolateGroupData::~IsolateGroupData() {
2928
script_url = NULL;
3029
free(packages_file_);
3130
packages_file_ = NULL;
32-
free(resolved_packages_config_);
33-
resolved_packages_config_ = NULL;
3431
kernel_buffer_ = NULL;
3532
kernel_buffer_size_ = 0;
3633
}

‎runtime/bin/isolate_data.h‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -74,18 +74,6 @@ class IsolateGroupData {
7474
kernel_buffer_size_ = size;
7575
}
7676

77-
const char* resolved_packages_config() const {
78-
return resolved_packages_config_;
79-
}
80-
81-
void set_resolved_packages_config(const char* packages_config) {
82-
if (resolved_packages_config_ != NULL) {
83-
free(resolved_packages_config_);
84-
resolved_packages_config_ = NULL;
85-
}
86-
resolved_packages_config_ = strdup(packages_config);
87-
}
88-
8977
bool RunFromAppSnapshot() const {
9078
// If the main isolate is using an app snapshot the [app_snapshot_] pointer
9179
// will be still nullptr (see main.cc:CreateIsolateGroupAndSetupHelper)
@@ -99,7 +87,6 @@ class IsolateGroupData {
9987
friend class IsolateData; // For packages_file_
10088

10189
std::unique_ptr<AppSnapshot> app_snapshot_;
102-
char* resolved_packages_config_;
10390
std::shared_ptr<uint8_t> kernel_buffer_;
10491
intptr_t kernel_buffer_size_;
10592
char* packages_file_ = nullptr;

0 commit comments

Comments
 (0)