Skip to content

Commit 4b9aa2b

Browse files
a-sivaa-siva
committed
Revert "[VM/Runtime] Cleanup package config initialization code"
This reverts commit 29e93bc. Reason for revert: Windows bots are failing with package resolution errors. Looks like we are missing a windows path sanitization step. Change-Id: Ib56f7e926b4f385fa3fde74c9c71947f64b8be1c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151469 Reviewed-by: Siva Annamalai <[email protected]>
1 parent d44457f commit 4b9aa2b

File tree

16 files changed

+278
-144
lines changed

16 files changed

+278
-144
lines changed

‎pkg/vm/bin/kernel_service.dart

Lines changed: 41 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -100,26 +100,15 @@ CompilerOptions setupCompilerOptions(
100100
int nullSafety,
101101
List<String> experimentalFlags,
102102
bool bytecode,
103-
String packageConfig,
104-
String workingDirectory,
103+
Uri packagesUri,
105104
List<String> errors) {
106105
final expFlags = <String>[];
107106
if (experimentalFlags != null) {
108107
for (String flag in experimentalFlags) {
109108
expFlags.addAll(flag.split(","));
110109
}
111110
}
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-
}
111+
123112
return new CompilerOptions()
124113
..fileSystem = fileSystem
125114
..target = new VmTarget(new TargetFlags(
@@ -169,7 +158,7 @@ abstract class Compiler {
169158
final List<String> experimentalFlags;
170159
final bool bytecode;
171160
final String packageConfig;
172-
final String workingDirectory;
161+
173162
// Code coverage and hot reload are only supported by incremental compiler,
174163
// which is used if vm-service is enabled.
175164
final bool supportCodeCoverage;
@@ -187,8 +176,21 @@ abstract class Compiler {
187176
this.bytecode: false,
188177
this.supportCodeCoverage: false,
189178
this.supportHotReload: false,
190-
this.packageConfig: null,
191-
this.workingDirectory: null}) {
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+
192194
options = setupCompilerOptions(
193195
fileSystem,
194196
platformKernelPath,
@@ -197,16 +199,8 @@ abstract class Compiler {
197199
nullSafety,
198200
experimentalFlags,
199201
bytecode,
200-
packageConfig,
201-
workingDirectory,
202+
packagesUri,
202203
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-
}
210204
}
211205

212206
Future<CompilerResult> compile(Uri script) {
@@ -343,8 +337,7 @@ class IncrementalCompilerWrapper extends Compiler {
343337
int nullSafety: kNullSafetyOptionUnspecified,
344338
List<String> experimentalFlags: null,
345339
bool bytecode: false,
346-
String packageConfig: null,
347-
String workingDirectory: null})
340+
String packageConfig: null})
348341
: super(isolateId, fileSystem, platformKernelPath,
349342
suppressWarnings: suppressWarnings,
350343
enableAsserts: enableAsserts,
@@ -353,8 +346,7 @@ class IncrementalCompilerWrapper extends Compiler {
353346
bytecode: bytecode,
354347
supportHotReload: true,
355348
supportCodeCoverage: true,
356-
packageConfig: packageConfig,
357-
workingDirectory: workingDirectory);
349+
packageConfig: packageConfig);
358350

359351
factory IncrementalCompilerWrapper.forExpressionCompilationOnly(
360352
Component component,
@@ -365,8 +357,7 @@ class IncrementalCompilerWrapper extends Compiler {
365357
bool enableAsserts: false,
366358
List<String> experimentalFlags: null,
367359
bool bytecode: false,
368-
String packageConfig: null,
369-
String workingDirectory: null}) {
360+
String packageConfig: null}) {
370361
IncrementalCompilerWrapper result = IncrementalCompilerWrapper(
371362
isolateId, fileSystem, platformKernelPath,
372363
suppressWarnings: suppressWarnings,
@@ -402,8 +393,7 @@ class IncrementalCompilerWrapper extends Compiler {
402393
nullSafety: nullSafety,
403394
experimentalFlags: experimentalFlags,
404395
bytecode: bytecode,
405-
packageConfig: packageConfig,
406-
workingDirectory: workingDirectory);
396+
packageConfig: packageConfig);
407397

408398
generator.resetDeltaState();
409399
Component fullComponent = await generator.compile();
@@ -434,16 +424,14 @@ class SingleShotCompilerWrapper extends Compiler {
434424
int nullSafety: kNullSafetyOptionUnspecified,
435425
List<String> experimentalFlags: null,
436426
bool bytecode: false,
437-
String packageConfig: null,
438-
String workingDirectory: null})
427+
String packageConfig: null})
439428
: super(isolateId, fileSystem, platformKernelPath,
440429
suppressWarnings: suppressWarnings,
441430
enableAsserts: enableAsserts,
442431
nullSafety: nullSafety,
443432
experimentalFlags: experimentalFlags,
444433
bytecode: bytecode,
445-
packageConfig: packageConfig,
446-
workingDirectory: workingDirectory);
434+
packageConfig: packageConfig);
447435

