Bug 1482435 - Separate out nodejs finding logic from configure and use it for ESLint. r=firefox-build-system-reviewers,gps
authorMark Banner <standard8@mozilla.com>
Tue, 25 Sep 2018 18:15:51 +0000
changeset 438163 951f04d1bb51
parent 438162 bc99ca10fe66
child 438164 d4c858c70bc9
push id69918
push usermbanner@mozilla.com
push dateTue, 25 Sep 2018 18:16:51 +0000
treeherderautoland@951f04d1bb51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfirefox-build-system-reviewers, gps
bugs1482435
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1482435 - Separate out nodejs finding logic from configure and use it for ESLint. r=firefox-build-system-reviewers,gps This extracts the current logic for finding nodejs into its own module in mozbuild. Configure and ESLint then use it. For ESLint, this will change the first location it looks for nodejs to be the .mozbuild directory. Differential Revision: https://phabricator.services.mozilla.com/D6430
build/moz.configure/node.configure
python/mozbuild/mozbuild/nodeutil.py
taskcluster/docker/recipes/install-node.sh
tools/lint/eslint/__init__.py
tools/lint/eslint/setup_helper.py
--- a/build/moz.configure/node.configure
+++ b/build/moz.configure/node.configure
@@ -1,102 +1,60 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # 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/.
 
 option('--disable-nodejs',
        help='Require Node.js to build')
+option(env='NODEJS', nargs=1, help='Path to nodejs')
 
 
-@depends(host)
-@imports('os')
-@imports(_from='os', _import='environ')
-def node_toolchain_search_path(host):
-    # XXX partly copied from tooltool.py; should be hoisted somewhere central
-
-    # Also add in the location to which `mach bootstrap` or
-    # `mach artifact toolchain` installs clang.
-    mozbuild_state_dir = environ.get('MOZBUILD_STATE_PATH', os.path.expanduser(
-                                     os.path.join('~', '.mozbuild')))
-
-    if host.kernel == "WINNT":
-        mozbuild_node_path = os.path.join(mozbuild_state_dir, 'node')
-    else:
-        mozbuild_node_path = os.path.join(mozbuild_state_dir, 'node', 'bin')
-
-    # We still fallback to the PATH, since on OSes that don't have toolchain
-    # artifacts available to download, Node may be coming from $PATH.
-    path = [environ.get('PATH')]
-    updated_path = [mozbuild_node_path] + path
-
-    return updated_path
+@depends('--enable-nodejs', 'NODEJS')
+@checking('for nodejs',
+          callback=lambda x: '%s (%s)' % (x.path, x.str_version) if x else 'no')
+@imports(_from='mozbuild.nodeutil', _import='find_node_executable')
+@imports(_from='mozbuild.nodeutil', _import='NODE_MIN_VERSION')
+def nodejs(require, env_node):
+    node_exe = env_node[0] if env_node else None
 
-# "nodejs" is first in the tuple on the assumption that it's only likely to
-# exist on systems (probably linux distros) where there is a program in the path
-# called "node" that does something else.
-nodejs = check_prog('NODEJS', ('nodejs', 'node',),
-                    allow_missing=True, paths=node_toolchain_search_path,
-                    paths_have_priority=True)
-
-
-@depends_if(nodejs)
-@checking('for node.js version')
-@imports('os')
-@imports('re')
-@imports('subprocess')
-def nodejs_version(node):
-    env = dict(os.environ)
-    env[b'NODE_DISABLE_COLORS'] = b'1'
-
-    out = check_cmd_output(node, '--version', env=env)
-
-    match = re.search(r'v(.+)$', out)
-
-    if not match:
-        return None
-
-    return Version(match.group(1))
-
-
-@depends('--enable-nodejs', nodejs, nodejs_version)
-def nodejs_suitability(require, node, version):
-    MIN_NODE_VERSION = Version('8.11')
+    nodejs, version = find_node_executable(node_exe)
 
     MAYBE_FILE_A_BUG = '''
 
-If you believe this is a bug, <https://mzl.la/2vLbXAv> is a good way
-to file.  Executing `mach bootstrap --no-system-changes` should
-install a compatible version in ~/.mozbuild on most platforms.
-More details: <https://bit.ly/2BbyD1E>
-'''
+    Executing `mach bootstrap --no-system-changes` should
+    install a compatible version in ~/.mozbuild on most platforms.
+    If you believe this is a bug, <https://mzl.la/2vLbXAv> is a good way
+    to file.  More details: <https://bit.ly/2BbyD1E>
+    '''
 
-    if not node:
-        msg = ('could not find Node.js executable; ensure `node` or `nodejs` '
-               'is in PATH or set NODEJS in environment to point to an '
-               'executable.%s' % (MAYBE_FILE_A_BUG)
+    if not nodejs:
+        msg = ('could not find Node.js executable later than %s; ensure '
+               '`node` or `nodejs` is in PATH or set NODEJS in environment '
+               'to point to an executable.%s' % (NODE_MIN_VERSION, MAYBE_FILE_A_BUG)
                )
 
         if require:
             raise FatalCheckError(msg)
         else:
             log.warning(msg)
             log.warning('(This will become an error in the near future.)')
             return
 
     if not version:
-        msg = 'could not resolve Node.js version.%s' % (MAYBE_FILE_A_BUG)
-        if require:
-            raise FatalCheckError(msg)
-        else:
-            log.warning(msg)
-            log.warning('(This will become an error in the near future.)')
-            return
-
-    if version < MIN_NODE_VERSION:
-        msg = 'NODEJS must point to node %s or newer; %s found. %s' % (
-            MIN_NODE_VERSION, version, MAYBE_FILE_A_BUG)
+        msg = 'NODEJS must point to node %s or newer; found node location: %s. %s' % (
+            NODE_MIN_VERSION, nodejs, MAYBE_FILE_A_BUG)
 
         if require:
             raise FatalCheckError(msg)
         else:
             log.warning(msg)
+            return
+
+    return namespace(
+        path=nodejs,
+        version=version,
+        str_version='.'.join(str(v) for v in version),
+    )
+
+
+set_config('NODEJS', depends_if(nodejs)(lambda p: p.path))
new file mode 100644
--- /dev/null
+++ b/python/mozbuild/mozbuild/nodeutil.py
@@ -0,0 +1,134 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# 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 __future__ import absolute_import
+
+import os
+import subprocess
+import platform
+from mozboot.util import get_state_dir
+import which
+
+from distutils.version import (
+    StrictVersion,
+)
+
+NODE_MIN_VERSION = StrictVersion("8.11.0")
+NPM_MIN_VERSION = StrictVersion("5.6.0")
+
+
+def find_node_paths():
+    """ Determines the possible paths for node executables.
+
+    Returns a list of paths, which includes the build state directory.
+    """
+    # Also add in the location to which `mach bootstrap` or
+    # `mach artifact toolchain` installs clang.
+    mozbuild_state_dir, _ = get_state_dir()
+
+    if platform.system() == "Windows":
+        mozbuild_node_path = os.path.join(mozbuild_state_dir, 'node')
+    else:
+        mozbuild_node_path = os.path.join(mozbuild_state_dir, 'node', 'bin')
+
+    # We still fallback to the PATH, since on OSes that don't have toolchain
+    # artifacts available to download, Node may be coming from $PATH.
+    paths = [mozbuild_node_path] + os.environ.get('PATH').split(os.pathsep)
+
+    if platform.system() == "Windows":
+        paths += [
+            "%s\\nodejs" % os.environ.get("SystemDrive"),
+            os.path.join(os.environ.get("ProgramFiles"), "nodejs"),
+            os.path.join(os.environ.get("PROGRAMW6432"), "nodejs"),
+            os.path.join(os.environ.get("PROGRAMFILES"), "nodejs")
+        ]
+
+    return paths
+
+
+def check_executable_version(exe):
+    """Determine the version of a Node executable by invoking it.
+
+    May raise ``subprocess.CalledProcessError`` or ``ValueError`` on failure.
+    """
+    out = subprocess.check_output([exe, "--version"]).lstrip('v').rstrip()
+    return StrictVersion(out)
+
+
+def simple_which(filename, path=None):
+    # Note: On windows, npm uses ".cmd"
+    exts = [".cmd", ".exe", ""] if platform.system() == "Windows" else [""]
+
+    for ext in exts:
+        try:
+            return which.which(filename + ext, path)
+        except which.WhichError:
+            pass
+
+    # If we got this far, we didn't find it with any of the extensions, so
+    # just return.
+    return None
+
+
+def find_node_executable(nodejs_exe=os.environ.get('NODEJS'), min_version=NODE_MIN_VERSION):
+    """Find a Node executable from the mozbuild directory.
+
+    Returns a tuple containing the the path to an executable binary and a
+    version tuple. Both tuple entries will be None if a Node executable
+    could not be resolved.
+    """
+    if nodejs_exe:
+        try:
+            version = check_executable_version(nodejs_exe)
+        except (subprocess.CalledProcessError, ValueError):
+            return None, None
+
+        if version >= min_version:
+            return nodejs_exe, version.version
+
+        return None, None
+
+    # "nodejs" is first in the tuple on the assumption that it's only likely to
+    # exist on systems (probably linux distros) where there is a program in the path
+    # called "node" that does something else.
+    return find_executable(['nodejs', 'node'], min_version)
+
+
+def find_npm_executable(min_version=NPM_MIN_VERSION):
+    """Find a Node executable from the mozbuild directory.
+
+    Returns a tuple containing the the path to an executable binary and a
+    version tuple. Both tuple entries will be None if a Node executable
+    could not be resolved.
+    """
+    return find_executable(["npm"], min_version)
+
+
+def find_executable(names, min_version):
+    paths = find_node_paths()
+
+    found_exe = None
+    for name in names:
+        try:
+            exe = simple_which(name, paths)
+        except which.WhichError:
+            continue
+
+        if not exe:
+            continue
+
+        if not found_exe:
+            found_exe = exe
+
+        # We always verify we can invoke the executable and its version is
+        # sane.
+        try:
+            version = check_executable_version(exe)
+        except (subprocess.CalledProcessError, ValueError):
+            continue
+
+        if version >= min_version:
+            return exe, version.version
+
+    return found_exe, None
--- a/taskcluster/docker/recipes/install-node.sh
+++ b/taskcluster/docker/recipes/install-node.sh
@@ -1,12 +1,15 @@
 #!/bin/bash
 # This Source Code Form is subject to the terms of the Mozilla Public
 # 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/.
 
 # This script installs Node v8.
+# XXX For now, this should match the version installed in
+# taskcluster/scripts/misc/repack-node.sh. Later we'll get the ESLint builder
+# to use the linux64-node toolchain directly.
 
-wget --progress=dot:mega https://nodejs.org/dist/v8.9.4/node-v8.9.4-linux-x64.tar.gz
-echo '21fb4690e349f82d708ae766def01d7fec1b085ce1f5ab30d9bda8ee126ca8fc  node-v8.9.4-linux-x64.tar.gz' | sha256sum -c
-tar -C /usr/local -xz --strip-components 1 < node-v8.9.4-linux-x64.tar.gz
+wget --progress=dot:mega https://nodejs.org/dist/v8.11.3/node-v8.11.3-linux-x64.tar.xz
+echo '08e2fcfea66746bd966ea3a89f26851f1238d96f86c33eaf6274f67fce58421a  node-v8.11.3-linux-x64.tar.xz' | sha256sum -c
+tar -C /usr/local -xJ --strip-components 1 < node-v8.11.3-linux-x64.tar.xz
 node -v  # verify
 npm -v
--- a/tools/lint/eslint/__init__.py
+++ b/tools/lint/eslint/__init__.py
@@ -5,16 +5,17 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import json
 import os
 import signal
 import sys
 sys.path.append(os.path.join(os.path.dirname(__file__), "eslint"))
 import setup_helper
+from mozbuild.nodeutil import find_node_executable
 
 from mozprocess import ProcessHandler
 
 from mozlint import result
 
 ESLINT_ERROR_MESSAGE = """
 An error occurred running eslint. Please check the following error messages:
 
@@ -47,29 +48,25 @@ def lint(paths, config, binary=None, fix
     module_path = setup_helper.get_project_root()
 
     # Valid binaries are:
     #  - Any provided by the binary argument.
     #  - Any pointed at by the ESLINT environmental variable.
     #  - Those provided by |mach lint --setup|.
 
     if not binary:
-        binary = os.environ.get('ESLINT', None)
-
-        if not binary:
-            binary = os.path.join(module_path, "node_modules", ".bin", "eslint")
-            if not os.path.isfile(binary):
-                binary = None
+        binary, _ = find_node_executable()
 
     if not binary:
         print(ESLINT_NOT_FOUND_MESSAGE)
         return 1
 
     extra_args = lintargs.get('extra_args') or []
     cmd_args = [binary,
+                os.path.join(module_path, "node_modules", "eslint", "bin", "eslint.js"),
                 # Enable the HTML plugin.
                 # We can't currently enable this in the global config file
                 # because it has bad interactions with the SublimeText
                 # ESLint plugin (bug 1229874).
                 '--plugin', 'html',
                 # This keeps ext as a single argument.
                 '--ext', '[{}]'.format(','.join(config['extensions'])),
                 '--format', 'json',
--- a/tools/lint/eslint/setup_helper.py
+++ b/tools/lint/eslint/setup_helper.py
@@ -9,49 +9,49 @@ import json
 import os
 import platform
 import re
 from mozfile.mozfile import remove as mozfileremove
 import subprocess
 import sys
 import shutil
 import tempfile
-from distutils.version import LooseVersion
+from distutils.version import LooseVersion, StrictVersion
+from mozbuild.nodeutil import (find_node_executable, find_npm_executable,
+                               NPM_MIN_VERSION, NODE_MIN_VERSION)
 sys.path.append(os.path.join(
     os.path.dirname(__file__), "..", "..", "..", "third_party", "python", "which"))
-import which
-
-NODE_MIN_VERSION = "8.9.1"
-NPM_MIN_VERSION = "5.5.1"
 
 NODE_MACHING_VERSION_NOT_FOUND_MESSAGE = """
-nodejs is out of date. You currently have node v%s but v%s is required.
-Please update nodejs from https://nodejs.org and try again.
+Could not find Node.js executable later than %s.
+
+Executing `mach bootstrap --no-system-changes` should
+install a compatible version in ~/.mozbuild on most platforms.
 """.strip()
 
 NPM_MACHING_VERSION_NOT_FOUND_MESSAGE = """
-npm is out of date. You currently have npm v%s but v%s is required.
-You can usually update npm with:
+Could not find npm executable later than %s.
 
-npm i -g npm
+Executing `mach bootstrap --no-system-changes` should
+install a compatible version in ~/.mozbuild on most platforms.
 """.strip()
 
 NODE_NOT_FOUND_MESSAGE = """
 nodejs is either not installed or is installed to a non-standard path.
-Please install nodejs from https://nodejs.org and try again.
 
-Valid installation paths:
+Executing `mach bootstrap --no-system-changes` should
+install a compatible version in ~/.mozbuild on most platforms.
 """.strip()
 
 NPM_NOT_FOUND_MESSAGE = """
 Node Package Manager (npm) is either not installed or installed to a
-non-standard path. Please install npm from https://nodejs.org (it comes as an
-option in the node installation) and try again.
+non-standard path.
 
-Valid installation paths:
+Executing `mach bootstrap --no-system-changes` should
+install a compatible version in ~/.mozbuild on most platforms.
 """.strip()
 
 
 VERSION_RE = re.compile(r"^\d+\.\d+\.\d+$")
 CARET_VERSION_RANGE_RE = re.compile(r"^\^((\d+)\.\d+\.\d+)$")
 
 project_root = None
 
@@ -83,28 +83,28 @@ def eslint_setup(should_clobber=False):
         node_modules_path = os.path.join(project_root, "node_modules")
         print("Clobbering node_modules...")
         if sys.platform.startswith('win') and have_winrm():
             process = subprocess.Popen(['winrm', '-rf', node_modules_path])
             process.wait()
         else:
             mozfileremove(node_modules_path)
 
-    npm_path, version = get_node_or_npm_path("npm")
+    npm_path, version = find_npm_executable()
     if not npm_path:
         return 1
 
     extra_parameters = ["--loglevel=error"]
 
     package_lock_json_path = os.path.join(get_project_root(), "package-lock.json")
     package_lock_json_tmp_path = os.path.join(tempfile.gettempdir(), "package-lock.json.tmp")
 
     # If we have an npm version newer than 5.8.0, just use 'ci', as that's much
     # simpler and does exactly what we want.
-    npm_is_older_version = version < LooseVersion("5.8.0")
+    npm_is_older_version = version < StrictVersion("5.8.0").version
 
     if npm_is_older_version:
         cmd = [npm_path, "install"]
         shutil.copy2(package_lock_json_path, package_lock_json_tmp_path)
     else:
         cmd = [npm_path, "ci"]
 
     cmd.extend(extra_parameters)
@@ -267,84 +267,16 @@ def get_possible_node_paths_win():
     return list({
         "%s\\nodejs" % os.environ.get("SystemDrive"),
         os.path.join(os.environ.get("ProgramFiles"), "nodejs"),
         os.path.join(os.environ.get("PROGRAMW6432"), "nodejs"),
         os.path.join(os.environ.get("PROGRAMFILES"), "nodejs")
     })
 
 
-def simple_which(filename, path=None):
-    exts = [".cmd", ".exe", ""] if platform.system() == "Windows" else [""]
-
-    for ext in exts:
-        try:
-            return which.which(filename + ext, path)
-        except which.WhichError:
-            pass
-
-    # If we got this far, we didn't find it with any of the extensions, so
-    # just return.
-    return None
-
-
-def which_path(filename):
-    """
-    Return the nodejs or npm path.
-    """
-    # Look in the system path first.
-    path = simple_which(filename)
-    if path is not None:
-        return path
-
-    if platform.system() == "Windows":
-        # If we didn't find it fallback to the non-system paths.
-        path = simple_which(filename, get_possible_node_paths_win())
-    elif filename == "node":
-        path = simple_which("nodejs")
-
-    return path
-
-
-def get_node_or_npm_path(filename, minversion=None):
-    node_or_npm_path = which_path(filename)
-
-    if not node_or_npm_path:
-        if filename in ('node', 'nodejs'):
-            print(NODE_NOT_FOUND_MESSAGE)
-        elif filename == "npm":
-            print(NPM_NOT_FOUND_MESSAGE)
-
-        if platform.system() == "Windows":
-            app_paths = get_possible_node_paths_win()
-
-            for p in app_paths:
-                print("  - %s" % p)
-        elif platform.system() == "Darwin":
-            print("  - /usr/local/bin/{}".format(filename))
-        elif platform.system() == "Linux":
-            print("  - /usr/bin/{}".format(filename))
-
-        return None, None
-
-    version_str = get_version(node_or_npm_path).lstrip('v')
-
-    version = LooseVersion(version_str)
-
-    if not minversion or version > minversion:
-        return node_or_npm_path, version
-
-    if filename == "npm":
-        print(NPM_MACHING_VERSION_NOT_FOUND_MESSAGE % (version_str.strip(), minversion))
-    else:
-        print(NODE_MACHING_VERSION_NOT_FOUND_MESSAGE % (version_str.strip(), minversion))
-
-    return None, None
-
-
 def get_version(path):
     try:
         version_str = subprocess.check_output([path, "--version"],
                                               stderr=subprocess.STDOUT)
         return version_str
     except (subprocess.CalledProcessError, OSError):
         return None
 
@@ -388,23 +320,30 @@ def get_project_root():
     return project_root
 
 
 def get_eslint_module_path():
     return os.path.join(get_project_root(), "tools", "lint", "eslint")
 
 
 def check_node_executables_valid():
-    # eslint requires at least node 6.9.1
-    node_path = get_node_or_npm_path("node", LooseVersion(NODE_MIN_VERSION))
+    node_path, version = find_node_executable()
     if not node_path:
+        print(NODE_NOT_FOUND_MESSAGE)
+        return False
+    if not version:
+        print(NODE_MACHING_VERSION_NOT_FOUND_MESSAGE % NODE_MIN_VERSION)
         return False
 
-    npm_path = get_node_or_npm_path("npm", LooseVersion(NPM_MIN_VERSION))
+    npm_path, version = find_npm_executable()
     if not npm_path:
+        print(NPM_NOT_FOUND_MESSAGE)
+        return False
+    if not version:
+        print(NPM_MACHING_VERSION_NOT_FOUND_MESSAGE % NPM_MIN_VERSION)
         return False
 
     return True
 
 
 def have_winrm():
     # `winrm -h` should print 'winrm version ...' and exit 1
     try: