display "latest GM failures" link in its own line under build step

master/skia_master_scripts/factory.py

Index: master/skia_master_scripts/factory.py
diff --git a/master/skia_master_scripts/factory.py b/master/skia_master_scripts/factory.py
index 97ae2a2c959874adc771903b407aff3509251c69..e68e8a824e65b777b99383f7167b784cc811d52f 100644
--- a/master/skia_master_scripts/factory.py
+++ b/master/skia_master_scripts/factory.py
@@ -248,7 +248,8 @@ class SkiaFactory(BuildFactory):
   def AddSlaveScript(self, script, description, args=None, timeout=None,
                      halt_on_failure=False,
                      is_upload_render_step=False, is_upload_bench_step=False,
-                     is_rebaseline_step=False, get_props_from_stdout=None,
+                     is_rebaseline_step=False,
+                     get_links_from_stdout=None, get_props_from_stdout=None,
                      workdir=None, do_step_if=None, always_run=False,
                      flunk_on_failure=True, exception_on_failure=False):
     """ Add a BuildStep consisting of a python script.
@@ -267,6 +268,10 @@ class SkiaFactory(BuildFactory):
         self._do_upload_bench_results is True
     is_rebaseline_step: boolean indicating whether this step is required for
         rebaseline-only builds.
+    get_links_from_stdout: optional dictionary. Keys are strings indicating
+        link text to set based on the output of this step. Values are
+        strings containing regular expressions for parsing the linked URL from
+        the output of the step.
     get_props_from_stdout: optional dictionary. Keys are strings indicating
         build properties to set based on the output of this step. Values are
         strings containing regular expressions for parsing the property from
@@ -305,6 +310,7 @@ class SkiaFactory(BuildFactory):
         halt_on_failure=halt_on_failure,
         is_upload_step=is_upload_render_step or is_upload_bench_step,
         is_rebaseline_step=is_rebaseline_step,
+        get_links_from_stdout=get_links_from_stdout,
         get_props_from_stdout=get_props_from_stdout,
         workdir=workdir,
         do_step_if=do_step_if,
@@ -532,8 +538,8 @@ class SkiaFactory(BuildFactory):
     """Compare the actually-generated GM images to the checked-in baselines."""
     self.AddSlaveScript(script='compare_gms.py',
                         description='CompareGMs',
-                        get_props_from_stdout={
-                            'latest_gm_failures_url':
+                        get_links_from_stdout={
+                            'Latest GM failures on this builder':

[epoger, 2014/06/05 22:15:36]

For more flexibility, we could even allow the client-side script to set up the
number of links, and the text label for each one...

We could define a meta-tag like this:
BUILDBOT_DISPLAY_LINK('link name','http://www.cnn.com')

And every time it appears in the stdout, we would add a link to the buildbot
page.

I guess we would still want build steps to have to "opt in" to get it; otherwise
we would have to run regex matches over the stdout of every build step, just in
case...

[borenet, 2014/06/06 12:10:34]

This makes me wish we had better performance measurements for the master.  My
inclination is to request that we always scan for links and properties and then
add functions to skia_slave_scripts/utils/misc.py:

SetBuildProperty(k, v)
SetLink(name, url)

Each of these would print to stdout in a predictable format like you suggested
above, and then we'd no longer need to do a master restart to set build
properties from a slave-side script.

[epoger, 2014/06/09 13:33:28]

I think those functions sound like a good idea.  Although I could imagine
wanting our native tools (gm and its ilk) to be able to trigger the links also,
and those tools wouldn't be able to call into skia_slave_scripts/utils/misc.py .
 Perhaps the format for the special notations could be shared between buildbot
Python code and C++ code within the skia repo.

At any rate, like you say, it would be awfully helpful to have better
performance measurements for the master before making such a change.  We know
that there are some steps that have ridiculously long stdio logs; I'm afraid
that parsing all of the logs by default could be problematic.

Also... there may be some way we could log these notations into a different
stream, or make them appear at the top of the stdio log and thus only parse the
first part of the log.

                             '%s([^\n]*)\n' % skia_vars.GetGlobalVariable(
                                 'latest_gm_failures_preamble')},
                         is_rebaseline_step=True)