Make sure eslint yarn installs don't run in parallel

Summary:
Previously, we were running the `eslint_ui.sh` helper script as part of the version check even if we didn't have any files to lint with `eslint`.
To speed it up, I removed the version check.

However arcanist runs linters in parallel, which now meant that we had multiple `yarn install`s running concurrently.
These would stomp on the contents of `node_modules` and the `eslint` binary causing some lint runs to emit no output which in turn cause `arc` to fatal.

Instead we hook into `willLintPaths` and run a setup script once if there are any paths before calling the parent class's `willLintPaths` which will actually create the futures to run the linter.

Test Plan:
Before this change
- edit multiple `ts` files
- `rm -rf src/ui/node_modules; arc lint --trace`
- notice failure

After this change
- edit multiple `ts` files
- `rm -rf src/ui/node_modules; arc lint --trace`
- notice `eslint_setup.sh` being run first
- notice `eslint_ui.sh` being run in parallel after `eslint_setup.sh` exits
- no failures

Reviewers: nlanam, #engineering

Reviewed By: nlanam, #engineering

Differential Revision: https://phab.corp.pixielabs.ai/D7541

GitOrigin-RevId: d92ba7a602974dbf8736df1c2191984b0fb41804

.arclint

@@ -153,6 +153,7 @@
       "type": "eslint",
       "bin": "./linters/eslint_ui.sh",
       "eslint.config": ".eslintrc.json",
+      "eslint.setup": "./linters/eslint_setup.sh",
       "include": [
         "(^src/ui/.*\\.(tsx|ts|js)$)"
       ]

linters/eslint_setup.sh

+#!/bin/bash
+set -e
+
+SCRIPTPATH="$( cd "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )"
+uipath="${SCRIPTPATH}/../src/ui"
+
+pushd "${uipath}"
+yarn install
+popd
\ No newline at end of file

linters/eslint_ui.sh

@@ -3,11 +3,7 @@ set -e
 
 SCRIPTPATH="$( cd "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )"
 uipath="${SCRIPTPATH}/../src/ui"
-eslint=./node_modules/.bin/eslint
 
-cd "${uipath}"
-if [[ ! -x "${eslint}" ]]; then
-    yarn install 1>/dev/null 2>/dev/null
-fi
-
-"${eslint}" "$@"
+pushd "${uipath}" > /dev/null
+./node_modules/.bin/eslint "$@"
+popd > /dev/null

linters/pinterest/src/ESLintLinter.php

@@ -22,8 +22,8 @@ final class ESLintLinter extends ArcanistExternalLinter {
   const ESLINT_WARNING = '1';
   const ESLINT_ERROR = '2';
 
-  private $cwd = '';
   private $flags = array();
+  private $setup_script = '';
 
   public function getInfoName() {
     return 'ESLint';
@@ -46,21 +46,6 @@ final class ESLintLinter extends ArcanistExternalLinter {
   }
 
   public function getDefaultBinary() {
-    if ($this->cwd) {
-      $realCWD = Filesystem::resolvePath($this->cwd, $this->getProjectRoot());
-      list($err, $stdout, $stderr) = exec_manual('yarn -s --cwd %s bin eslint', $realCWD);
-      if ($stdout) {
-        return strtok($stdout, "\n");
-      }
-    } else {
-      $localBinaryPath = Filesystem::resolvePath('./node_modules/.bin/eslint');
-
-      if (Filesystem::binaryExists($localBinaryPath)) {
-        return $localBinaryPath;
-      }
-    }
-
-    // Fallback on global install & fallthrough to internal existence checks
     return 'eslint';
   }
 
@@ -89,17 +74,9 @@ final class ESLintLinter extends ArcanistExternalLinter {
         'type' => 'optional string',
         'help' => pht('Use configuration from this file or shareable config. (https://eslint.org/docs/user-guide/command-line-interface#-c---config)'),
       ),
-      'eslint.cwd' => array(
+      'eslint.setup' => array(
         'type' => 'optional string',
-        'help' => pht('Specify a project sub-directory for both the local eslint-cli install and the sub-directory to lint within.'),
-      ),
-      'eslint.env' => array(
-        'type' => 'optional string',
-        'help' => pht('Specify environments. To specify multiple environments, separate them using commas. (https://eslint.org/docs/user-guide/command-line-interface#--env)'),
-      ),
-      'eslint.fix' => array(
-        'type' => 'optional bool',
-        'help' => pht('Specify whether eslint should autofix issues. (https://eslint.org/docs/user-guide/command-line-interface#fixing-problems)'),
+        'help' => pht('An optional setup script to run before invoking the linter.'),
       ),
     );
     return $options + parent::getLinterConfigurationOptions();
@@ -111,41 +88,30 @@ final class ESLintLinter extends ArcanistExternalLinter {
         $this->flags[] = '--config';
         $this->flags[] = $value;
         return;
-      case 'eslint.cwd':
-        $this->cwd = $value;
-        return;
-      case 'eslint.env':
-        $this->flags[] = '--env ';
-        $this->flags[] = $value;
-        return;
-      case 'eslint.fix':
-        if ($value) {
-          $this->flags[] = '--fix ';
-        }
+      case 'eslint.setup':
+        $this->setup_script = $value;
         return;
     }
     return parent::setLinterConfigurationValue($key, $value);
   }
 
   public function getInstallInstructions() {
-    return pht(
-      "\n\t%s[%s globally] run: `%s`\n\t[%s locally] run either: `%s` OR `%s`",
-      $this->cwd ? pht("[%s globally] (required for %s) run: `%s`\n\t",
-        'yarn',
-        '--cwd',
-        'npm install --global yarn@1') : '',
-      'eslint',
-      'npm install --global eslint',
-      'eslint',
-      'npm install --save-dev eslint',
-      'yarn add --dev eslint'
-    );
+    return pht($this->setup_script);
   }
 
   protected function canCustomizeLintSeverities() {
     return false;
   }
 
+  public function willLintPaths(array $paths) {
+    if (!empty($paths) && !empty($this->setup_script)) {
+      // Call the setup script to yarn install!
+      execx($this->setup_script);
+    }
+
+    return parent::willLintPaths($paths);
+  }
+
   protected function parseLinterOutput($path, $err, $stdout, $stderr) {
     // Gate on $stderr b/c $err (exit code) is expected.
     if ($stderr) {