Include svn revision range in Blink roll descriptions.

Include svn revision range in Blink roll descriptions.

Prior to http://crrev.com/495493003, Blink rolls had the Blink svn revision range in their descriptions, which made it easy to determine which roll contained a given revision number by scanning the log of src/DEPS.  To do this with git hashes is much more difficult since they are not sequential and cannot be compared at a glance.

This change adds the svn range even when git_mode is True.  The resulting descriptions look like:

  Blink roll hash1:hash2 (svn 1234:1235)

[skobes, 2014-09-10 23:35:54]

skobes@chromium.org changed reviewers:
+ szager@chromium.org

[szager1, 2014-09-11 00:52:04]

lgtm, thanks

https://codereview.chromium.org/559243002/diff/1/scripts/tools/blink_roller/a...
File scripts/tools/blink_roller/auto_roll.py (right):

https://codereview.chromium.org/559243002/diff/1/scripts/tools/blink_roller/a...
scripts/tools/blink_roller/auto_roll.py:245: 'revision')
nit: indentation should be four characters in from start of previous line.

[skobes, 2014-09-11 01:32:42]

https://codereview.chromium.org/559243002/diff/1/scripts/tools/blink_roller/a...
File scripts/tools/blink_roller/auto_roll.py (right):

https://codereview.chromium.org/559243002/diff/1/scripts/tools/blink_roller/a...
scripts/tools/blink_roller/auto_roll.py:245: 'revision')
On 2014/09/11 00:52:04, szager1 wrote:
> nit: indentation should be four characters in from start of previous line.

Done.

[skobes, 2014-09-11 01:32:46]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2014-09-11 01:33:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/559243002/20001

[commit-bot: I haz the power, 2014-09-11 01:34:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-11 01:34:02]

Failed to apply patch for scripts/tools/blink_roller/auto_roll.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         scripts
  Created missing directory scripts.
  A         scripts/tools
  Created missing directory scripts/tools.
  A         scripts/tools/blink_roller
  Created missing directory scripts/tools/blink_roller.
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: scripts/tools/blink_roller/auto_roll.py
  |diff --git a/scripts/tools/blink_roller/auto_roll.py
b/scripts/tools/blink_roller/auto_roll.py
  |index
c1508eba697faafe111676c57694e1c01b0e65b3..745e3dc04ed04b1042ff793d8fae8309a80bdf92
100755
  |--- a/scripts/tools/blink_roller/auto_roll.py
  |+++ b/scripts/tools/blink_roller/auto_roll.py
  --------------------------
  No file to patch.  Skipping patch.
  6 out of 6 hunks ignored

Patch:       scripts/tools/blink_roller/auto_roll.py
Index: scripts/tools/blink_roller/auto_roll.py
diff --git a/scripts/tools/blink_roller/auto_roll.py
b/scripts/tools/blink_roller/auto_roll.py
index
c1508eba697faafe111676c57694e1c01b0e65b3..745e3dc04ed04b1042ff793d8fae8309a80bdf92
100755
--- a/scripts/tools/blink_roller/auto_roll.py
+++ b/scripts/tools/blink_roller/auto_roll.py
@@ -104,6 +104,10 @@ def _current_sheriff_emails():
   return _emails_from_url(CHROMIUM_SHERIFF_URL)
 
 
