Bug 1371293 - Automatically clobber node_modules when upgrading from ESLint 3 to 4. r?ahal draft
authorMark Banner <standard8@mozilla.com>
Tue, 07 Nov 2017 14:30:56 +0000
changeset 697128 f07faa1ef4a51ac9130e0891d1cbe0745329a43d
parent 697127 02aabafaf3837ae12a730d830ae5ba3a360241b8
child 697129 28fc0a847d1bd13679c03b9412239f55e84b7b5f
push id88906
push userbmo:standard8@mozilla.com
push dateMon, 13 Nov 2017 12:27:39 +0000
reviewersahal
bugs1371293
milestone59.0a1
Bug 1371293 - Automatically clobber node_modules when upgrading from ESLint 3 to 4. r?ahal This is intended to help with ensuring that developer's profiles cleanup and upgrade correctly, as we've seen issues in the past. MozReview-Commit-ID: CqCRDN0y64I
tools/lint/eslint/__init__.py
tools/lint/eslint/hook_helper.py
tools/lint/eslint/setup_helper.py
--- a/tools/lint/eslint/__init__.py
+++ b/tools/lint/eslint/__init__.py
@@ -39,18 +39,17 @@ def lint(paths, config, binary=None, fix
     module_path = setup_helper.get_project_root()
 
     if not setup_helper.check_node_executables_valid():
         return 1
 
     if setup:
         return setup_helper.eslint_setup()
 
-    if setup_helper.eslint_module_needs_setup():
-        setup_helper.eslint_setup()
+    setup_helper.eslint_maybe_setup()
 
     # Valid binaries are:
     #  - Any provided by the binary argument.
     #  - Any pointed at by the ESLINT environmental variable.
     #  - Those provided by mach eslint --setup.
     #
     #  eslint --setup installs some mozilla specific plugins and installs
     #  all node modules locally. This is the preferred method of
--- a/tools/lint/eslint/hook_helper.py
+++ b/tools/lint/eslint/hook_helper.py
@@ -73,18 +73,17 @@ def runESLint(print_func, files):
         basepath = setup_helper.get_project_root()
 
         if not basepath:
             return False
 
         if not setup_helper.check_node_executables_valid():
             return False
 
-        if setup_helper.eslint_module_needs_setup():
-            setup_helper.eslint_setup()
+        setup_helper.eslint_maybe_setup()
 
         dir = os.path.join(basepath, "node_modules", ".bin")
 
         eslint_path = os.path.join(dir, "eslint")
         if os.path.exists(os.path.join(dir, "eslint.cmd")):
             eslint_path = os.path.join(dir, "eslint.cmd")
         output = check_output([eslint_path,
                                "--format", "json", "--plugin", "html"] + files,
--- a/tools/lint/eslint/setup_helper.py
+++ b/tools/lint/eslint/setup_helper.py
@@ -4,16 +4,17 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from filecmp import dircmp
 import json
 import os
 import platform
 import re
+import shutil
 import subprocess
 import sys
 from distutils.version import LooseVersion
 sys.path.append(os.path.join(
     os.path.dirname(__file__), "..", "..", "..", "third_party", "python", "which"))
 import which
 
 NODE_MIN_VERSION = "6.9.1"
@@ -48,29 +49,43 @@ Valid installation paths:
 
 
 VERSION_RE = re.compile(r"^\d+\.\d+\.\d+$")
 CARET_VERSION_RANGE_RE = re.compile(r"^\^((\d+)\.\d+\.\d+)$")
 
 project_root = None
 
 
-def eslint_setup():
+def eslint_maybe_setup():
+    """Setup ESLint only if it is needed."""
+    has_issues, needs_clobber = eslint_module_needs_setup()
+
+    if has_issues:
+        eslint_setup(needs_clobber)
+
+
+def eslint_setup(should_clobber=False):
     """Ensure eslint is optimally configured.
 
     This command will inspect your eslint configuration and
     guide you through an interactive wizard helping you configure
     eslint for optimal use on Mozilla projects.
     """
     orig_cwd = os.getcwd()
     sys.path.append(os.path.dirname(__file__))
 
     # npm sometimes fails to respect cwd when it is run using check_call so
     # we manually switch folders here instead.
-    os.chdir(get_project_root())
+    project_root = get_project_root()
+    os.chdir(project_root)
+
+    if should_clobber:
+        node_modules_path = os.path.join(project_root, "node_modules")
+        print("Clobbering node_modules...")
+        shutil.rmtree(node_modules_path)
 
     npm_path = get_node_or_npm_path("npm")
     if not npm_path:
         return 1
 
     extra_parameters = ["--loglevel=error"]
 
     # Install ESLint and external plugins
@@ -146,16 +161,17 @@ def check_eslint_files(node_modules_path
     dcmp = dircmp(os.path.join(node_modules_path, name),
                   os.path.join(get_eslint_module_path(), name))
 
     return check_file_diffs(dcmp)
 
 
 def eslint_module_needs_setup():
     has_issues = False
+    needs_clobber = False
     node_modules_path = os.path.join(get_project_root(), "node_modules")
 
     for name, expected_data in expected_eslint_modules().iteritems():
         # expected_eslint_modules returns a string for the version number of
         # dependencies for installation of eslint generally, and an object
         # for our in-tree plugins (which contains the entire module info).
         if "version" in expected_data:
             version_range = expected_data["version"]
@@ -171,22 +187,28 @@ def eslint_module_needs_setup():
 
         data = json.load(open(path))
 
         if version_range.startswith("file:"):
             # We don't need to check local file installations for versions, as
             # these are symlinked, so we'll always pick up the latest.
             continue
 
+        if name == "eslint" and LooseVersion("4.0.0") > LooseVersion(data["version"]):
+            print("ESLint is an old version, clobbering node_modules directory")
+            needs_clobber = True
+            has_issues = True
+            continue
+
         if not version_in_range(data["version"], version_range):
             print("%s v%s should be v%s." % (name, data["version"], version_range))
             has_issues = True
             continue
 
-    return has_issues
+    return has_issues, needs_clobber
 
 
 def version_in_range(version, version_range):
     """
     Check if a module version is inside a version range.  Only supports explicit versions and
     caret ranges for the moment, since that's all we've used so far.
     """
     if version == version_range: