Issue 5857 - Retry npm install on failure

ensure_dependencies.py

 #!/usr/bin/env 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/.
 
 import sys
 import os
 import posixpath
 import re
@@ skipping 26 matching lines @@
   adblockplus = adblockplus hg:893426c6a6ab git:git@github.com:user/adblockplus.git@b2ffd52b
   # Clone the adblockpluschrome repository into the adblockpluschrome directory,
   # from a specific Git repository, specifying the revision ID.
   adblockpluschrome = git:git@github.com:user/adblockpluschrome.git@1fad3a7
 '''
 
 SKIP_DEPENDENCY_UPDATES = os.environ.get(
     'SKIP_DEPENDENCY_UPDATES', ''
 ).lower() not in ('', '0', 'false')
 
+NPM_LOCKFILE = '.npm_install_lock'

[Sebastian Noack, 2017/10/11 23:22:33]

This file would have to be added to .hgignore/.gitignore, why not just putting
it inside node_modules, which has to be ignored anyway?

[tlucas, 2017/10/11 23:48:55]

It is added to .hgignore/.gitignore, please see below.
When "npm install" is run for the first time, there is no folder "node_modules"
yet - and i did not like the idea to manually add a folder and a file - and only
delete the file on failure.

[Sebastian Noack, 2017/10/12 00:08:00]

Ah right, I see. Yeah, if we can just make sure that it gets ignored, it's
probably not worth to deal with the scenario in which the node_modules folder
doesn't exist yet. LGTM.

+
 
 class Mercurial():
     def istype(self, repodir):
         return os.path.exists(os.path.join(repodir, '.hg'))
 
     def clone(self, source, target):
         if not source.endswith('/'):
             source += '/'
         subprocess.check_call(['hg', 'clone', '--quiet', '--noupdate', source, target])
 
@@ skipping 207 matching lines @@
 
         # In case a package.json does not exist at all or if there are no
         # production dependencies declared, we don't need to run npm and can
         # bail out early.
         if not package_data.get('dependencies', False):
             return
     except IOError:
         return
 
     try:
+        # Create an empty file, which gets deleted after successfully
+        # installing Node.js dependencies.
+        lockfile_path = os.path.join(target, NPM_LOCKFILE)
+        open(lockfile_path, 'a').close()
+
         cmd = ['npm', 'install', '--only=production', '--loglevel=warn']
         subprocess.check_output(cmd, cwd=target)
 
+        repo_types[vcs].ignore(os.path.join(target, NPM_LOCKFILE), target)

[kzar, 2017/10/13 10:14:23]

I wonder if we should just ignore this file consistently, since we never
un-ignore it again anyway?

[tlucas, 2017/10/13 10:31:36]

This file should - in a best case scenario - never remain. IMHO an entry in
.gitignore / .hgignore would only confuse people, if it never is created.

[kzar, 2017/10/13 11:20:42]

Yea, fair enough.

         repo_types[vcs].ignore(os.path.join(target, 'node_modules'), target)
+
+        os.remove(lockfile_path)
     except OSError as e:
         import errno
         if e.errno == errno.ENOENT:
             logging.error('Failed to install Node.js dependencies for %s,'
                           ' please ensure Node.js is installed.', target)
         else:
             raise
 
 
 def ensure_repo(parentrepo, parenttype, target, type, root, sourcename):
@@ skipping 70 matching lines @@
                 vcs = key
         source, rev = merge_seqs(sources.get('*'), sources.get(vcs))
 
         if not (vcs and source and rev):
             logging.warning('No valid source / revision found to create %s' % target)
             continue
 
         repo_cloned = ensure_repo(repodir, parenttype, target, vcs,
                                   _root.get(vcs, ''), source)
         repo_updated = update_repo(target, vcs, rev)
-        if repo_cloned or repo_updated:
+        recent_npm_failed = os.path.exists(os.path.join(target, NPM_LOCKFILE))
+        if repo_cloned or repo_updated or recent_npm_failed:
             resolve_npm_dependencies(target, vcs)
         resolve_deps(target, level + 1, self_update=False,
                      overrideroots=overrideroots, skipdependencies=skipdependencies)
 
     if self_update and '_self' in config and '*' in config['_self']:
         source = safe_join(repodir, config['_self']['*'])
         try:
             with io.open(source, 'rb') as handle:
                 sourcedata = handle.read()
         except IOError as e:
@@ skipping 38 matching lines @@
     args = parser.parse_args()
 
     if args.quiet:
         logging.disable(logging.INFO)
 
     repos = args.repos
     if not len(repos):
         repos = [os.path.dirname(__file__)]
     for repo in repos:
         resolve_deps(repo)