Use Git hash to post try job http request to bisect builders.

tools/bisect-perf-regression.py

 #!/usr/bin/env python
 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Performance Test Bisect Tool
 
 This script bisects a series of changelists using binary search. It starts at
 a bad revision where a performance metric has regressed, and asks for a last
 known-good revision. It will then binary search across this revision range by
@@ skipping 1298 matching lines @@
 
   def PostBuildRequestAndWait(self, revision, fetch_build, patch=None):
     """POSTs the build request job to the try server instance.
 
     A try job build request is posted to tryserver.chromium.perf master,
     and waits for the binaries to be produced and archived on cloud storage.
     Once the build is ready and stored onto cloud, build archive is downloaded
     into the output folder.
 
     Args:
       revision: A Git hash revision.

[qyearsley, 2014/08/27 16:41:55]

I think it would be less confusing if this were called git_hash or at least
git_revision.

[prasadv, 2014/08/27 17:15:39]

Done.

       fetch_build: Function to check and download build from cloud storage.
       patch: A DEPS patch (used while bisecting 3rd party repositories).
 
     Returns:
       Downloaded archive file path when requested build exists and download is
       successful, otherwise None.
     """
     # Get SVN revision for the given SHA.
     svn_revision = self.source_control.SVNFindRev(revision)
     if not svn_revision:
       raise RuntimeError(
           'Failed to determine SVN revision for %s' % revision)

[qyearsley, 2014/08/27 16:41:55]

We don't need to do this now, right?

[prasadv, 2014/08/27 17:15:39]

I felt SVN or Commit position are more readable rather than git hash,therefore I
kept this as is so that we can use this value in error msgs.
Let me know if this should still be removed.

[qyearsley, 2014/08/27 17:58:24]

Now, the function SVNFindRev might also find git commit position as well, right?
Saying that we're finding SVN revision (and naming that function SVNFindRev)
might be a bit misleading.

I think ideally, we should either:
  (1) refer to the revision in error messages by showing the first 8-10
characters of the git hash -- this is shorter and can also be used to find a
particular commit on all of our tools, e.g. http://crrev.com/459825d2 or "git
log -1 459825d2".
  (2) If you want to use an integer, then the function to fetch it should be
called FindSVNRevisionOrGitNumber.

I'm in favor of option 1 because it's simpler.

 
     def GetBuilderNameAndBuildTime(target_platform, target_arch='ia32'):
       """Gets builder bot name and build time in seconds based on platform."""
       # Bot names should match the one listed in tryserver.chromium's
       # master.cfg which produces builds for bisect.
       if bisect_utils.IsWindowsHost():
         if bisect_utils.Is64BitWindows() and target_arch == 'x64':
           return ('win_perf_bisect_builder', MAX_WIN_BUILD_TIME)
         return ('win_perf_bisect_builder', MAX_WIN_BUILD_TIME)
       if bisect_utils.IsLinuxHost():
         if target_platform == 'android':
           return ('android_perf_bisect_builder', MAX_LINUX_BUILD_TIME)
         return ('linux_perf_bisect_builder', MAX_LINUX_BUILD_TIME)
       if bisect_utils.IsMacHost():
         return ('mac_perf_bisect_builder', MAX_MAC_BUILD_TIME)
       raise NotImplementedError('Unsupported Platform "%s".' % sys.platform)
     if not fetch_build:
       return False
 
     bot_name, build_timeout = GetBuilderNameAndBuildTime(
        self.opts.target_platform, self.opts.target_arch)
     builder_host = self.opts.builder_host
     builder_port = self.opts.builder_port
     # Create a unique ID for each build request posted to try server builders.
     # This ID is added to "Reason" property of the build.
     build_request_id = GetSHA1HexDigest(
         '%s-%s-%s' % (svn_revision, patch, time.time()))

[qyearsley, 2014/08/27 16:41:55]

Perhaps the build ID request should use the git hash instead of svn revision
now?

 
     # Creates a try job description.
+    # Always use Git hash to post build request since Commit positions are
+    # not supported by builders to build.
     job_args = {
-        'revision': 'src@%s' % svn_revision,
+        'revision': 'src@%s' % revision,
         'bot': bot_name,
         'name': build_request_id,
     }
     # Update patch information if supplied.
     if patch:
       job_args['patch'] = patch
     # Posts job to build the revision on the server.
     if bisect_builder.PostTryJob(builder_host, builder_port, job_args):
       target_file, error_msg = _WaitUntilBuildIsReady(
           fetch_build, bot_name, builder_host, builder_port, build_request_id,
           build_timeout)
       if not target_file:
         print '%s [revision: %s]' % (error_msg, svn_revision)

[qyearsley, 2014/08/27 16:41:55]

Git hash can be printed here.

         return None
       return target_file
     print 'Failed to post build request for revision: [%s]' % svn_revision

[qyearsley, 2014/08/27 16:41:55]

And here.

     return None
 
   def IsDownloadable(self, depot):
     """Checks if build can be downloaded based on target platform and depot."""
     if (self.opts.target_platform in ['chromium', 'android'] and
         self.opts.gs_bucket):
       return (depot == 'chromium' or
               'chromium' in DEPOT_DEPS_NAME[depot]['from'] or
               'v8' in DEPOT_DEPS_NAME[depot]['from'])
     return False
@@ skipping 1973 matching lines @@
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Error: %s' % e.message
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 
 if __name__ == '__main__':
   sys.exit(main())