Fix ordering of args from compiler_option_sets and add test for scala…

…c profiling (#7683)

### Problem

The options from `--compiler-option-sets-enabled-args` or `--compiler-option-sets-disabled-args` for tasks which mix in `CompilerOptionSetsMixin` don't keep their ordering, see: https://github.com/pantsbuild/pants/blob/5eed2e7be5aa4672485f06e043bcec24bb9c668d/src/python/pants/option/compiler_option_sets_mixin.py#L65

This breaks the ability to use options which require two successive arguments via compiler option sets. Scalac profiling is an example of this, requiring a successive output file argument.

### Solution

- Use a list to collect `compiler_option_sets` arguments.
- Add an integration test for scalac profiling.

### Result

Scalac profiling is now possible using a compiler option set!

src/python/pants/backend/jvm/tasks/jvm_compile/jvm_compile.py

@@ -69,6 +69,7 @@ def register_options(cls, register):
              help='When set, any invalid/incompatible analysis files will be deleted '
                   'automatically.  When unset, an error is raised instead.')
 
+    # TODO(#7682): convert these into option sets!
     register('--warnings', default=True, type=bool, fingerprint=True,
              help='Compile with all configured warnings enabled.')
 

src/python/pants/option/compiler_option_sets_mixin.py

@@ -62,16 +62,23 @@ def get_merged_args_for_compiler_option_sets(self, compiler_option_sets):
       compiler_option_sets = self.get_options().default_compiler_option_sets
       logger.debug('using default option sets: {}'.format(compiler_option_sets))
 
-    compiler_options = set()
+    compiler_options = []
+
+    # Set values for disabled options (they will come before the enabled options). This allows
+    # enabled option sets to override the disabled ones, if the underlying command has later
+    # options supersede earlier options.
+    compiler_options.extend(
+      disabled_arg
+      for option_set_key, disabled_args in self.get_options().compiler_option_sets_disabled_args.items()
+      if option_set_key not in compiler_option_sets
+      for disabled_arg in disabled_args
+    )
 
     # Set values for enabled options.
-    for option_set_key in compiler_option_sets:
-      val = self.get_options().compiler_option_sets_enabled_args.get(option_set_key, ())
-      compiler_options.update(val)
+    compiler_options.extend(
+      enabled_arg
+      for option_set_key in compiler_option_sets
+      for enabled_arg in self.get_options().compiler_option_sets_enabled_args.get(option_set_key, [])
+    )
 
-    # Set values for disabled options.
-    for option_set_key, disabled_args in self.get_options().compiler_option_sets_disabled_args.items():
-      if not option_set_key in compiler_option_sets:
-        compiler_options.update(disabled_args)
-
-    return list(compiler_options)
+    return compiler_options

tests/python/pants_test/backend/jvm/tasks/jvm_compile/zinc/zinc_compile_integration_base.py

@@ -38,6 +38,38 @@ def test_scala_compile_jar(self):
         self.assertTrue(jar.getinfo(SHAPELESS_CLSFILE),
                         'Expected a jar containing the expected class.')
 
+  # TODO: this could be converted into a unit test!
+  def test_consecutive_compiler_option_sets(self):
+    """Test that the ordering of args in compiler option sets are respected.
+
+    Generating a scalac profile requires two consecutive arguments, '-Yprofile-destination' and its
+    following argument, the file to write the CSV profile to. We want to be able to allow users to
+    successfully run scalac with profiling from pants, so we test this case in particular. See the
+    discussion from https://github.com/pantsbuild/pants/pull/7683.
+    """
+    with temporary_dir() as tmp_dir:
+      profile_destination = os.path.join(tmp_dir, 'scala_profile.csv')
+      self.do_command(
+        'compile',
+        SHAPELESS_TARGET,
+        # Flags to enable profiling and statistics on target
+        config={
+          'compile.zinc': {
+            'default_compiler_option_sets': ['profile'],
+            'compiler_option_sets_enabled_args': {
+              'profile': [
+                '-S-Ystatistics',
+                '-S-Yhot-statistics-enabled',
+                '-S-Yprofile-enabled',
+                '-S-Yprofile-destination',
+                '-S{}'.format(profile_destination),
+                '-S-Ycache-plugin-class-loader:last-modified',
+              ],
+            },
+          },
+        })
+      self.assertTrue(os.path.isfile(profile_destination))
+
   def test_scala_empty_compile(self):
     with self.do_test_compile('testprojects/src/scala/org/pantsbuild/testproject/emptyscala',
                               expected_files=[]):

tests/python/pants_test/pants_run_integration_test.py

@@ -616,10 +616,9 @@ def do_command(self, *args, **kwargs):
       success - indicate whether to expect pants run to succeed or fail.
     :return: a PantsResult object
     """
-    success = kwargs.get('success', True)
-    cmd = []
-    cmd.extend(list(args))
-    pants_run = self.run_pants(cmd)
+    success = kwargs.pop('success', True)
+    cmd = list(args)
+    pants_run = self.run_pants(cmd, **kwargs)
     if success:
       self.assert_success(pants_run)
     else: