Switching nacl_deps_bump to git.

nacl_deps_bump.py

Index: nacl_deps_bump.py
===================================================================
--- nacl_deps_bump.py	(revision 293713)
+++ nacl_deps_bump.py	(working copy)
@@ -19,11 +19,10 @@
 import sys
 import time
 
-# This dependency can be installed with:
-# apt-get install python-svn
-import pysvn
 
+NACL_GIT_ROOT = 'native_client'
 
+
 def ReadFile(filename):
   fh = open(filename, "r")
   try:
@@ -40,16 +39,6 @@
     fh.close()
 
 
-# The 'svn:' URL is faster than the 'http:' URL but only works if you
-# have SVN committer credentials set up.
-# NACL_SVN = 'http://src.chromium.org/native_client/trunk/src/native_client'
-NACL_SVN = 'svn://svn.chromium.org/native_client/trunk/src/native_client'
-# When querying for the latest revision, use the root URL.  Otherwise,
-# if the most recent change touched a branch and not trunk, the query
-# will return an empty list.
-NACL_SVN_ROOT = 'svn://svn.chromium.org/native_client'
-
-
 def MatchKey(data, key):
   if key == 'nacl_revision':
     # Pattern for fields in Chromium's DEPS file.
@@ -74,91 +63,24 @@
                   data[match.end(1):]])
 
 
-def SetDepsFieldWithComment(data, key, value, comment):
-  assert key == 'nacl_revision', key
-  match = MatchKey(data, key)
-  return data[:match.start(1)] + value + "', # " + comment + data[match.end():]
+def GetNaClRev(git_dir):
+  return subprocess.check_output(
+      ['git', 'rev-parse', 'origin/master'], cwd=git_dir).strip()
 
 
-def GetLatestRootRev():
-  rev = pysvn.Revision(pysvn.opt_revision_kind.head)
-  lst = pysvn.Client().log(NACL_SVN_ROOT, revision_start=rev, revision_end=rev,
-                           discover_changed_paths=True)
-  assert len(lst) == 1, lst
-  return lst[0].revision.number
-
-
-def GetNaClRev():
-  # Find the revision number for the most recent commit to the subdir
-  # specified by NACL_SVN.  Unfortunately, SVN does not make it easy
-  # to query this directly.  If the HEAD commit did not change the
-  # subdir (e.g. because it changed other branches), "svn log
-  # -rHEAD:HEAD <subdir>" yields an empty list.  This means we must
-  # start with the latest root revision and search backwards until we
-  # hit a change to the subdir.
-  now = time.time()
-  rev_num = GetLatestRootRev()
-  while True:
-    rev = pysvn.Revision(pysvn.opt_revision_kind.number, rev_num)
-    lst = pysvn.Client().log(NACL_SVN, revision_start=rev, revision_end=rev)
-    assert len(lst) in (0, 1), lst
-    if len(lst) == 1:
-      age_mins = (now - lst[0].date) / 60
-      print 'r%i committed %.1f minutes ago' % (
-          lst[0].revision.number, age_mins)
-      return lst[0].revision.number
-    rev_num -= 1
-
-
-def GetLog(rev1, rev2):
-  # Get info on commits from rev1 (older, exclusive) to rev2 (newer,
-  # inclusive).  Returns commit info formatted as a string, and a list
-  # of author e-mail addresses.
-  items = pysvn.Client().log(
-      NACL_SVN,
-      revision_start=pysvn.Revision(pysvn.opt_revision_kind.number, rev1 + 1),
-      revision_end=pysvn.Revision(pysvn.opt_revision_kind.number, rev2))
+def GetLog(git_dir, rev1, rev2):
+  stdout = subprocess.check_output(
+      ['git', 'log', '--pretty=format:%h %ae %s', rev1 + '..' + rev2],

[Mark Seaborn, 2015/01/28 16:34:21]

Can you add "--reverse", so that the oldest changes are listed first, please? 
That would match the old behaviour and pnacl/deps_update.py.

+      cwd=git_dir)
   got = []
   authors = []
-  for item in items:
-    line1 = item.message.split('\n')[0]
-    author = item.author.split('@', 1)[0]
-    if line1 == 'Update .DEPS.git' and author == 'chrome-admin':
-      # Skip these automated commits.
-      continue
-    authors.append(item.author)
-    got.append('r%i: (%s) %s\n' % (item.revision.number, author, line1))
+  for line in stdout.splitlines():
+    h, author, message = line.split(' ', 2)
+    authors.append(author)
+    got.append('%s (%s) %s\n' % (h, author, message))

[Mark Seaborn, 2015/01/28 16:44:15]

Can you add a colon, i.e.
  '%s: (%s) %s\n'
to match the old format and match pnacl/deps_update.py?

   return ''.join(got), authors
 
 
-def AssertIsGitRev(git_rev):
-  assert re.match('[0-9a-f]{40}$', git_rev) is not None, git_rev
-
-
-# Returns the result of "git svn find-rev", which converts Git commit IDs
-# to SVN revision numbers and vice versa.
-def GitSvnFindRev(git_dir, arg):
-  stdout = subprocess.check_output(['git', 'svn', 'find-rev', arg],
-                                   cwd=git_dir)
-  # git-svn annoyingly sends "Partial-building" log output to stdout rather
-  # than stderr, so we have to ignore everything except the result on the
-  # last line.
-  lines = stdout.strip().split('\n')
-  return lines[-1].strip()
-
-
-def GitToSvnRev(git_dir, git_rev):
-  AssertIsGitRev(git_rev)
-  rev = GitSvnFindRev(git_dir, git_rev)
-  return int(rev)
-
-
-def SvnToGitRev(git_dir, svn_rev):
-  git_rev = GitSvnFindRev(git_dir, 'r%i' % int(svn_rev))
-  AssertIsGitRev(git_rev)
-  return git_rev
-
-
 def RunTrybots():
   try_cmd = ['git', 'cl', 'try']
   # Run the default trybots.
@@ -183,8 +105,8 @@
 
 def Main(args):
   parser = optparse.OptionParser('%prog [options]\n\n' + __doc__.strip())
-  parser.add_option('-r', '--revision', default=None, type='int',
-                    help='NaCl SVN revision to use (default is HEAD)')
+  parser.add_option('-r', '--revision', default=None,
+                    help='NaCl GIT revision to use (default is HEAD)')

[Mark Seaborn, 2015/01/28 16:34:21]

Nit: capitalise as "Git"

   parser.add_option('--no-fetch', action='store_true', default=False,
                     help='Do not run "git fetch".  This is useful to speed'
                       ' up rerunning the script if you know that the local Git'
@@ -215,16 +137,16 @@
   if len(changes) != 0:
     raise AssertionError('You have uncommitted changes:\n%s' % changes)
 
-  svn_rev = options.revision
-  if svn_rev is None:
-    svn_rev = GetNaClRev()
-
-  nacl_git_dir = 'native_client'
+  nacl_git_dir = NACL_GIT_ROOT
   if not options.no_fetch:
     subprocess.check_call(['git', 'fetch'])
     subprocess.check_call(['git', 'fetch'], cwd=nacl_git_dir)

[JF, 2015/01/28 07:12:17]

I'm not sure I understand what the NaCl cwd is expected to be. I assume we now
need a git checkout for this to work? Will the script warn if it's run from the
wrong place?

[Mark Seaborn, 2015/01/28 16:34:21]

This just assumes it's run from a Chromium checkout with the usual native_client
subdir.  Brad is not changing this assumption.

[bradn, 2015/01/28 17:37:32]

Correct.

[bradn, 2015/01/28 17:37:32]

What mark said.

 
-  branch_name = 'nacl-deps-r%s' % svn_rev
+  new_rev = options.revision
+  if new_rev is None:
+   new_rev = GetNaClRev(nacl_git_dir)
+
+  branch_name = 'nacl-deps-%s' % new_rev
   if options.force_branch:
     checkout_opt = '-B'
   else:
@@ -233,17 +155,12 @@
                          'origin/master'])
 
   deps_data = ReadFile('DEPS')
-  old_rev_git = GetDepsField(deps_data, 'nacl_revision')
+  old_rev = GetDepsField(deps_data, 'nacl_revision')
 
-  new_rev_comment = 'from svn revision r%s' % svn_rev
-  new_rev_git = SvnToGitRev(nacl_git_dir, svn_rev)
-  deps_data = SetDepsFieldWithComment(deps_data, 'nacl_revision', new_rev_git,
-                                      new_rev_comment)
+  deps_data = SetDepsField(deps_data, 'nacl_revision', new_rev)
 
-  old_rev = GitToSvnRev(nacl_git_dir, old_rev_git)
-
-  msg_logs, authors = GetLog(old_rev, svn_rev)
-  msg = 'NaCl: Update revision in DEPS, r%i -> r%i' % (old_rev, svn_rev)
+  msg_logs, authors = GetLog(nacl_git_dir, old_rev, new_rev)
+  msg = 'NaCl: Update revision in DEPS, %s -> %s' % (old_rev[:9], new_rev[:9])

[JF, 2015/01/28 07:12:17]

Isn't the default short hash 7?

[Mark Seaborn, 2015/01/28 16:34:21]

Yes, JF is right.  Let's use 7.

[bradn, 2015/01/28 17:37:32]

Done.

[bradn, 2015/01/28 17:37:32]

Done.

   msg += '\n\nThis pulls in the following Native Client changes:\n\n'
   msg += msg_logs
   msg += '\nBUG=none\nTEST=browser_tests and nacl_integration\n'
@@ -253,23 +170,6 @@
                       sorted(set(authors)))
   print 'CC:', cc_list
 
-  # Copy revision numbers across from native_client/DEPS.
-  # We do this because 'From()' is not supported in Chrome's DEPS.
-  proc = subprocess.Popen(['svn', 'cat', '%s/DEPS@%s' % (NACL_SVN, svn_rev)],
-                          stdout=subprocess.PIPE)
-  nacl_deps = proc.communicate()[0]
-  assert proc.wait() == 0, proc.wait()
-  tools_rev = GetDepsField(nacl_deps, 'tools_rev')
-  # Chromium's DEPS file used to contain a single "nacl_tools_revision"
-  # field that we would update to match NaCl's "tools_rev".  Since Chromium
-  # switched to using Git revisions in DEPS, "nacl_tools_revision" has been
-  # replaced by multiple fields.  We don't currently support updating those
-  # fields automatically.  Instead, just assert that NaCl's "tools_rev"
-  # hasn't changed.
-  # TODO(mseaborn): Fix this automatic syncing.  Maybe this should wait

[Mark Seaborn, 2015/01/28 16:34:21]

I guess we can add back this automatic syncing if and when we need it.  These
revs rarely change.

[bradn, 2015/01/28 17:37:32]

Yeah I figure its problematic for the moment.

-  # until NaCl has also switched to using Git revisions in its DEPS file.
-  assert tools_rev in ('13077', '13800'), tools_rev
-
   WriteFile('DEPS', deps_data)
 
   if options.no_commit: