Issue 5857 - Retry npm install on failure

ensure_dependencies.py

Index: ensure_dependencies.py
diff --git a/ensure_dependencies.py b/ensure_dependencies.py
index b1aedfde38ef4a12145691aa313dd2635bf6b613..2f80143225c3a7024f8f1eb6565b107c26aaa262 100755
--- a/ensure_dependencies.py
+++ b/ensure_dependencies.py
@@ -44,6 +44,8 @@ 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):
@@ -271,10 +273,18 @@ def resolve_npm_dependencies(target, vcs):
         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:
@@ -365,7 +375,8 @@ def resolve_deps(repodir, level=0, self_update=True, overrideroots=None, skipdep
         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)