From 0c547d2833e9ccdc88d36b3d187354905af7df27 Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Wed, 22 May 2024 20:06:49 +0000 Subject: [PATCH] [ CLI ] Fix DART_VM_OPTIONS usage only resulting in printing of help message Fixes https://github.com/dart-lang/sdk/issues/55767 TEST=pkg/dartdev/test/commands/compile_test.dart Change-Id: I6a773acbd9fc21c086fc459c7cb983ea1ff11fcd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367720 Auto-Submit: Ben Konyi Reviewed-by: Derek Xu Commit-Queue: Derek Xu --- pkg/dartdev/test/commands/compile_test.dart | 27 ++++++++++++++++-- runtime/bin/main_impl.cc | 13 +++++---- runtime/bin/main_options.cc | 31 +++++++++++++++++---- runtime/bin/main_options.h | 1 + 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/pkg/dartdev/test/commands/compile_test.dart b/pkg/dartdev/test/commands/compile_test.dart index a6ae4aaeaa8..c76a15b18fc 100644 --- a/pkg/dartdev/test/commands/compile_test.dart +++ b/pkg/dartdev/test/commands/compile_test.dart @@ -688,8 +688,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')); @@ -710,6 +712,7 @@ void main() { expect(File(outFile).existsSync(), true, reason: 'File not found: $outFile'); + // Verify CSV options are processed. result = Process.runSync( outFile, [], @@ -719,8 +722,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: { + '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 { diff --git a/runtime/bin/main_impl.cc b/runtime/bin/main_impl.cc index 0e9331b4e0e..fd9493ab1a7 100644 --- a/runtime/bin/main_impl.cc +++ b/runtime/bin/main_impl.cc @@ -1177,10 +1177,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(); @@ -1237,7 +1238,8 @@ 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); } } } @@ -1245,7 +1247,8 @@ void main(int argc, char** argv) { // 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()); diff --git a/runtime/bin/main_options.cc b/runtime/bin/main_options.cc index 25d484c60c0..99d92070dd1 100644 --- a/runtime/bin/main_options.cc +++ b/runtime/bin/main_options.cc @@ -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()); @@ -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); @@ -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) { diff --git a/runtime/bin/main_options.h b/runtime/bin/main_options.h index 8b5e6eb5a9f..4a44ce0929e 100644 --- a/runtime/bin/main_options.h +++ b/runtime/bin/main_options.h @@ -107,6 +107,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,