Skip to content

Commit

Permalink
[stable] [ CLI ] Fix DART_VM_OPTIONS usage only resulting in printing…
Browse files Browse the repository at this point in the history
… of help message

Fixes #55767

TEST=pkg/dartdev/test/commands/compile_test.dart

Change-Id: I6a773acbd9fc21c086fc459c7cb983ea1ff11fcd
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/367720
Cherry-pick-request: #55818
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367721
Reviewed-by: Siva Annamalai <[email protected]>
Commit-Queue: Ben Konyi <[email protected]>
  • Loading branch information
bkonyi authored and Commit Queue committed May 30, 2024
1 parent 9b9939e commit 16c3fef
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 14 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 3.4.3

This is a patch release that:

- Fixes an issue where `DART_VM_OPTIONS` were not correctly parsed for
standalone Dart executables created with `dart compile exe` (issue [#55767]).

[#55767]: https://github.com/dart-lang/sdk/issues/55767

## 3.4.2 - 2024-05-29

This is a patch release that:
Expand Down
27 changes: 24 additions & 3 deletions pkg/dartdev/test/commands/compile_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -687,8 +687,10 @@ void main() {
}, skip: isRunningOnIA32);

test('Compile and run exe with DART_VM_OPTIONS', () async {
final p = project(mainSrc: '''void main() {
// Empty
final p = project(mainSrc: '''
import 'dart:math';
void main() {
print(Random().nextInt(1000));
}''');
final inFile = path.canonicalize(path.join(p.dirPath, p.relativeFilePath));
final outFile = path.canonicalize(path.join(p.dirPath, 'myexe'));
Expand All @@ -709,6 +711,7 @@ void main() {
expect(File(outFile).existsSync(), true,
reason: 'File not found: $outFile');

// Verify CSV options are processed.
result = Process.runSync(
outFile,
[],
Expand All @@ -718,8 +721,26 @@ void main() {
);

expect(result.stderr, isEmpty);
// vm_name is a verbose flag and will only be shown if --verbose is
// processed.
expect(result.stdout, contains('vm_name'));
expect(result.exitCode, 255);
expect(result.exitCode, 0);

// Verify non-help options work.
//
// Regression test for https://github.com/dart-lang/sdk/issues/55767
result = Process.runSync(
outFile,
[],
environment: <String, String>{
'DART_VM_OPTIONS': '--random_seed=42',
},
);

expect(result.stderr, isEmpty);
// This value should be consistent as long as --random_seed is processed.
expect(result.stdout, contains('64'));
expect(result.exitCode, 0);
}, skip: isRunningOnIA32);

test('Compile exe without info', () async {
Expand Down
13 changes: 8 additions & 5 deletions runtime/bin/main_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1195,10 +1195,11 @@ void main(int argc, char** argv) {

auto parse_arguments = [&](int argc, char** argv,
CommandLineOptions* vm_options,
CommandLineOptions* dart_options) {
CommandLineOptions* dart_options,
bool parsing_dart_vm_options) {
bool success = Options::ParseArguments(
argc, argv, vm_run_app_snapshot, vm_options, &script_name, dart_options,
&print_flags_seen, &verbose_debug_seen);
argc, argv, vm_run_app_snapshot, parsing_dart_vm_options, vm_options,
&script_name, dart_options, &print_flags_seen, &verbose_debug_seen);
if (!success) {
if (Options::help_option()) {
Options::PrintUsage();
Expand Down Expand Up @@ -1255,15 +1256,17 @@ void main(int argc, char** argv) {
// Any Dart options that are generated based on parsing DART_VM_OPTIONS
// are useless, so we'll throw them away rather than passing them along.
CommandLineOptions tmp_options(env_argc + EXTRA_VM_ARGUMENTS);
parse_arguments(env_argc, env_argv, &vm_options, &tmp_options);
parse_arguments(env_argc, env_argv, &vm_options, &tmp_options,
/*parsing_dart_vm_options=*/true);
}
}
}
#endif

// Parse command line arguments.
if (app_snapshot == nullptr) {
parse_arguments(argc, argv, &vm_options, &dart_options);
parse_arguments(argc, argv, &vm_options, &dart_options,
/*parsing_dart_vm_options=*/false);
}

DartUtils::SetEnvironment(Options::environment());
Expand Down
31 changes: 25 additions & 6 deletions runtime/bin/main_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -502,16 +502,24 @@ bool Options::ProcessVMDebuggingOptions(const char* arg,
bool Options::ParseArguments(int argc,
char** argv,
bool vm_run_app_snapshot,
bool parsing_dart_vm_options,
CommandLineOptions* vm_options,
char** script_name,
CommandLineOptions* dart_options,
bool* print_flags_seen,
bool* verbose_debug_seen) {
// Store the executable name.
Platform::SetExecutableName(argv[0]);
int i = 0;
#if !defined(DART_PRECOMPILED_RUNTIME)
// DART_VM_OPTIONS is only implemented for compiled executables.
ASSERT(!parsing_dart_vm_options);
#endif // !defined(DART_PRECOMPILED_RUNTIME)
if (!parsing_dart_vm_options) {
// Store the executable name.
Platform::SetExecutableName(argv[0]);

// Start the rest after the executable name.
int i = 1;
// Start the rest after the executable name.
i = 1;
}

CommandLineOptions temp_vm_options(vm_options->max_count());

Expand Down Expand Up @@ -667,8 +675,10 @@ bool Options::ParseArguments(int argc,
return false;
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)
// Handle argument parsing errors.
else { // NOLINT
// Handle argument parsing errors and missing script / command name when not
// processing options set via DART_VM_OPTIONS.
else if (!parsing_dart_vm_options || Options::help_option() || // NOLINT
Options::version_option()) { // NOLINT
return false;
}
USE(enable_dartdev_analytics);
Expand All @@ -680,6 +690,15 @@ bool Options::ParseArguments(int argc,

vm_options->AddArguments(vm_argv, vm_argc);

#if !defined(DART_PRECOMPILED_RUNTIME)
// If we're parsing DART_VM_OPTIONS, there shouldn't be any script set or
// Dart arguments left to parse.
if (parsing_dart_vm_options) {
ASSERT(i == argc);
return true;
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)

// If running with dartdev, attempt to parse VM flags which are part of the
// dartdev command (e.g., --enable-vm-service, --observe, etc).
if (!run_script) {
Expand Down
1 change: 1 addition & 0 deletions runtime/bin/main_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class Options {
static bool ParseArguments(int argc,
char** argv,
bool vm_run_app_snapshot,
bool parsing_dart_vm_options,
CommandLineOptions* vm_options,
char** script_name,
CommandLineOptions* dart_options,
Expand Down

0 comments on commit 16c3fef

Please sign in to comment.