1377 - Blacklist dependencies from SCM tracking

ensure_dependencies.py

Index: ensure_dependencies.py
===================================================================
--- a/ensure_dependencies.py
+++ b/ensure_dependencies.py
@@ -25,7 +25,9 @@
 import logging
 import subprocess
 import urlparse
+
 from collections import OrderedDict
+from ConfigParser import RawConfigParser
 
 USAGE = """
 A dependencies file should look like this:
@@ -42,7 +44,20 @@
   buildtools = buildtools hg:016d16f7137b git:f3f8692f82e5
 """
 
-class Mercurial():
+class SCM(object):
+  def _ignore(self, target, file_name):

[Wladimir Palant, 2014/10/07 22:30:14]

That should simply be a helper function _ensure_line_exists, no object
inheritance necessary.

[aalvz, 2014/10/08 13:49:18]

Done.

[Wladimir Palant, 2014/10/08 21:40:30]

And the same file format? That's merely a coincidence that you can reuse the
same code here. This is a helper function with no relation to revision control
systems. Once again, using inheritance here implies similarities that don't
exist in reality.

+    with open(file_name, 'a+') as f:
+      file_content = [l.strip() for l in f.readlines()]
+      if not target in file_content:
+        if len(file_content):
+          f.seek(-1, os.SEEK_CUR)
+          last_character = f.read(1)
+          if not last_character in "\r\n":

[Wladimir Palant, 2014/10/07 22:30:14]

No need to overcomplicate this:

file_content.append(line)
f.seek(0, os.SEEK_SET)
for l in file_content:
  print >>f, l
f.truncate()

Yes, this will overwrite the entire file contents - but the performance
difference shouldn't matter here.

[aalvz, 2014/10/08 13:49:18]

I think the solution you are suggesting and the current one are not that
different and is not more natural in the first solution instead of writing and
overwriting it all? :)

[mathias, 2014/10/08 17:37:55]

While the first point is not really true (though it's not obvious that the outer
`if` block ensures the file having a size of 1 or more), the second being
arguable and the third easy to fix,.. I do see that leaving these caveats for
the Python interpreter to handle is indeed a better choice, especially when
unifying the line-breaks is classified a "desired" side-effect.

Will become adjusted in the next patch set.

+            f.write(os.linesep)
+        f.write(target)
+        f.write(os.linesep)
+
+class Mercurial(SCM):
   def istype(self, repodir):
     return os.path.exists(os.path.join(repodir, ".hg"))
 
@@ -67,7 +82,30 @@
   def update(self, repo, rev):
     subprocess.check_call(["hg", "update", "--repository", repo, "--quiet", "--check", "--rev", rev])
 
-class Git():
+  def ignore(self, target, repo):
+
+    if not self.istype(target):
+
+      config_path = os.path.join(repo, ".hg", "hgrc")
+      ignore_path = os.path.join(".", ".hg", "dependencies")
+
+      config = RawConfigParser()
+      config.read(config_path)
+
+      if not config.has_section("ui"):
+        config.add_section("ui")
+
+      if not config.has_option("ui", "ignore.dependencies"):
+        config.set("ui", "ignore.dependencies", ignore_path)
+        with open(config_path, "w") as stream:
+          config.write(stream)
+      else:
+        ignore_path = config.get("ui", "ignore.dependencies")

[Wladimir Palant, 2014/10/07 22:30:14]

Please set that option regardless of whether it is already there - since it has
to be an absolute path, it might be wrong if the repository moved. So you should
always update it in order to have it point to the right location.

[mathias, 2014/10/08 16:32:11]

Agreed, this is necessary due to the issue above, and will be fixed in the next
patch set. Still a bit awkward though..

+
+      module = os.path.relpath(target, repo)
+      self._ignore(module, ignore_path)
+
+class Git(SCM):
   def istype(self, repodir):
     return os.path.exists(os.path.join(repodir, ".git"))
 
@@ -87,6 +125,11 @@
   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")
+    self._ignore(module, exclude_file)
+
 repo_types = {
   "hg": Mercurial(),
   "git": Git(),
@@ -161,7 +204,6 @@
 def ensure_repo(parentrepo, target, roots, sourcename):
   if os.path.exists(target):
     return
-
   parenttype = get_repo_type(parentrepo)
   type = None
   for key in roots:
@@ -174,6 +216,10 @@
   logging.info("Cloning repository %s into %s" % (url, target))
   repo_types[type].clone(url, target)
 
+  for repo in repo_types.itervalues():
+    if repo.istype(parentrepo):
+      repo.ignore(target, parentrepo)
+
 def update_repo(target, revisions):
   type = get_repo_type(target)
   if type is None: