Revert "[ CLI ] Improved consistency of -D and --define across tools and"

This reverts commit e83e78431f.

Reason for revert: Failing on SIMARM and AOT bots

Original change's description:
> [ CLI ] Improved consistency of -D and --define across tools and
> commands
>
> - Added support for --define to the VM and dart2js
> - Added support for -D and --define for `dart run` and `dart compile js`
>
> Remaining improvements:
> - Add support for providing multiple comma separated values for `dart
>   run`, `dart`, and `dart2js`
>
> Related issue: https://github.com/dart-lang/sdk/issues/44562
>
> TEST=Updated CLI tests and added new dart2js tests.
>
> Change-Id: I9379c7aee1eab377adb3438393d9ad79c4938cc4
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/178262
> Commit-Queue: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>

TBR=bkonyi@google.com,asiva@google.com,sigmund@google.com

Change-Id: I1c594ae7db551619cc3191ff7f832c4fc61a4171
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/179081
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/compiler/lib/src/dart2js.dart

@@ -234,22 +234,9 @@ Future<api.CompilationResult> compile(List<String> argv,
     passThrough(argument);
   }
 
-  void addInEnvironment(Iterator<String> arguments) {
-    final isDefine = arguments.current.startsWith('--define');
-    String argument;
-    if (arguments.current == '--define') {
-      arguments.moveNext();
-      argument = arguments.current;
-    } else {
-      argument = arguments.current.substring(isDefine ? '--define='.length : 2);
-    }
-    // Allow for ' ' or '=' after --define
+  void addInEnvironment(String argument) {
     int eqIndex = argument.indexOf('=');
-    if (eqIndex <= 0) {
-      helpAndFail('Invalid value for --define: $argument');
-      return;
-    }
-    String name = argument.substring(0, eqIndex);
+    String name = argument.substring(2, eqIndex);
     String value = argument.substring(eqIndex + 1);
     environment[name] = value;
   }
@@ -575,8 +562,7 @@ Future<api.CompilationResult> compile(List<String> argv,
     new OptionHandler('${Flags.mergeFragmentsThreshold}=.+', passThrough),
 
     // The following three options must come last.
-    new OptionHandler('-D.+=.*|--define=.+=.*|--define', addInEnvironment,
-        multipleArguments: true),
+    new OptionHandler('-D.+=.*', addInEnvironment),
     new OptionHandler('-.*', (String argument) {
       helpAndFail("Unknown option '$argument'.");
     }),
@@ -1042,7 +1028,7 @@ Supported options:
   -v, --verbose
     Display verbose information.
 
-  -D<name>=<value>, --define=<name>=<value>
+  -D<name>=<value>
     Define an environment declaration.
 
   --version

pkg/dartdev/lib/src/commands/compile.dart

@@ -59,11 +59,7 @@ class CompileJSCommand extends CompileSubcommandCommand {
         help: 'Generate minified output.',
         abbr: 'm',
         negatable: false,
-      )
-      ..addMultiOption('define', abbr: 'D', valueHelp: 'key=value', help: '''
-Define an environment declaration. To specify multiple declarations, use multiple options or use commas to separate key-value pairs.
-For example: dart compile $cmdName -Da=1,b=2 main.dart''');
-
+      );
     addExperimentalFlags(argParser, verbose);
   }
 
