[Findit] Fetch DEPS from buildspec/ instead of trunk for chrome official builds.

appengine/findit/common/chromium_deps.py

Index: appengine/findit/common/chromium_deps.py
diff --git a/appengine/findit/common/chromium_deps.py b/appengine/findit/common/chromium_deps.py
index 970b2df6dce69523948993dca745357711c35c59..19ee6345128ab1835d515d9baec0bf8151996a99 100644
--- a/appengine/findit/common/chromium_deps.py
+++ b/appengine/findit/common/chromium_deps.py
@@ -2,18 +2,35 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-from common import git_repository
-from common import http_client_appengine
+import base64
+import re
+
+from common import auth_util
 from common import dependency
 from common import deps_parser
-
+from common import git_repository
+from common import http_client_appengine
 
 _CHROMIUM_ROOT_DIR = 'src/'
 _CHROMIUM_REPO_MASTER = 'https://chromium.googlesource.com/chromium/src.git'
 
+_CHROME_VERSION_PATTERN = re.compile(r'^\d+\.\d+\.\d+\.\d+$')
+
+_BUILDSPEC_REPO = ('https://chrome-internal.googlesource.com/chrome/tools/'
+                   'buildspec.git/')
+
+
+def IsChromeVersion(revision):
+  """Determines if a revision is a chrome version."""
+  if _CHROME_VERSION_PATTERN.match(revision):
+    return True
+
+  return False
+
 
 class DEPSDownloader(deps_parser.DEPSLoader):
   """Downloads DEPS from remote Git repo."""
+
   def __init__(self, check_deps_git_first=False):
     """
     Args:
@@ -46,13 +63,26 @@ class DEPSDownloader(deps_parser.DEPSLoader):
     return content
 
 
+class DEPSDownloaderForChromeVersion(deps_parser.DEPSLoader):

[stgao, 2016/05/05 17:32:28]

Can we just reuse DEPSDownloader above with some slight change to it?

The code #69-#76 seems duplicate of the implementation in GitRepository.

[Sharu Jiang, 2016/05/05 20:26:45]

Done.

+
+  def Load(self, _, version, deps_file):

[Martin Barbella, 2016/05/05 16:37:37]

Is there any way to refactor DEPSLoader such that the unused argument isn't
needed? For example adding the URL in the DEPSDownloader constructor (not sure
if this would be clean since I didn't check the implementation, just throwing
the idea out there).

[stgao, 2016/05/05 17:32:28]

This breaks the API expected by deps_parser.

[Sharu Jiang, 2016/05/05 20:26:45]

Done.

+    http_client = http_client_appengine.HttpClientAppengine()
+    url = '%s/+/master/releases/%s/%s' % (_BUILDSPEC_REPO, version, deps_file)
+    headers = {'Authorization': 'Bearer ' + auth_util.GetAuthToken()}
+
+    status_code, content = http_client.Get(url, headers=headers)
+    if status_code != 200:
+      return None
+    return base64.b64decode(content)
+

[Martin Barbella, 2016/05/05 16:37:37]

Additional blank line here.

[Sharu Jiang, 2016/05/05 20:26:45]

Done.

 def GetChromeDependency(revision, platform, check_deps_git_first=False):
   """Returns all dependencies of Chrome as a dict for the given revision and OS.
 
   Args:
-    revision (str): The revision of a Chrome build.
+    revision (str): The revision of a Chrome build, it can be a githash or a
+      chrome version for a official build.
     platform (str): The target platform of the Chrome build, should be one of
-        'win', 'ios', 'mac', 'unix', 'android', or 'all'.
+      'win', 'ios', 'mac', 'unix', 'android', or 'all'.
     check_deps_git_first (bool): If True, use .DEPS.git instead of DEPS.
 
   Returns:
@@ -61,8 +91,21 @@ def GetChromeDependency(revision, platform, check_deps_git_first=False):
   root_dep = dependency.Dependency(

[Martin Barbella, 2016/05/05 16:37:37]

Would it be possible or cleaner to use the converted revision when creating
this? It feels a bit hacky to replace revision after the fact.

[Sharu Jiang, 2016/05/05 20:26:45]

I have to keep it as a version until we call
deps_parser.UpdateDependencyTree(root_dep, [platform], deps_loader), since we
need the version to pull DEPS from buildspec/ within it.

       _CHROMIUM_ROOT_DIR, _CHROMIUM_REPO_MASTER, revision, 'DEPS')

[stgao, 2016/05/05 17:32:28]

When reusing DEPSDownloader above, this might need some change.

[Sharu Jiang, 2016/05/05 20:26:45]

