Issue 2443 - Honour SKIP_DEPENDENCY_UPDATES environment variable

ensure_dependencies.py

 #!/usr/bin/env python
 # coding: utf-8
 
 # 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
@@ skipping 16 matching lines @@
   # 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
 """
 
+SKIP_DEPENDENCY_UPDATES = os.environ.get(
+  "SKIP_DEPENDENCY_UPDATES", ""
+).lower() not in ("", "0", "false")
+
 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):
@@ skipping 43 matching lines @@
     source = source.rstrip("/")
     if not source.endswith(".git"):
       source += ".git"
     subprocess.check_call(["git", "clone", "--quiet", source, target])
 
   def get_revision_id(self, repo, rev="HEAD"):
     command = ["git", "rev-parse", "--revs-only", rev + '^{commit}']
     return subprocess.check_output(command, cwd=repo).strip()
 
   def pull(self, repo):
+    # Fetch tracked branches, new tags and the list of available remote branches
     subprocess.check_call(["git", "fetch", "--quiet", "--all", "--tags"], cwd=repo)
+    # Next we need to ensure all remote branches are tracked
+    newly_tracked = False
+    remotes = subprocess.check_output(["git", "branch", "--remotes"], cwd=repo)
+    for match in re.finditer(r"^\s*(origin/(\S+))$", remotes, re.M):
+      remote, local = match.groups()
+      with open(os.devnull, "wb") as devnull:
+        if subprocess.call(["git", "branch", "--track", local, remote],
+                           cwd=repo, stdout=devnull, stderr=devnull) == 0:
+          newly_tracked = True
+    # Finally fetch any newly tracked remote branches
+    if newly_tracked:
+      subprocess.check_call(["git", "fetch", "--quiet", "origin"], cwd=repo)
 
   def update(self, repo, rev):
     subprocess.check_call(["git", "checkout", "--quiet", rev], cwd=repo)
 
   def ignore(self, target, repo):
     module = os.path.relpath(target, repo)
     exclude_file = os.path.join(repo, ".git", "info", "exclude")
     _ensure_line_exists(exclude_file, module)
 
   def postprocess_url(self, url):
@@ skipping 49 matching lines @@
         if spec:
           result[key] = spec
     return result
   except IOError, e:
     if e.errno != errno.ENOENT:
       raise
     return None
 
 def safe_join(path, subpath):
   # This has been inspired by Flask's safe_join() function
-  forbidden = set([os.sep, os.altsep]) - set([posixpath.sep, None])
+  forbidden = {os.sep, os.altsep} - {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
+
+  if SKIP_DEPENDENCY_UPDATES:
+    logging.warning("SKIP_DEPENDENCY_UPDATES environment variable set, "
+                    "%s not cloned", 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)
 
@@ skipping 20 matching lines @@
     return
 
   if type in revisions:
     revision = revisions[type]
   elif "*" in revisions:
     revision = revisions["*"]
   else:
     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)

[Wladimir Palant, 2015/05/06 18:17:32]

For reference, I think the check belongs here:

if resolved_revision != current_revision and
os.environ.get("SKIP_ENSURE_DEPENDENCIES", "").lower() not in ("", "0",
"false"):
  logging.warning(...)
  sys.exit()

The repository will still be created if it doesn't exist yet - that's typically
a one-time thing when the build is created for the first time. I don't see any
use case where that would be an issue.

[kzar, 2015/05/07 13:02:38]

OK I've done this, I have some reservations though. The sys.exit here would
cause nested dependencies to be treated differently depending on the checked out
revision of the parent dependency, so instead I return.

Thing is I'm still not convinced about my latest patchset, it seems weird that
the script clones all the dependencies but then doesn't check out the
appropriate revision for them. I can't help but feel patch set 2 would have been
easier to reason with, but I'm happy to go with whatever the consensus is, I
don't expect to use this feature myself.

[Sebastian Noack, 2015/05/07 13:17:19]

Yeah, I'm pretty sure Wladimir meant to return here.
Patchset 2 certainly was a footgun. I really don't want to start over that
discussion. But I see your point about the kinda unexpected behavior with newly
cloned repos. It's not a very common case, but if you think it would be better
to either perform the update regardless of SKIP_DEPENDENCY_UPDATES  for freshly
cloned repos, or to skip cloning repos as well, fine with me.

[kzar, 2015/05/07 13:44:29]

Done.

-  if not resolved_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)
+  current_revision = repo_types[type].get_revision_id(target)
+
+  if resolved_revision != current_revision:
+    if SKIP_DEPENDENCY_UPDATES:
+      logging.warning("SKIP_DEPENDENCY_UPDATES environment variable set, "
+                      "%s not checked out to %s", target, revision)
+      return
+
     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:
+      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)
+
     logging.info("Updating repository %s to revision %s" % (target, resolved_revision))
     repo_types[type].update(target, resolved_revision)
 
 def resolve_deps(repodir, level=0, self_update=True, overrideroots=None, skipdependencies=set()):
   config = read_deps(repodir)
   if config is None:
     if level == 0:
       logging.warning("No dependencies file in directory %s, nothing to do...\n%s" % (repodir, USAGE))
     return
   if level >= 10:
     logging.warning("Too much subrepository nesting, ignoring %s" % repo)
+    return
 
   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", {}), revisions["_source"])
     update_repo(target, revisions)
@@ skipping 30 matching lines @@
     if not pattern in file_content:
       file_content.append(pattern)
       f.seek(0, os.SEEK_SET)
       f.truncate()
       for l in file_content:
         print >>f, l
 
 if __name__ == "__main__":
   logging.basicConfig(format='%(levelname)s: %(message)s', level=logging.INFO)
 
-  if os.environ.get("SKIP_ENSURE_DEPENDENCIES", "").lower() not in ("", "0", "false"):
-    logging.warning("SKIP_ENSURE_DEPENDENCIES environment variable set, skipping.")
-    sys.exit()
-
   parser = argparse.ArgumentParser(description="Verify dependencies for a set of repositories, by default the repository of this script.")
   parser.add_argument("repos", metavar="repository", type=str, nargs="*", help="Repository path")
   parser.add_argument("-q", "--quiet", action="store_true", help="Suppress informational output")
   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)