448436
@override
449437
Future<CompilerResult> compileInternal(Uri script) async {
@@ -479,7 +467,6 @@ Future<Compiler> lookupOrBuildNewIncrementalCompiler(int isolateId,
479467
List<String> experimentalFlags: null,
480468
bool bytecode: false,
481469
String packageConfig: null,
482-
String workingDirectory: null,
483470
String multirootFilepaths,
484471
String multirootScheme}) async {
485472
IncrementalCompilerWrapper compiler = lookupIncrementalCompiler(isolateId);
@@ -493,8 +480,7 @@ Future<Compiler> lookupOrBuildNewIncrementalCompiler(int isolateId,
493480
if (sourceFiles != null &&
494481
sourceFiles.length > 0 &&
495482
sourceFiles[1] == null) {
496-
// Just use first compiler that should represent main isolate as a source
497-
// for cloning.
483+
// Just use first compiler that should represent main isolate as a source for cloning.
498484
var source = isolateCompilers.entries.first;
499485
compiler = await source.value.clone(isolateId);
500486
} else {
@@ -512,8 +498,7 @@ Future<Compiler> lookupOrBuildNewIncrementalCompiler(int isolateId,
512498
nullSafety: nullSafety,
513499
experimentalFlags: experimentalFlags,
514500
bytecode: bytecode,
515-
packageConfig: packageConfig,
516-
workingDirectory: workingDirectory);
501+
packageConfig: packageConfig);
517502
}
518503
isolateCompilers[isolateId] = compiler;
519504
}
@@ -837,18 +822,20 @@ Future _processLoadRequest(request) async {
837822
} else if (tag == kDetectNullabilityTag) {
838823
FileSystem fileSystem = _buildFileSystem(
839824
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+
}
840836
final List<String> errors = <String>[];
841-
var options = setupCompilerOptions(
842-
fileSystem,
843-
platformKernelPath,
844-
false,
845-
false,
846-
nullSafety,
847-
experimentalFlags,
848-
false,
849-
packageConfig,
850-
workingDirectory,
851-
errors);
837+
var options = setupCompilerOptions(fileSystem, platformKernelPath, false,
838+
false, nullSafety, experimentalFlags, false, packagesUri, errors);
852839

853840
// script should only be null for kUpdateSourcesTag.
854841
assert(script != null);
@@ -874,7 +861,6 @@ Future _processLoadRequest(request) async {
874861
experimentalFlags: experimentalFlags,
875862
bytecode: bytecode,
876863
packageConfig: packageConfig,
877-
workingDirectory: workingDirectory,
878864
multirootFilepaths: multirootFilepaths,
879865
multirootScheme: multirootScheme);
880866
} else {
@@ -888,8 +874,7 @@ Future _processLoadRequest(request) async {
888874
nullSafety: nullSafety,
889875
experimentalFlags: experimentalFlags,
890876
bytecode: bytecode,
891-
packageConfig: packageConfig,
892-
workingDirectory: workingDirectory);
877+
packageConfig: packageConfig);
893878
}
894879

895880
CompilationResult result;

‎runtime/bin/dartutils.cc

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,15 @@ 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+
305314
// TODO(iposva): Allocate from the zone instead of leaking error string
306315
// here. On the other hand the binary is about to exit anyway.
307316
#define SET_ERROR_MSG(error_msg, format, ...) \
@@ -361,6 +370,20 @@ Dart_Handle DartUtils::MakeUint8Array(const uint8_t* buffer, intptr_t len) {
361370
return array;
362371
}
363372

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+
364387
static bool CheckMagicNumber(const uint8_t* buffer,
365388
intptr_t buffer_length,
366389
const MagicNumberData& magic_number) {
@@ -442,6 +465,9 @@ Dart_Handle DartUtils::PrepareBuiltinLibrary(Dart_Handle builtin_lib,
442465
Dart_SetField(builtin_lib, NewString("_traceLoading"), Dart_True());
443466
RETURN_IF_ERROR(result);
444467
}
468+
// Set current working directory.
469+
result = SetWorkingDirectory();
470+
RETURN_IF_ERROR(result);
445471
}
446472
return Dart_True();
447473
}
@@ -488,6 +514,21 @@ Dart_Handle DartUtils::PrepareCLILibrary(Dart_Handle cli_lib) {
488514
wait_for_event_handle);
489515
}
490516

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+
491532
Dart_Handle DartUtils::PrepareForScriptLoading(bool is_service_isolate,
492533
bool trace_loading) {
493534
// First ensure all required libraries are available.

‎runtime/bin/dartutils.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ 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+
164166
static Dart_Handle SetupIOLibrary(const char* namespc_path,
165167
const char* script_uri,
166168
bool disable_exit);
@@ -223,6 +225,8 @@ class DartUtils {
223225

224226
static bool SetOriginalWorkingDirectory();
225227

228+
static Dart_Handle ResolveScript(Dart_Handle url);
229+
226230
enum MagicNumber {
227231
kAppJITMagicNumber,
228232
kKernelMagicNumber,
@@ -261,6 +265,7 @@ class DartUtils {
261265
static Dart_Handle EnvironmentCallback(Dart_Handle name);
262266

263267
private:
268+
static Dart_Handle SetWorkingDirectory();
264269
static Dart_Handle PrepareBuiltinLibrary(Dart_Handle builtin_lib,
265270
Dart_Handle internal_lib,
266271
bool is_service_isolate,

‎runtime/bin/dfe.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,7 @@ 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,
244-
DartUtils::original_working_directory);
243+
platform_strong_dill_for_compilation_size_, incremental, package_config);
245244
}
246245

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

‎runtime/bin/isolate_data.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ 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),
1819
kernel_buffer_(NULL),
1920
kernel_buffer_size_(0),
2021
isolate_run_app_snapshot_(isolate_run_app_snapshot) {
@@ -28,6 +29,8 @@ IsolateGroupData::~IsolateGroupData() {
2829
script_url = NULL;
2930
free(packages_file_);
3031
packages_file_ = NULL;
32+
free(resolved_packages_config_);
33+
resolved_packages_config_ = NULL;
3134
kernel_buffer_ = NULL;
3235
kernel_buffer_size_ = 0;
3336
}

‎runtime/bin/isolate_data.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,18 @@ 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+
7789
bool RunFromAppSnapshot() const {
7890
// If the main isolate is using an app snapshot the [app_snapshot_] pointer
7991
// will be still nullptr (see main.cc:CreateIsolateGroupAndSetupHelper)
@@ -87,6 +99,7 @@ class IsolateGroupData {
8799
friend class IsolateData; // For packages_file_
88100

89101
std::unique_ptr<AppSnapshot> app_snapshot_;
102+
char* resolved_packages_config_;
90103
std::shared_ptr<uint8_t> kernel_buffer_;
91104
intptr_t kernel_buffer_size_;
92105
char* packages_file_ = nullptr;

0 commit comments

Comments
 (0)