It seems no change is needed.

 
-  deps_parser.UpdateDependencyTree(
-      root_dep, [platform], DEPSDownloader(check_deps_git_first))
+  root_is_chrome_version = IsChromeVersion(root_dep.revision)
+
+  if root_is_chrome_version:
+    deps_loader = DEPSDownloaderForChromeVersion()
+  else:
+    # For official chrome release build, fetch the DEPS generated by buildspec.
+    deps_loader = DEPSDownloader(check_deps_git_first)
+
+  deps_parser.UpdateDependencyTree(root_dep, [platform], deps_loader)
+
+  if root_is_chrome_version:
+    http_client = http_client_appengine.HttpClientAppengine()
+    repo = git_repository.GitRepository(_CHROMIUM_REPO_MASTER, http_client)
+    # Convert chrome version revision to githash revision to be consistent.

[stgao, 2016/05/05 17:32:28]

"version revision" seems a bit confusing.

[Sharu Jiang, 2016/05/05 20:26:45]

Done.

+    root_dep.revision = repo.GetRevisionForChromeVersion(root_dep.revision)

[stgao, 2016/05/05 17:32:28]

If we convert the version to git-hash here, will that have side-effect on
pulling changelogs in the regression range?

[Sharu Jiang, 2016/05/05 20:26:45]

No, after this, pulling changelogs would be exactly the same.

[stgao, 2016/05/05 21:02:46]

What if there are patch cherrypick in the Chrome branches?

[Sharu Jiang, 2016/05/05 23:03:45]

Hm... I see, in this case, we shouldn't do the conversion at all. We can treat
the chrome version the same as revision, since for pulling changelogs, they are
the same.

 
   dependencies = {}
 
@@ -78,14 +121,21 @@ def GetChromeDependency(revision, platform, check_deps_git_first=False):
 
 
 def GetChromiumDEPSRolls(old_cr_revision, new_cr_revision, platform,
-                         check_deps_git_first=False):
+                         check_deps_git_first=False, skip_chromium_roll=True):
   """Returns a list of dependency rolls between the given Chromium revisions.
 
   Args:
-    old_cr_revision (str): The Git commit hash for the old Chromium revision.
-    new_cr_revision (str): The Git commit hash for the new Chromium revision.
+    old_cr_revision (str): The old Chromium revision, it can be a githash or a
+      chrome version for a official build.
+    new_cr_revision (str): The new Chromium revision, it can be a githash or a
+      chrome version for a official build.
     platform (str): The target OS platform of the Chrome or test binary.
     check_deps_git_first (bool): If True, use .DEPS.git instead of DEPS.
+    skip_chromium_roll (bool): If False, chromium roll will be contained in
+      the return.
+
+  Returns:
+    A list of DependencyRoll objects in the revision range.
   """
   old_deps = GetChromeDependency(
       old_cr_revision, platform, check_deps_git_first)
@@ -95,7 +145,7 @@ def GetChromiumDEPSRolls(old_cr_revision, new_cr_revision, platform,
   rolls = []
 
   for path, new_dep in new_deps.iteritems():
-    if path == _CHROMIUM_ROOT_DIR:  # Skip the root dependency -- chromium.
+    if skip_chromium_roll and path == _CHROMIUM_ROOT_DIR:
       continue
 
     old_revision = None
@@ -117,22 +167,20 @@ def GetChromiumDEPSRolls(old_cr_revision, new_cr_revision, platform,
 
 
 def GetDEPSRollsDict(old_cr_revision, new_cr_revision, platform):
-  """Gets dep_path to DependencyRoll dictionary for deps in
-  (old_cr_revision, new_cr_revision].
+  """Gets dep_path to DependencyRoll dictionary for deps between revisions.
 
   Args:
-    old_cr_revision (str): The Git commit hash for the old Chromium revision.
-    new_cr_revision (str): The Git commit hash for the new Chromium revision.
+    old_cr_revision (str): The old Chromium revision, it can be a githash or a
+      chrome version for a official build.
+    new_cr_revision (str): The new Chromium revision, it can be a githash or a
+      chrome version for a official build.
     platform (str): The target OS platform of the Chrome or test binary.
 
   Returns:
-    A dict, mapping dep path to DependencyRoll.
+    A dict, mapping dep path to its DependencyRoll.
   """
-  deps_rolls = GetChromiumDEPSRolls(old_cr_revision, new_cr_revision, platform)
-  # Add chromium as dependency roll.
-  deps_rolls.append(dependency.DependencyRoll(
-      _CHROMIUM_ROOT_DIR, _CHROMIUM_REPO_MASTER,
-      old_cr_revision, new_cr_revision))
+  deps_rolls = GetChromiumDEPSRolls(old_cr_revision, new_cr_revision, platform,
+                                    skip_chromium_roll=False)
 
   deps_rolls_dict = {}
   for dep_roll in deps_rolls: