Issue 170 - Replacing Mercurial subrepositories

ensure_dependencies.py

 #!/usr/bin/env python
 # coding: utf-8

[Sebastian Noack, 2014/09/04 10:07:30]

I don't find any non-ascii characters below. Also according to PEP-8, this
should be avoided, except you really have to use non-ascii characters in your
comments or docstring, e.g. for author names. For regular strings you should
always use unicode literals like u'\xe4'.

[Wladimir Palant, 2014/09/04 13:32:12]

We specify this in all Python files, UTF-8 is the encoding we generally use for
all source code files. I guess this can go away once we migrate to Python 3.

[Felix Dahlke, 2014/09/05 08:55:53]

What PEP-8 says, specifically, is this:

"Files using ASCII (in Python 2) or UTF-8 (in Python 3) should not have an
encoding declaration."

Since we're using UTF-8 in Python 2, we need the encoding declaration.

[Sebastian Noack, 2014/09/05 09:29:30]

Keep reading ;P

"... non-default encodings should be used only for test purposes or when a
comment or docstring needs to mention an author name that contains non-ASCII
characters; otherwise, using \x, \u, \U, or \N escapes is the preferred way to
include non-ASCII data in string literals."

[Felix Dahlke, 2014/09/05 09:55:01]

You left the first part out: "In the standard library [...]" - I do think PEP8
intends to leave this up to us. That said, I really don't have a strong opinion
on it, let's just be consistent. I think we should discuss this in a separate
issue and then apply the resulting change to all source files - feel free to
file one.

 
 # This file is part of the Adblock Plus build tools,
 # Copyright (C) 2006-2014 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import sys
 import os
+import posixpath
 import re
 import io
 import errno
-import warnings
+import logging
 import subprocess
 import urlparse
 from collections import OrderedDict
+
+USAGE = """
+A dependencies file should look like this:
+
+  # VCS-specific root URLs for the repositories
+  _root = hg:https://hg.adblockplus.org/ git:https://github.com/adblockplus/
+  # File to update this script from (optional)
+  _self = buildtools/ensure_dependencies.py
+  # Check out elemhidehelper repository into extensions/elemhidehelper directory
+  # at tag "1.2".
+  extensions/elemhidehelper = elemhidehelper 1.2
+  # Check out buildtools repository into buildtools directory at VCS-specific
+  # revision IDs.
+  buildtools = buildtools hg:016d16f7137b git:f3f8692f82e5
+"""
 
 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])
 
   def get_revision_id(self, repo, rev=None):
     command = ["hg", "id", "--repository", repo, "--id"]
     if rev:
       command.extend(["--rev", rev])
 
     # Ignore stderr output and return code here: if revision lookup failed we
     # should simply return an empty string.
-    result, dummy = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate()
+    result = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate()[0]
     return result.strip()
 
   def pull(self, repo):
     subprocess.check_call(["hg", "pull", "--repository", repo, "--quiet"])
 
   def update(self, repo, rev):
     subprocess.check_call(["hg", "update", "--repository", repo, "--quiet", "--check", "--rev", rev])
 
 class Git():
   def istype(self, repodir):
@@ skipping 15 matching lines @@
   def update(self, repo, rev):
     subprocess.check_call(["git", "-C", repo, "checkout", "--quiet", rev])
 
 repo_types = {
   "hg": Mercurial(),
   "git": Git(),
 }
 
 def parse_spec(path, line):
   if "=" not in line:
-    warnings.warn("Invalid line in file %s: %s" % (path, line))
+    logging.warning("Invalid line in file %s: %s" % (path, line))
     return None, None
 
   key, value = line.split("=", 1)
   key = key.strip()
   items = value.split()
   if not len(items):
-    warnings.warn("No value specified for key %s in file %s" % (key, path))
+    logging.warning("No value specified for key %s in file %s" % (key, path))
     return key, None
 
   result = OrderedDict()
   if not key.startswith("_"):
     result["_source"] = items.pop(0)
 
   for item in items:
     if ":" in item:
       type, value = item.split(":", 1)
     else:
       type, value = ("*", item)
     if type in result:
-      warnings.warn("Ignoring duplicate value for type %s (key %s in file %s)" % (type, key, path))
+      logging.warning("Ignoring duplicate value for type %s (key %s in file %s)" % (type, key, path))
     else:
       result[type] = value
   return key, result
 
 def read_deps(repodir):
   result = {}
   deps_path = os.path.join(repodir, "dependencies")
   try:
     with io.open(deps_path, "rt", encoding="utf-8") as handle:
       for line in handle:
         # Remove comments and whitespace
         line = re.sub(r"#.*", "", line).strip()
         if not line:
           continue
 
         key, spec = parse_spec(deps_path, line)
         if spec:
           result[key] = spec
     return result
   except IOError, e:
     if e.errno != errno.ENOENT:
       raise
     return None
 
 def safe_join(path, subpath):
-  return os.path.join(path, *[f for f in subpath.split("/") if f not in (os.curdir, os.pardir)])
+  # This has been inspired by Flask's safe_join() function
+  forbidden = set([os.sep, os.altsep]) - set([posixpath.sep, None])
+  if any(sep in subpath for sep in forbidden):
+    raise Exception("Illegal directory separator in dependency path %s" % subpath)
+
+  normpath = posixpath.normpath(subpath)
+  if posixpath.isabs(normpath):
+    raise Exception("Dependency path %s cannot be absolute" % subpath)
+  if normpath == posixpath.pardir or normpath.startswith(posixpath.pardir + posixpath.sep):
+    raise Exception("Dependency path %s has to be inside the repository" % subpath)
+  return os.path.join(path, *normpath.split(posixpath.sep))
 
 def get_repo_type(repo):
   for name, repotype in repo_types.iteritems():
     if repotype.istype(repo):
       return name
   return None
 
 def ensure_repo(parentrepo, target, roots, sourcename):
   if os.path.exists(target):
     return
 
   parenttype = get_repo_type(parentrepo)
   type = None
   for key in roots:
     if key == parenttype or (key in repo_types and type is None):
       type = key
   if type is None:
     raise Exception("No valid source found to create %s" % target)
 
   url = urlparse.urljoin(roots[type], sourcename)
-  print "Cloning repository %s into %s" % (url, target)
+  logging.info("Cloning repository %s into %s" % (url, target))
   repo_types[type].clone(url, target)
 
 def update_repo(target, revisions):
   type = get_repo_type(target)
   if type is None:
-    warnings.warn("Type of repository %s unknown, skipping update" % target)
+    logging.warning("Type of repository %s unknown, skipping update" % target)
     return
 
   if type in revisions:
     revision = revisions[type]
   elif "*" in revisions:
     revision = revisions["*"]
   else:
-    warnings.warn("No revision specified for repository %s (type %s), skipping update" % (target, type))
+    logging.warning("No revision specified for repository %s (type %s), skipping update" % (target, type))
     return
 
   resolved_revision = repo_types[type].get_revision_id(target, revision)
   if not resolved_revision:
-    print "Revision %s is unknown, downloading remote changes" % revision
+    logging.info("Revision %s is unknown, downloading remote changes" % revision)
     repo_types[type].pull(target)
     resolved_revision = repo_types[type].get_revision_id(target, revision)
     if not resolved_revision:
       raise Exception("Failed to resolve revision %s" % revision)
 
   current_revision = repo_types[type].get_revision_id(target)
   if resolved_revision != current_revision:
-    print "Updating repository %s to revision %s" % (target, resolved_revision)
+    logging.info("Updating repository %s to revision %s" % (target, resolved_revision))
     repo_types[type].update(target, resolved_revision)
 
-def resolve_deps(repodir, level=0):
+def resolve_deps(repodir, level=0, self_update=True, overrideroots=None, skipdependencies=set()):
   config = read_deps(repodir)
   if config is None:
     if level == 0:
-      warnings.warn("No dependencies file in directory %s, nothing to do..." % repodir)
+      logging.warning("No dependencies file in directory %s, nothing to do...\n%s" % (repodir, USAGE))
     return
   if level >= 10:
-    warnings.warn("Too much subrepository nesting, ignoring %s" % repo)
-
-  for dir in config:
-    if dir.startswith("_"):
+    logging.warning("Too much subrepository nesting, ignoring %s" % repo)
+
+  if overrideroots is not None:
+    config["_root"] = overrideroots
+
+  for dir, revisions in config.iteritems():
+    if dir.startswith("_") or revisions["_source"] in skipdependencies:
       continue
     target = safe_join(repodir, dir)
-    ensure_repo(repodir, target, config.get("_root", {}), config[dir]["_source"])
-    update_repo(target, config[dir])
-    resolve_deps(target, level + 1)
-
-  if level == 0 and "_self" in config and "*" in config["_self"]:
+    ensure_repo(repodir, target, config.get("_root", {}), revisions["_source"])
+    update_repo(target, revisions)
+    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, e:
       if e.errno != errno.ENOENT:
         raise
-      warnings.warn("File %s doesn't exist, skipping self-update" % source)
+      logging.warning("File %s doesn't exist, skipping self-update" % source)
       return
 
     target = __file__
     with io.open(target, "rb") as handle:
       targetdata = handle.read()
 
     if sourcedata != targetdata:
-      print "Updating %s from %s, don't forget to commit" % (source, target)
+      logging.info("Updating %s from %s, don't forget to commit" % (source, target))
       with io.open(target, "wb") as handle:
         handle.write(sourcedata)
-      print "Restarting %s" % target
       if __name__ == "__main__":
+        logging.info("Restarting %s" % target)
         os.execv(sys.executable, [sys.executable, target] + sys.argv[1:])

[Sebastian Noack, 2014/09/04 10:07:30]

I think you have to call os.exit(0) afterwards. Otherwise the script is
restarted for every repo in the loop.

[Sebastian Noack, 2014/09/04 13:50:17]

I just realized that you are using os.execv(), which doesn't return, but
replaces the current process. So you don't have to terminate the process
afterwards. In fact no code below will be executed anyway.

       else:
-        import importlib

[Sebastian Noack, 2014/09/04 10:07:30]

Is there a case where this script is actually imported as a module, or can we
just ignore that case?

Note that it only works as long as the name and expected arguments of
resolve_deps() don't change. And since the calling module will still keep a
reference to the old version of this module, it also won't work if this function
is called multiple times.

[Wladimir Palant, 2014/09/04 13:32:12]

build.py will import it as module. And - no, it shouldn't call it multiple
times.

Worst-case scenario: self-update succeeds but restart fails and build needs to
be started again. I think we can live with that.

[Sebastian Noack, 2014/09/04 13:44:52]

I think you didn't get it. When buildtools calls resolve_deps the second time,
it will run the old version. That is because when you reload a module, all
references to that module that have been imported before, still point to the old
version. And since we already updated the source code, the check above won't
work, and the code thinks that it already is at its newest version.

[Wladimir Palant, 2014/09/04 14:34:45]

I did get it, but I was rather referring to the scenario where the function
signature changes. As to the function being called multiple times from another
module - I don't really see how that would happen. Some long-running process
creating multiple builds from the same repository?

If you think that this code is too dangerous - we can consider running
ensure_dependencies.py via subprocess.check_call() from build.py. It will waste
resources, particularly problematic on the build server, but other than that it
should work the same.

[Wladimir Palant, 2014/09/04 15:55:38]

Actually, this doesn't seem to be that much of an issue. reload() doesn't create
a new module object, it updates the existing one. This means that all references
to this module object are still valid, merely references to objects or functions
are outdated. So calling ensure_dependencies.resolve_deps() will still be ok
even after the module was reloaded. There is only a problem if this function was
imported into the global namespace or no longer exists in the module.

[Sebastian Noack, 2014/09/04 16:22:00]

That is correct. Though this might be a footgun. I think I better like the idea
to call this script with the subprocess module. That approach would also be
robust against changes regarding the name and arguments of resolve_deps().

-        me = importlib.import_module(__name__)
-        reload(me)
-        me.resolve_deps(repodir, level)
-
-def showwarning(message, category, filename, lineno, file=sys.stderr, line=None):
-  print >>file, "%s:%i: %s" % (filename, lineno, message)
+        logging.warning("Cannot restart %s automatically, please rerun" % target)
 
 if __name__ == "__main__":
-  warnings.showwarning = showwarning
-
+  logging.basicConfig(format='%(levelname)s: %(message)s', level=logging.INFO)
   repos = sys.argv[1:]
   if not len(repos):
     repos = [os.getcwd()]
   for repo in repos:
     resolve_deps(repo)