Add --ignore-optional commandline flag for yarn install process. (pan…

…tsbuild#5209)

###  Problem
npm install currently ignores optional dependencies while yarn does not in Pants.  This can cause issues with webpack -> ... -> fsevents -> node-gyp dependency chain. node-gyp will require native compilation (and thus gcc/g++ support) and fsevents only works in Mac in any case. pantsbuild#5205 

### Solution
Add --ignore-optional command line flags to yarn install

### Result
Optional dependencies are no longer installed using yarn as package manager.

contrib/node/src/python/pants/contrib/node/subsystems/node_distribution.py

@@ -165,6 +165,7 @@ def run(self, **kwargs):
       :rtype: :class:`subprocess.Popen`
       """
       env, kwargs = self._prepare_env(kwargs)
+      logger.debug('Running command {}'.format(self.cmd))
       return subprocess.Popen(self.cmd, env=env, **kwargs)
 
     def check_output(self, **kwargs):

contrib/node/src/python/pants/contrib/node/subsystems/resolvers/npm_resolver.py

@@ -24,6 +24,9 @@ class NpmResolver(Subsystem, NodeResolverBase):
   @classmethod
   def register_options(cls, register):
     super(NpmResolver, cls).register_options(register)
+    register(
+      '--install-optional', type=bool, default=False, fingerprint=True,
+      help='If enabled, install optional dependencies.')
     NodeResolve.register_resolver_for_type(NodeModule, cls)
 
   def resolve_target(self, node_task, target, results_dir, node_paths):
@@ -33,6 +36,7 @@ def resolve_target(self, node_task, target, results_dir, node_paths):
         raise TaskError(
           'Cannot find package.json. Did you forget to put it in target sources?')
       package_manager = node_task.get_package_manager_for_target(target=target)
+      install_optional = self.get_options().install_optional
       if package_manager == node_task.node_distribution.PACKAGE_MANAGER_NPM:
         if os.path.exists('npm-shrinkwrap.json'):
           node_task.context.log.info('Found npm-shrinkwrap.json, will not inject package.json')
@@ -44,7 +48,10 @@ def resolve_target(self, node_task, target, results_dir, node_paths):
             'not fully supported.')
           self._emit_package_descriptor(node_task, target, results_dir, node_paths)
         # TODO: expose npm command options via node subsystems.
-        result, npm_install = node_task.execute_npm(['install', '--no-optional'],
+        args = ['install']
+        if not install_optional:
+          args.append('--no-optional')
+        result, npm_install = node_task.execute_npm(args,
                                                     workunit_name=target.address.reference(),
                                                     workunit_labels=[WorkUnitLabel.COMPILER])
         if result != 0:
@@ -54,8 +61,11 @@ def resolve_target(self, node_task, target, results_dir, node_paths):
         if not os.path.exists('yarn.lock'):
           raise TaskError(
             'Cannot find yarn.lock. Did you forget to put it in target sources?')
+        args = []
+        if not install_optional:
+          args.append('--ignore-optional')
         returncode, yarnpkg_command = node_task.execute_yarnpkg(
-          args=[],
+          args,
           workunit_name=target.address.reference(),
           workunit_labels=[WorkUnitLabel.COMPILER])
         if returncode != 0:

contrib/node/tests/python/pants_test/contrib/node/tasks/BUILD

@@ -98,6 +98,7 @@ python_tests(
   name='node_resolve',
   sources=['test_node_resolve.py'],
   dependencies=[
+    '3rdparty/python:mock',
     'contrib/node/src/python/pants/contrib/node/targets:node_module',
     'contrib/node/src/python/pants/contrib/node/targets:node_preinstalled_module',
     'contrib/node/src/python/pants/contrib/node/targets:node_remote_module',

contrib/node/tests/python/pants_test/contrib/node/tasks/test_node_resolve.py

@@ -9,6 +9,7 @@
 import os
 from textwrap import dedent
 
+import mock
 from pants.build_graph.target import Target
 from pants_test.tasks.task_test_base import TaskTestBase
 
@@ -85,7 +86,9 @@ def test_resolve_simple(self):
                               sources=['util.js', 'package.json'],
                               dependencies=[typ])
 
-    context = self.context(target_roots=[target])
+    context = self.context(target_roots=[target], options={
+      'npm-resolver': {'install_optional': False}
+    })
     task = self.create_task(context)
     task.execute()
 
@@ -140,7 +143,9 @@ def test_resolve_simple_graph(self):
                             target_type=NodeModule,
                             sources=['leaf.js', 'package.json'],
                             dependencies=[util, typ2])
-    context = self.context(target_roots=[leaf])
+    context = self.context(target_roots=[leaf], options={
+      'npm-resolver': {'install_optional': False}
+    })
     task = self.create_task(context)
     task.execute()
 
@@ -205,7 +210,9 @@ def test_resolve_preserves_package_json(self):
                                        target_type=NodeModule,
                                        sources=['package.json'],
                                        dependencies=[util])
-    context = self.context(target_roots=[scripts_project])
+    context = self.context(target_roots=[scripts_project], options={
+      'npm-resolver': {'install_optional': False}
+    })
     task = self.create_task(context)
     task.execute()
 
@@ -230,3 +237,64 @@ def test_resolve_preserves_package_json(self):
       self.assertNotIn('devDependencies', package)
       self.assertNotIn('peerDependencies', package)
       self.assertNotIn('optionalDependencies', package)
+
+  def _test_resolve_optional_install_helper(
+      self, install_optional, package_manager, expected_params):
+    self.create_file('src/node/util/package.json', contents=dedent("""
+      {
+        "name": "util",
+        "version": "0.0.1"
+      }
+    """))
+    self.create_file('src/node/util/util.js', contents=dedent("""
+      var typ = require('typ');
+      console.log("type of boolean is: " + typ.BOOLEAN);
+    """))
+    # yarn execution path requires yarn.lock
+    self.create_file('src/node/util/yarn.lock', contents='')
+    target = self.make_target(spec='src/node/util',
+                              target_type=NodeModule,
+                              sources=['util.js', 'package.json', 'yarn.lock'],
+                              dependencies=[],
+                              package_manager=package_manager)
+
+    context = self.context(target_roots=[target], options={
+      'npm-resolver': {'install_optional': install_optional}
+    })
+    task = self.create_task(context)
+
+    method_to_mock = {
+      'npm': 'execute_npm',
+      'yarn': 'execute_yarnpkg'
+    }[package_manager]
+    with mock.patch.object(task, method_to_mock) as exec_call:
+      exec_call.return_value = (0, None)
+      task.execute()
+      exec_call.assert_called_once_with(
+        expected_params,
+        workunit_labels=mock.ANY,
+        workunit_name=mock.ANY)
+
+  def test_resolve_default_no_optional_install_npm(self):
+    self._test_resolve_optional_install_helper(
+      install_optional=False,
+      package_manager='npm',
+      expected_params=['install', '--no-optional'])
+
+  def test_resolve_optional_install_npm(self):
+    self._test_resolve_optional_install_helper(
+      install_optional=True,
+      package_manager='npm',
+      expected_params=['install'])
+
+  def test_resolve_default_no_optional_install_yarn(self):
+    self._test_resolve_optional_install_helper(
+      install_optional=False,
+      package_manager='yarn',
+      expected_params=['--ignore-optional'])
+
+  def test_resolve_optional_install_yarn(self):
+    self._test_resolve_optional_install_helper(
+      install_optional=True,
+      package_manager='yarn',
+      expected_params=[])