git-sync-deps tool

tools/grab_deps.py

+#!/usr/bin/python
+# Copyright 2014 Google Inc.
+#
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+

[mtklein, 2014/04/11 15:58:47]

Add a brief usage?

# Run to grab Skia deps, with optional platform support.
# python tools/grab_deps.py [android] [chromeos] [barelinux]

[hal.canary, 2014/04/11 17:46:51]

Done.

+
+import git_utils
+import misc_utils
+import os
+import subprocess
+import sys
+
+
+SKIA_CHECKOUT_PATH = os.path.join(os.path.dirname(__file__), os.pardir)
+
+
+def rm_directory_if_exists(directory):
+  if os.path.isdir(directory) and not os.path.islink(directory):
+    shutil.rmtree(directory)
+  elif os.path.lexists(directory):
+    os.remove(directory)
+
+
+def git_checkout_to_directory(repo, checkout, directory, verbose=False):
+  """TODO(halcanary): document this function.

[epoger, 2014/04/11 16:16:11]

Please document it before committing, unless this is urgent.

[hal.canary, 2014/04/11 17:46:51]

Done.

+  """
+  git = git_utils.git_executable()
+  assert git
+  try:
+    with misc_utils.ChangeDir(directory):
+      if verbose:
+        print '%s\n>>>> %s\n>>>> %s\n' % (directory, repo, checkout)
+      subprocess.check_call([git, 'fetch'])
+      subprocess.check_call([git, 'checkout', '--quiet', checkout])
+  except:
+    rm_directory_if_exists(directory)
+    subprocess.check_call([git, 'clone', repo, directory])
+    with misc_utils.ChangeDir(directory):
+      if verbose:
+        print '%s\n>>>> %s\n>>>> %s\n' % (directory, repo, checkout)
+      subprocess.check_call([git, 'checkout', '--quiet', checkout])
+
+
+def main(argv):
+  deps_file = os.path.join(SKIA_CHECKOUT_PATH, 'DEPS')
+  DEPS = {}
+  execfile(deps_file, DEPS)
+  dependencies = DEPS['deps'].copy()
+  for arg in argv:

[epoger, 2014/04/11 16:16:11]

Please add comment like:

# Add OS-specific dependencies

[epoger, 2014/04/11 16:16:11]

Does it make sense for us to pick up multiple OSes from deps_os?  Or should at
most one OS flavor be specified?

[hal.canary, 2014/04/11 17:46:51]

Yes, since I might test my branch by cross-compiling to multiple systems.

[hal.canary, 2014/04/11 17:46:51]

Done.

+    if arg not in DEPS['deps_os']:
+      print arg, '?'

[epoger, 2014/04/11 16:16:11]

Maybe instead of print/exit, do this?

raise Exception('arg %s not found within deps_os keys %s' % (
    arg, DEPS['deps_os']))

[hal.canary, 2014/04/11 17:46:51]

Done.

+      exit(1)
+    for dep in DEPS['deps_os'][arg]:
+      dependencies[dep] = DEPS['deps_os'][arg][dep]
+  for directory in dependencies:
+    if '@' in dependencies[directory]:
+      repo, checkout = dependencies[directory].split('@', 1)

[epoger, 2014/04/11 16:16:11]

maybe "tag", "githash", "version", or "revision" would be more descriptive than
"checkout"?

[hal.canary, 2014/04/11 17:46:51]

I mean 'checkout' as anything that can be passed to `git checkout`.  I don't
know a better term.

[epoger, 2014/04/11 18:28:39]

In "git checkout --help", they use the term "tree-ish", which I think is
terrible-ish:

       git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...
           When <paths> or --patch are given, git checkout does not switch
           branches. It updates the named paths in the working tree from the
           index file or from a named <tree-ish> (most often a commit). In
           this case, the -b and --track options are meaningless and giving
           either of them results in an error. The <tree-ish> argument can be
           used to specify a specific tree-ish (i.e. commit, tag or tree) to
           update the index for the given paths before updating the working
           tree.

I think your "checkoutable" is better than that, at least!

I suppose I would prefer "commit_or_tag" , but if you prefer "checkoutable"
that's fine too.

+    else:
+      repo, checkout = dependencies[directory], 'origin/master'
+
+    with misc_utils.ChangeDir(SKIA_CHECKOUT_PATH):
+      git_checkout_to_directory(repo, checkout, directory, verbose=True)
+
+if __name__ == '__main__':
+  main(sys.argv[1:])
+