+def _do_git_fetch(git_dir):
+  subprocess2.check_call(['git', '--git-dir', git_dir, 'fetch'])
+
+
 PROJECT_CONFIGS = {
   'blink': {
     'extra_emails_fn': _current_gardener_emails,
@@ -148,7 +152,8 @@ class AutoRoller(object):
   CHROMIUM_SVN_DEPS_URL = 'http://src.chromium.org/chrome/trunk/src/DEPS'
   # 'webkit_revision': '149598',
   REVISION_REGEXP = (
-      r'^  [\'"]%s_revision[\'"]: [\'"](?P<revision>[0-9a-fA-F]{2,40})[\'"],')
+      r'^  [\'"]%s_revision[\'"]: [\'"](?P<revision>[0-9a-fA-F]{2,40})[\'"],'
+      r'( # from svn revision (?P<svn_revision>\d+))?')
 
   ROLL_BOT_INSTRUCTIONS = textwrap.dedent(
     '''This roll was created by the Blink AutoRollBot.
@@ -169,6 +174,7 @@ class AutoRoller(object):
     self._rietveld = rietveld.Rietveld(
       self.RIETVELD_URL, self._author, None)
     self._cached_last_roll_revision = None
+    self._cached_deps_contents = None
 
     project_config = PROJECT_CONFIGS.get(self._project, {
       'path_to_project': os.path.join('third_party', self._project),
@@ -236,36 +242,43 @@ class AutoRoller(object):
         SVN revision number.
     """
     if not self._cached_last_roll_revision:
-      subprocess2.check_call(['git', '--git-dir', self._chromium_git_dir,
-                              'fetch'])
-      git_show_cmd = ['git', '--git-dir', self._chromium_git_dir, 'show',
-                      'origin/master:DEPS']
-      deps_contents = subprocess2.check_output(git_show_cmd)
-      pattern = self.REVISION_REGEXP % self._project_alias
-      match = re.search(pattern, deps_contents, re.MULTILINE)
-      self._cached_last_roll_revision = match.group('revision')
+      self._cached_last_roll_revision = self._last_roll_revision_helper(
+          'revision')
     if self._git_mode:
       assert len(self._cached_last_roll_revision) == 40
     return self._cached_last_roll_revision
 
+  def _last_roll_revision_svn(self):
+    return self._last_roll_revision_helper('svn_revision')
+
+  def _last_roll_revision_helper(self, match_group):
+    if not self._cached_deps_contents:
+      git_show_cmd = ['git', '--git-dir', self._chromium_git_dir, 'show',
+                      'origin/master:DEPS']
+      self._cached_deps_contents = subprocess2.check_output(git_show_cmd)
+
+    pattern = self.REVISION_REGEXP % self._project_alias
+    match = re.search(pattern, self._cached_deps_contents, re.MULTILINE)
+    return match.group(match_group)
+
   def _current_revision(self):
-    subprocess2.check_call(['git', '--git-dir', self._project_git_dir,
-                            'fetch'])
     if self._git_mode:
       git_revparse_cmd = ['git', '--git-dir', self._project_git_dir,
                           'rev-parse', 'origin/master']
       return subprocess2.check_output(git_revparse_cmd).rstrip()
     else:
-      git_show_cmd = ['git', '--git-dir', self._project_git_dir, 'show', '-s',
-                      'origin/master']
-      git_log = subprocess2.check_output(git_show_cmd)
-      match = re.search('^\s*git-svn-id:.*@(?P<svn_revision>\d+)\ ',
-                        git_log, re.MULTILINE)
-      if match:
-        return match.group('svn_revision')
-      else:
-        raise AutoRollException(
-            'Could not determine the current SVN revision.')
+      return self._current_revision_svn()
+
+  def _current_revision_svn(self):
+    git_show_cmd = ['git', '--git-dir', self._project_git_dir, 'show', '-s',
+                    'origin/master']
+    git_log = subprocess2.check_output(git_show_cmd)
+    match = re.search('^\s*git-svn-id:.*@(?P<svn_revision>\d+)\ ',
+                      git_log, re.MULTILINE)
+    if match:
+      return match.group('svn_revision')
+    else:
+      return None
 
   def _emails_to_cc_on_rolls(self):
     return _filter_emails(self._get_extra_emails())
@@ -393,8 +406,23 @@ class AutoRoller(object):
             self._url_for_issue(issue_number)
         return 0
 
+    _do_git_fetch(self._chromium_git_dir)
+    _do_git_fetch(self._project_git_dir)
+
     last_roll_revision = self._last_roll_revision()
     new_roll_revision = self._current_revision()
+
+    if not new_roll_revision:
+      raise AutoRollException(
+          'Could not determine the current revision.')
+
+    if self._git_mode:
+      last_roll_revision_svn = self._last_roll_revision_svn()
+      new_roll_revision_svn = self._current_revision_svn()
+    else:
+      last_roll_revision_svn = None
+      new_roll_revision_svn = None
+
     self._compare_revisions(last_roll_revision, new_roll_revision)
 
     display_from_rev = (
@@ -408,6 +436,11 @@ class AutoRoller(object):
         'from_revision': display_from_rev,
         'to_revision': display_to_rev,
     }
+
+    if last_roll_revision_svn and new_roll_revision_svn:
+      commit_msg += ' (svn %s:%s)' % (
+                    last_roll_revision_svn, new_roll_revision_svn)
+
     revlink = self._get_revision_link(last_roll_revision, new_roll_revision)
     if revlink:
       commit_msg += '\n\n' + revlink

[skobes, 2014-09-11 04:34:57]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2014-09-11 04:35:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/559243002/40001

[commit-bot: I haz the power, 2014-09-11 04:35:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-11 04:35:42]

Failed to apply patch for scripts/tools/blink_roller/auto_roll.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         scripts
  Created missing directory scripts.
  A         scripts/tools
  Created missing directory scripts/tools.
  A         scripts/tools/blink_roller
  Created missing directory scripts/tools/blink_roller.
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: scripts/tools/blink_roller/auto_roll.py
  |diff --git a/scripts/tools/blink_roller/auto_roll.py
b/scripts/tools/blink_roller/auto_roll.py
  |index
c1508eba697faafe111676c57694e1c01b0e65b3..745e3dc04ed04b1042ff793d8fae8309a80bdf92
100755
  |--- a/scripts/tools/blink_roller/auto_roll.py
  |+++ b/scripts/tools/blink_roller/auto_roll.py
  --------------------------
  No file to patch.  Skipping patch.
  6 out of 6 hunks ignored

Patch:       scripts/tools/blink_roller/auto_roll.py
Index: scripts/tools/blink_roller/auto_roll.py
diff --git a/scripts/tools/blink_roller/auto_roll.py
b/scripts/tools/blink_roller/auto_roll.py
index
c1508eba697faafe111676c57694e1c01b0e65b3..745e3dc04ed04b1042ff793d8fae8309a80bdf92
100755
--- a/scripts/tools/blink_roller/auto_roll.py
+++ b/scripts/tools/blink_roller/auto_roll.py
@@ -104,6 +104,10 @@ def _current_sheriff_emails():
   return _emails_from_url(CHROMIUM_SHERIFF_URL)
 
 
+def _do_git_fetch(git_dir):
+  subprocess2.check_call(['git', '--git-dir', git_dir, 'fetch'])
+
+
 PROJECT_CONFIGS = {
   'blink': {
     'extra_emails_fn': _current_gardener_emails,
@@ -148,7 +152,8 @@ class AutoRoller(object):
   CHROMIUM_SVN_DEPS_URL = 'http://src.chromium.org/chrome/trunk/src/DEPS'
   # 'webkit_revision': '149598',
   REVISION_REGEXP = (
-      r'^  [\'"]%s_revision[\'"]: [\'"](?P<revision>[0-9a-fA-F]{2,40})[\'"],')
+      r'^  [\'"]%s_revision[\'"]: [\'"](?P<revision>[0-9a-fA-F]{2,40})[\'"],'
+      r'( # from svn revision (?P<svn_revision>\d+))?')
 
   ROLL_BOT_INSTRUCTIONS = textwrap.dedent(
     '''This roll was created by the Blink AutoRollBot.
@@ -169,6 +174,7 @@ class AutoRoller(object):
     self._rietveld = rietveld.Rietveld(
       self.RIETVELD_URL, self._author, None)
     self._cached_last_roll_revision = None
+    self._cached_deps_contents = None
 
     project_config = PROJECT_CONFIGS.get(self._project, {
       'path_to_project': os.path.join('third_party', self._project),
@@ -236,36 +242,43 @@ class AutoRoller(object):
         SVN revision number.
     """
     if not self._cached_last_roll_revision:
-      subprocess2.check_call(['git', '--git-dir', self._chromium_git_dir,
-                              'fetch'])
-      git_show_cmd = ['git', '--git-dir', self._chromium_git_dir, 'show',
-                      'origin/master:DEPS']
-      deps_contents = subprocess2.check_output(git_show_cmd)
-      pattern = self.REVISION_REGEXP % self._project_alias
-      match = re.search(pattern, deps_contents, re.MULTILINE)
-      self._cached_last_roll_revision = match.group('revision')
+      self._cached_last_roll_revision = self._last_roll_revision_helper(
+          'revision')
     if self._git_mode:
       assert len(self._cached_last_roll_revision) == 40
     return self._cached_last_roll_revision
 
+  def _last_roll_revision_svn(self):
+    return self._last_roll_revision_helper('svn_revision')
+
+  def _last_roll_revision_helper(self, match_group):
+    if not self._cached_deps_contents:
+      git_show_cmd = ['git', '--git-dir', self._chromium_git_dir, 'show',
+                      'origin/master:DEPS']
+      self._cached_deps_contents = subprocess2.check_output(git_show_cmd)
+
+    pattern = self.REVISION_REGEXP % self._project_alias
+    match = re.search(pattern, self._cached_deps_contents, re.MULTILINE)
+    return match.group(match_group)
+
   def _current_revision(self):
-    subprocess2.check_call(['git', '--git-dir', self._project_git_dir,
-                            'fetch'])
     if self._git_mode:
       git_revparse_cmd = ['git', '--git-dir', self._project_git_dir,
                           'rev-parse', 'origin/master']
       return subprocess2.check_output(git_revparse_cmd).rstrip()
     else:
-      git_show_cmd = ['git', '--git-dir', self._project_git_dir, 'show', '-s',
-                      'origin/master']
-      git_log = subprocess2.check_output(git_show_cmd)
-      match = re.search('^\s*git-svn-id:.*@(?P<svn_revision>\d+)\ ',
-                        git_log, re.MULTILINE)
-      if match:
-        return match.group('svn_revision')
-      else:
-        raise AutoRollException(
-            'Could not determine the current SVN revision.')
+      return self._current_revision_svn()
+
+  def _current_revision_svn(self):
+    git_show_cmd = ['git', '--git-dir', self._project_git_dir, 'show', '-s',
+                    'origin/master']
+    git_log = subprocess2.check_output(git_show_cmd)
+    match = re.search('^\s*git-svn-id:.*@(?P<svn_revision>\d+)\ ',
+                      git_log, re.MULTILINE)
+    if match:
+      return match.group('svn_revision')
+    else:
+      return None
 
   def _emails_to_cc_on_rolls(self):
     return _filter_emails(self._get_extra_emails())
@@ -393,8 +406,23 @@ class AutoRoller(object):
             self._url_for_issue(issue_number)
         return 0
 
+    _do_git_fetch(self._chromium_git_dir)
+    _do_git_fetch(self._project_git_dir)
+
     last_roll_revision = self._last_roll_revision()
     new_roll_revision = self._current_revision()
+
+    if not new_roll_revision:
+      raise AutoRollException(
+          'Could not determine the current revision.')
+
+    if self._git_mode:
+      last_roll_revision_svn = self._last_roll_revision_svn()
+      new_roll_revision_svn = self._current_revision_svn()
+    else:
+      last_roll_revision_svn = None
+      new_roll_revision_svn = None
+
     self._compare_revisions(last_roll_revision, new_roll_revision)
 
     display_from_rev = (
@@ -408,6 +436,11 @@ class AutoRoller(object):
         'from_revision': display_from_rev,
         'to_revision': display_to_rev,
     }
+
+    if last_roll_revision_svn and new_roll_revision_svn:
+      commit_msg += ' (svn %s:%s)' % (
+                    last_roll_revision_svn, new_roll_revision_svn)
+
     revlink = self._get_revision_link(last_roll_revision, new_roll_revision)
     if revlink:
       commit_msg += '\n\n' + revlink

[skobes, 2014-09-11 22:18:09]

This change has been committed in https://codereview.chromium.org/568593002/.