Issue 2956 - Ensure the correct version of jsshell is present

utils.py

Index: utils.py
diff --git a/utils.py b/utils.py
index ac3014ae675e7cf67fd33965c7f4f5a2eb13195b..4b11c3cc46b4c520e67b4dcb0e82aca81713c5dd 100644
--- a/utils.py
+++ b/utils.py
@@ -7,6 +7,8 @@
 import os
 import platform
 from StringIO import StringIO
+import shutil
+import subprocess
 import sys
 import urllib
 import zipfile
@@ -14,14 +16,27 @@ import zipfile
 def ensureJSShell():
   baseDir = os.path.dirname(__file__)
   shell_dir = os.path.join(baseDir, 'mozilla')
+
   if not os.path.exists(shell_dir):
     os.makedirs(shell_dir)
   if sys.platform == 'win32':
     path = os.path.join(shell_dir, 'js.exe')
   else:
     path = os.path.join(shell_dir, 'js')
+
   if os.path.exists(path):
-    return path
+    # Return the path if jsshell is present and its version is correct
+    try:
+      version = [s for s in subprocess.check_output([path, "--help"]).split("\n")

[Sebastian Noack, 2015/11/18 19:53:37]

This can be simplified:

  m = re.search(r"^Version:.*(\d+)", subprocess.check_output([path, "--help"]),
re.MULTILINE)
  if m and m.group(1) == "31":
    ..

Also consider moving the version as well as the URL into a global at the top of
the file to make it easier to find the stuff that need to be changed when
updating.

[kzar, 2015/11/19 10:56:41]

Done.

+                 if s.startswith("Version")][0].split()[1]
+    except (IndexError, subprocess.CalledProcessError):
+      pass
+    else:
+      if version == "JavaScript-C31.5.0esrpre":

[sergei, 2015/11/18 15:30:06]

Nit: I don't know the convention for buildtools but I would read this version
string from some text file. So, the updater of jsshell need to edit this file
instead of python script. Seems also better for the history log.

[sergei, 2015/11/18 15:30:06]

Nit: In addition it seems better to show some warning explaining what happened.
Otherwise, I'm afraid, the updater will have to spend some time investigating
who has reverted just updated version?...

I would not like to overcomplicate it, however I would like to mention the
following dirty solution. The updater should append the line with a new version.
So
if version list is not empty:
  if version is not in version list:
    # instead of downgrading show the warning
    warning("Don't forget to update jsshell.version")
  else if version is not last entry in version list:
    trace("Updating jsshell")
    rmtree...

[kzar, 2015/11/18 15:40:49]

Well I would have separated the version string from the code, but as the
downloaded version is also determined by a hard-coded string in this script I
didn't see the point.

As for showing a warning instead of upgrading jsshell, I disagree. I think that
the behaviour I implemented is more consistent with how it currently works. (Ie
the correct version of jsshell is grabbed magically in the background.)

[sergei, 2015/11/18 15:50:18]

I have missed it, in this case it's totally fine for me and, I guess, we can
omit the downgrading warning.
I meant the warning about downgrading, upgrading can be quite.

+        return path
+    # Otherwise remove the old / faulty version, we'll re-download
+    shutil.rmtree(shell_dir)
+    os.makedirs(shell_dir)
 
   supported_platforms = {
     'win32': 'win32',