@@ -95,26 +91,15 @@ For example: dart compile $cmdName -Da=1,b=2 main.dart''');
     if (!checkFile(sourcePath)) {
       return 1;
     }
-    final args = <String>[
-      '--libraries-spec=$librariesPath',
-      if (argResults.enabledExperiments.isNotEmpty)
-        "--enable-experiment=${argResults.enabledExperiments.join(',')}",
-      if (argResults.wasParsed(commonOptions['outputFile'].flag))
-        "-o${argResults[commonOptions['outputFile'].flag]}",
-      if (argResults.wasParsed('minified')) '-m',
-    ];
 
-    if (argResults.wasParsed('define')) {
-      for (final define in argResults['define']) {
-        args.add('-D$define');
-      }
-    }
-
-    // Add any args that weren't parsed to the end. This will likely only ever
-    // be the script name.
-    args.addAll(argResults.rest);
-
-    VmInteropHandler.run(sdk.dart2jsSnapshot, args,
+    VmInteropHandler.run(
+        sdk.dart2jsSnapshot,
+        [
+          '--libraries-spec=$librariesPath',
+          if (argResults.enabledExperiments.isNotEmpty)
+            "--enable-experiment=${argResults.enabledExperiments.join(',')}",
+          ...argResults.arguments,
+        ],
         packageConfigOverride: null);
 
     return 0;

pkg/dartdev/lib/src/commands/run.dart

@@ -100,12 +100,8 @@ class RunCommand extends DartdevCommand {
         );
     }
     argParser
-      ..addMultiOption(
-        'define',
-        abbr: 'D',
-        valueHelp: 'key=value',
-        help: 'Define an environment declaration.',
-      )
+      ..addMultiOption('define',
+          abbr: 'D', help: 'Defines an environment variable', hide: true)
       ..addFlag(
         'disable-service-auth-codes',
         hide: !verbose,

pkg/dartdev/test/commands/compile_test.dart

@@ -183,13 +183,7 @@ void defineCompileTests() {
   });
 
   test('Compile JS', () {
-    final p = project(mainSrc: '''
-        void main() {
-          print('1: ' + const String.fromEnvironment('foo'));
-          print('2: ' + const String.fromEnvironment('baz'));
-          print('3: ' + const String.fromEnvironment('bar'));
-          print('4: ' + const String.fromEnvironment('fad'));
-        }''');
+    final p = project(mainSrc: "void main() { print('Hello from JS'); }");
     final inFile = path.canonicalize(path.join(p.dirPath, p.relativeFilePath));
     final outFile = path.canonicalize(path.join(p.dirPath, 'main.js'));
 
@@ -197,8 +191,6 @@ void defineCompileTests() {
       'compile',
       'js',
       '-m',
-      '-Dfoo=bar,baz=bad',
-      '--define=bar=foo,fad=lad',
       '-o',
       outFile,
       '-v',
@@ -206,14 +198,7 @@ void defineCompileTests() {
     ]);
     expect(result.stderr, isEmpty);
     expect(result.exitCode, 0);
-    final file = File(outFile);
-    expect(file.existsSync(), true, reason: 'File not found: $outFile');
-
-    // Ensure the -D and --define arguments were processed correctly.
-    final contents = file.readAsStringSync();
-    expect(contents.contains('1: bar'), true);
-    expect(contents.contains('2: bad'), true);
-    expect(contents.contains('3: foo'), true);
-    expect(contents.contains('4: lad'), true);
+    expect(File(outFile).existsSync(), true,
+        reason: 'File not found: $outFile');
   });
 }

pkg/dartdev/test/commands/run_test.dart

@@ -179,7 +179,6 @@ void main(List<String> args) => print("$b $args");
       '--no-pause-isolates-on-exit',
       '--no-pause-isolates-on-unhandled-exceptions',
       '-Dfoo=bar',
-      '--define=bar=foo',
       p.relativeFilePath,
     ]);
     expect(
@@ -201,7 +200,6 @@ void main(List<String> args) => print("$b $args");
       '--no-pause-isolates-on-unhandled-exceptions',
       '--disable-service-auth-codes',
       '-Dfoo=bar',
-      '--define=bar=foo',
       p.relativeFilePath,
     ]);
 

pkg/dev_compiler/lib/src/kernel/command.dart

@@ -730,19 +730,8 @@ Map<String, String> parseAndRemoveDeclaredVariables(List<String> args) {
   var declaredVariables = <String, String>{};
   for (var i = 0; i < args.length;) {
     var arg = args[i];
-    String rest;
-    const defineFlag = '--define';
     if (arg.startsWith('-D') && arg.length > 2) {
-      rest = arg.substring(2);
-    } else if (arg.startsWith('$defineFlag=') &&
-        arg.length > defineFlag.length + 1) {
-      rest = arg.substring(defineFlag.length + 1);
-    } else if (arg == defineFlag) {
-      i++;
-      rest = args[i];
-    }
-
-    if (rest != null) {
+      var rest = arg.substring(2);
       var eq = rest.indexOf('=');
       if (eq <= 0) {
         var kind = eq == 0 ? 'name' : 'value';

pkg/front_end/tool/_fasta/command_line.dart

@@ -199,7 +199,6 @@ const Map<String, ValueSpecification> optionSpecification =
   Flags.linkDependencies: const UriListValue(),
   Flags.noDeps: const BoolValue(false),
   "-D": const DefineValue(),
-  "--define": const AliasValue("-D"),
   "-h": const AliasValue(Flags.help),
   "--out": const AliasValue(Flags.output),
   "-o": const AliasValue(Flags.output),

runtime/bin/main_options.cc

@@ -141,9 +141,6 @@ void Options::PrintUsage() {
 "  all VM options).\n"
 "--packages=<path>\n"
 "  Where to find a package spec file.\n"
-"--define=<key>=<value> or -D<key>=<value>\n"
-"  Define an environment declaration. To specify multiple declarations,\n"
-"  use multiple instances of this option.\n"
 "--observe[=<port>[/<bind-address>]]\n"
 "  The observe flag is a convenience flag used to run a program with a\n"
 "  set of options which are often useful for debugging under Observatory.\n"
@@ -177,9 +174,6 @@ void Options::PrintUsage() {
 "  all VM options).\n"
 "--packages=<path>\n"
 "  Where to find a package spec file.\n"
-"--define=<key>=<value> or -D<key>=<value>\n"
-"  Define an environment declaration. To specify multiple declarations,\n"
-"  use multiple instances of this option.\n"
 "--observe[=<port>[/<bind-address>]]\n"
 "  The observe flag is a convenience flag used to run a program with a\n"
 "  set of options which are often useful for debugging under Observatory.\n"
@@ -546,8 +540,7 @@ bool Options::ParseArguments(int argc,
     while (tmp_i < argc) {
       // Check if this flag is a potentially valid VM flag. If not, we've likely
       // hit a script name and are done parsing VM flags.
-      if (!OptionProcessor::IsValidFlag(argv[tmp_i]) &&
-          !OptionProcessor::IsValidShortFlag(argv[tmp_i])) {
+      if (!OptionProcessor::IsValidFlag(argv[tmp_i])) {
         break;
       }
       OptionProcessor::TryProcess(argv[tmp_i], vm_options);

runtime/bin/options.cc

@@ -41,35 +41,22 @@ bool OptionProcessor::TryProcess(const char* option,
   return false;
 }
 
-static bool IsPrefix(const char* prefix, size_t prefix_len, const char* str) {
-  ASSERT(prefix != nullptr);
-  ASSERT(str != nullptr);
-  const size_t str_len = strlen(str);
-  if (str_len < prefix_len) {
-    return false;
-  }
-  return strncmp(prefix, str, prefix_len) == 0;
-}
-
 bool OptionProcessor::ProcessEnvironmentOption(
     const char* arg,
     CommandLineOptions* vm_options,
     dart::SimpleHashMap** environment) {
   ASSERT(arg != NULL);
   ASSERT(environment != NULL);
-  const char* kShortPrefix = "-D";
-  const char* kLongPrefix = "--define=";
-  const int kShortPrefixLen = strlen(kShortPrefix);
-  const int kLongPrefixLen = strlen(kLongPrefix);
-  const bool is_short_form = IsPrefix(kShortPrefix, kShortPrefixLen, arg);
-  const bool is_long_form = IsPrefix(kLongPrefix, kLongPrefixLen, arg);
-  if (is_short_form) {
-    arg = arg + kShortPrefixLen;
-  } else if (is_long_form) {
-    arg = arg + kLongPrefixLen;
-  } else {
+  if (*arg == '\0') {
     return false;
   }
+  if (*arg != '-') {
+    return false;
+  }
+  if (*(arg + 1) != 'D') {
+    return false;
+  }
+  arg = arg + 2;
   if (*arg == '\0') {
     return true;
   }
@@ -82,20 +69,12 @@ bool OptionProcessor::ProcessEnvironmentOption(
   const char* equals_pos = strchr(arg, '=');
   if (equals_pos == NULL) {
     // No equal sign (name without value) currently not supported.
-    if (is_short_form) {
-      Syslog::PrintErr("No value given to -D option\n");
-    } else {
-      Syslog::PrintErr("No value given to --define option\n");
-    }
+    Syslog::PrintErr("No value given to -D option\n");
     return true;
   }
   int name_len = equals_pos - arg;
   if (name_len == 0) {
-    if (is_short_form) {
-      Syslog::PrintErr("No name given to -D option\n");
-    } else {
-      Syslog::PrintErr("No name given to --define option\n");
-    }
+    Syslog::PrintErr("No name given to -D option\n");
     return true;
   }
   // Split name=value into name and value.

tests/corelib/string_from_environment_test.dart

@@ -1,7 +1,7 @@
 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
-// SharedOptions=-Da=a -Db=bb -Dc=ccc -Dd= --define=e=eeee
+// SharedOptions=-Da=a -Db=bb -Dc=ccc -Dd=
 
 import "package:expect/expect.dart";
 
@@ -10,5 +10,4 @@ main() {
   Expect.equals('bb', const String.fromEnvironment('b'));
   Expect.equals('ccc', const String.fromEnvironment('c'));
   Expect.equals('', const String.fromEnvironment('d'));
-  Expect.equals('eeee', const String.fromEnvironment('e'));
 }

tests/corelib_2/string_from_environment_test.dart

@@ -1,7 +1,7 @@
 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
-// SharedOptions=-Da=a -Db=bb -Dc=ccc -Dd= --define=e=eeee
+// SharedOptions=-Da=a -Db=bb -Dc=ccc -Dd=
 
 import "package:expect/expect.dart";
 
@@ -10,5 +10,4 @@ main() {
   Expect.equals('bb', const String.fromEnvironment('b'));
   Expect.equals('ccc', const String.fromEnvironment('c'));
   Expect.equals('', const String.fromEnvironment('d'));
-  Expect.equals('eeee', const String.fromEnvironment('e'));
 }