Remove base-* directories from gm expected/actual paths; just use platform names

Remove base-* directories from gm expected/actual paths; just use platform names
BUG=skia:1512
The buildbot master continues to set gm_image_subdir, but the slaves no longer
use that component when building GM paths (they just use builder_name instead).
But gm_image_subdir is still used for SKP storage paths under
gs://chromium-skia-gm/playback/gm-actual , for now...

Immediately before committing this CL, I will copy the actual-results.json files
from their old locations to their new locations (within the skia-autogen repo)
so that both old and new versions of rebaseline.py will be able to retrieve
actual results, like so:

svn cp base-android-galaxy-nexus/Test-Android-GalaxyNexus-SGX540-Arm7-Debug/base-android-galaxy-nexus Test-Android-GalaxyNexus-SGX540-Arm7-Debug
svn cp base-android-galaxy-nexus/Test-Android-GalaxyNexus-SGX540-Arm7-Release/base-android-galaxy-nexus Test-Android-GalaxyNexus-SGX540-Arm7-Release
svn cp base-android-nexus-10/Test-Android-Nexus10-MaliT604-Arm7-Debug/base-android-nexus-10 Test-Android-Nexus10-MaliT604-Arm7-Debug
...
svn cp base-shuttle-win7-intel-float/Test-Win7-ShuttleA-HD2000-x86-Debug/base-shuttle-win7-intel-float Test-Win7-ShuttleA-HD2000-x86-Debug
svn cp base-shuttle-win7-intel-float/Test-Win7-ShuttleA-HD2000-x86-Release/base-shuttle-win7-intel-float Test-Win7-ShuttleA-HD2000-x86-Release


Once we see this CL land successfully, I will delete the actual-results.json
files from their OLD location within skia-autogen, like so:

svn rm base-android-galaxy-nexus
svn rm base-android-nexus-10
...
svn rm base-shuttle-win7-intel-directwrite
svn rm base-shuttle-win7-intel-float

R=borenet@google.com, rmistry@google.com

Committed: https://code.google.com/p/skia/source/detail?r=10821

[epoger, 2013-08-13 19:59:37]

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py#new...
trunk/tools/rebaseline.py:45: 'Test-Android-GalaxyNexus-SGX540-Arm7-Debug',
Patchset 1 updates rebaseline.py to use builder names, rather than subdir names.

https://codereview.chromium.org/23120002/diff/1/trunk/tools/tests/rebaseline/...
File
trunk/tools/tests/rebaseline/output/using-json1-expectations/output-expected/command_line
(right):

https://codereview.chromium.org/23120002/diff/1/trunk/tools/tests/rebaseline/...
trunk/tools/tests/rebaseline/output/using-json1-expectations/output-expected/command_line:1:
python tools/rebaseline.py --expectations-root
tools/tests/rebaseline/output/using-json1-expectations/output-actual/gm-expectations
--actuals-base-url tools/tests/rebaseline/input/json1 --builders
Test-Android-GalaxyNexus-SGX540-Arm7-Debug Test-Win7-ShuttleA-HD2000-x86-Release
Patchset 1 also updates the rebaseline.py self-test results.

[epoger, 2013-08-13 20:04:06]

P.S. This CL is an alternate staging of the changes attempted in
https://codereview.chromium.org/23001003/ .

[epoger, 2013-08-13 20:12:33]

https://codereview.chromium.org/23120002/diff/7001/trunk/tools/verify_images_...
File trunk/tools/verify_images_for_gm_results.py (right):

https://codereview.chromium.org/23120002/diff/7001/trunk/tools/verify_images_...
trunk/tools/verify_images_for_gm_results.py:50:
'http://skia-autogen.googlecode.com/svn/gm-actual/Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug/actual-results.json',
Patchset 2 just updates comments within verify_images_for_gm_results.py

[epoger, 2013-08-13 20:33:57]

https://codereview.chromium.org/23120002/diff/11001/buildbot/slave/skia_slave...
File buildbot/slave/skia_slave_scripts/build_step.py (right):

https://codereview.chromium.org/23120002/diff/11001/buildbot/slave/skia_slave...
buildbot/slave/skia_slave_scripts/build_step.py:172: self._builder_name)
Patchset 3 updates the buildbot slave scripts to use builder_name instead of
gm_image_subdir to construct paths when reading/writing gm results.

(The skp code, like in skia_slave_scripts/playback_dirs.py , continues to use
gm_image_subdir to construct its paths.  That will change sometime, hopefully
soon.)

[epoger, 2013-08-13 20:47:56]

Patchset 4 rearranges the directory structure under trunk/expectations/gm . 
Here's exactly what I ran:

svn cp base-android-galaxy-nexus Test-Android-GalaxyNexus-SGX540-Arm7-Debug
svn cp base-android-galaxy-nexus Test-Android-GalaxyNexus-SGX540-Arm7-Release
svn cp base-android-nexus-10 Test-Android-Nexus10-MaliT604-Arm7-Debug
svn cp base-android-nexus-10 Test-Android-Nexus10-MaliT604-Arm7-Release
svn cp base-android-nexus-4 Test-Android-Nexus4-Adreno320-Arm7-Debug
svn cp base-android-nexus-4 Test-Android-Nexus4-Adreno320-Arm7-Release
svn cp base-android-nexus-7 Test-Android-Nexus7-Tegra3-Arm7-Debug
svn cp base-android-nexus-7 Test-Android-Nexus7-Tegra3-Arm7-Release
svn cp base-android-nexus-s Test-Android-NexusS-SGX540-Arm7-Debug
svn cp base-android-nexus-s Test-Android-NexusS-SGX540-Arm7-Release
svn cp base-android-razr-i Test-Android-RazrI-SGX540-x86-Debug
svn cp base-android-razr-i Test-Android-RazrI-SGX540-x86-Release
svn cp base-android-xoom Test-Android-Xoom-Tegra2-Arm7-Debug
svn cp base-android-xoom Test-Android-Xoom-Tegra2-Arm7-Release
svn cp base-macmini-10_8 Test-Mac10.8-MacMini4.1-GeForce320M-x86_64-Debug
svn cp base-macmini-10_8 Test-Mac10.8-MacMini4.1-GeForce320M-x86_64-Release
svn cp base-macmini-10_8 Test-Mac10.8-MacMini4.1-GeForce320M-x86-Debug
svn cp base-macmini-10_8 Test-Mac10.8-MacMini4.1-GeForce320M-x86-Release
svn cp base-macmini-lion-float Test-Mac10.7-MacMini4.1-GeForce320M-x86_64-Debug
svn cp base-macmini-lion-float
Test-Mac10.7-MacMini4.1-GeForce320M-x86_64-Release
svn cp base-macmini-lion-float Test-Mac10.7-MacMini4.1-GeForce320M-x86-Debug
svn cp base-macmini-lion-float Test-Mac10.7-MacMini4.1-GeForce320M-x86-Release
svn cp base-macmini Test-Mac10.6-MacMini4.1-GeForce320M-x86_64-Debug
svn cp base-macmini Test-Mac10.6-MacMini4.1-GeForce320M-x86_64-Release
svn cp base-macmini Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug
svn cp base-macmini Test-Mac10.6-MacMini4.1-GeForce320M-x86-Release
svn cp base-shuttle_ubuntu12_ati5770 Test-Ubuntu12-ShuttleA-ATI5770-x86_64-Debug
svn cp base-shuttle_ubuntu12_ati5770
Test-Ubuntu12-ShuttleA-ATI5770-x86_64-Release
svn cp base-shuttle_ubuntu12_ati5770 Test-Ubuntu12-ShuttleA-ATI5770-x86-Debug
svn cp base-shuttle_ubuntu12_ati5770 Test-Ubuntu12-ShuttleA-ATI5770-x86-Release
svn cp base-shuttle_ubuntu12_ati5770 Test-Ubuntu12-ShuttleA-NoGPU-x86_64-Debug
svn cp base-shuttle-win7-intel-angle Test-Win7-ShuttleA-HD2000-x86-Debug-ANGLE
svn cp base-shuttle-win7-intel-angle Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE
svn cp base-shuttle-win7-intel-directwrite
Test-Win7-ShuttleA-HD2000-x86-Debug-DirectWrite
svn cp base-shuttle-win7-intel-directwrite
Test-Win7-ShuttleA-HD2000-x86-Release-DirectWrite
svn cp base-shuttle-win7-intel-float Test-Win7-ShuttleA-HD2000-x86_64-Debug
svn cp base-shuttle-win7-intel-float Test-Win7-ShuttleA-HD2000-x86_64-Release
svn cp base-shuttle-win7-intel-float Test-Win7-ShuttleA-HD2000-x86-Debug
svn cp base-shuttle-win7-intel-float Test-Win7-ShuttleA-HD2000-x86-Release
svn rm base-android-galaxy-nexus
svn rm base-android-nexus-10
svn rm base-android-nexus-4
svn rm base-android-nexus-7
svn rm base-android-nexus-s
svn rm base-android-razr-i
svn rm base-android-xoom
svn rm base-macmini
svn rm base-macmini-10_8
svn rm base-macmini-lion-float
svn rm base-shuttle_ubuntu12_ati5770
svn rm base-shuttle-win7-intel-angle
svn rm base-shuttle-win7-intel-directwrite
svn rm base-shuttle-win7-intel-float

[epoger, 2013-08-13 20:57:54]

Ready for review at patchset 4.

This is a fairly large changelist... you might find it easiest to review one
patchset at a time (using my per-patchset comments).  Or not.

We can of course review in person Wednesday morning.

[rmistry, 2013-08-14 12:28:05]

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py#new...
trunk/tools/rebaseline.py:43: # TODO(epoger): Generate this list automatically.
This should be very easy to do (you could add it to this CL):

master_url = get from global_variables.json
# Below step will take a few seconds to run.
all_builders = list(json.load(urllib2.urlopen(master_url + 'json/builders')))
test_builders = filter(lambda x: 'Trybot' not in x and 'Test' in x,
all_builders)

Also, lets remove the '_LIST' from 'BUILDERS_LIST'.

https://codereview.chromium.org/23120002/diff/22001/trunk/tools/tests/run.sh
File trunk/tools/tests/run.sh (right):

https://codereview.chromium.org/23120002/diff/22001/trunk/tools/tests/run.sh#...
trunk/tools/tests/run.sh:152: # during a transition period (we need the svn rm
of those subdirs to take
For my information, what is the transition plan?

[borenet, 2013-08-14 12:57:48]

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py#new...
trunk/tools/rebaseline.py:43: # TODO(epoger): Generate this list automatically.
On 2013/08/14 12:28:05, rmistry wrote:
> This should be very easy to do (you could add it to this CL):
> 
> master_url = get from global_variables.json
> # Below step will take a few seconds to run.
> all_builders = list(json.load(urllib2.urlopen(master_url + 'json/builders')))
> test_builders = filter(lambda x: 'Trybot' not in x and 'Test' in x,
> all_builders)
> 
> Also, lets remove the '_LIST' from 'BUILDERS_LIST'.

+1.  We need to get rid of the handwritten list ASAP.

https://codereview.chromium.org/23120002/diff/11001/buildbot/slave/skia_slave...
File buildbot/slave/skia_slave_scripts/build_step.py (right):

https://codereview.chromium.org/23120002/diff/11001/buildbot/slave/skia_slave...
buildbot/slave/skia_slave_scripts/build_step.py:172: self._builder_name)
On 2013/08/13 20:33:57, epoger wrote:
> Patchset 3 updates the buildbot slave scripts to use builder_name instead of
> gm_image_subdir to construct paths when reading/writing gm results.
> 
> (The skp code, like in skia_slave_scripts/playback_dirs.py , continues to use
> gm_image_subdir to construct its paths.  That will change sometime, hopefully
> soon.)

I guess the SKP code causes a wrinkle, but I would prefer that we continue to
use self._gm_image_subdir instead of self._builder_name.

[epoger, 2013-08-16 20:16:25]

Thanks for the suggestion, PTAL

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/1/trunk/tools/rebaseline.py#new...
trunk/tools/rebaseline.py:43: # TODO(epoger): Generate this list automatically.
On 2013/08/14 12:57:48, borenet wrote:
> On 2013/08/14 12:28:05, rmistry wrote:
> > This should be very easy to do (you could add it to this CL):

Wow, that's pretty cool.  Thanks!  Done.  (You'll see that I added a helper file
to do this for both rebaseline.py and submit_try .)

https://codereview.chromium.org/23120002/diff/11001/buildbot/slave/skia_slave...
File buildbot/slave/skia_slave_scripts/build_step.py (right):

https://codereview.chromium.org/23120002/diff/11001/buildbot/slave/skia_slave...
buildbot/slave/skia_slave_scripts/build_step.py:172: self._builder_name)
On 2013/08/14 12:57:49, borenet wrote:
> I guess the SKP code causes a wrinkle, but I would prefer that we continue to
> use self._gm_image_subdir instead of self._builder_name.

So, once we have made both the GM and SKP expectations "per-builder", are you
proposing that we should store the builder name as both self._builder_name and
self._gm_image_subdir ?  That seems redundant to me.

I think it would be simpler, at that point, for every slave script which
reads/writes expectations to assume that builder_name is the proper key.

https://codereview.chromium.org/23120002/diff/22001/trunk/tools/tests/run.sh
File trunk/tools/tests/run.sh (right):

https://codereview.chromium.org/23120002/diff/22001/trunk/tools/tests/run.sh#...
trunk/tools/tests/run.sh:152: # during a transition period (we need the svn rm
of those subdirs to take
On 2013/08/14 12:28:05, rmistry wrote:
> For my information, what is the transition plan?

Once this CL lands, I can remove the "grep -v ^base-" immediately.  I just need
it for the tests to pass in my local checkout, where the pending "svn rm" of the
base-* subdirs hasn't taken effect yet.

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/buildbot_glob...
File trunk/tools/buildbot_globals.py (right):

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/buildbot_glob...
trunk/tools/buildbot_globals.py:1: #!/usr/bin/python
Added buildbot_globals.py

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/submit_try
File trunk/tools/submit_try (right):

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/submit_try#ne...
trunk/tools/submit_try:24: import buildbot_globals
submit_try now uses buildbot_globals.py

[epoger, 2013-08-20 15:04:38]

Ping: PTAL at patchset 5.

While you do that, I'm working on rebasing this CL to r10814.  It's a bit ugly.

[rmistry, 2013-08-20 15:13:03]

Some minor comments. LGTM pending Eric's LGTM.

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/buildbot_glob...
File trunk/tools/buildbot_globals.py (right):

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/buildbot_glob...
trunk/tools/buildbot_globals.py:23: return _global_vars[var_name]['value']
Optional: Catch KeyError and raise a better exception, maybe something like
NoSuchGlobalVariable.

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/rebaseline.py...
trunk/tools/rebaseline.py:44: TEST_BUILDERS = filter(lambda x: 'Trybot' not in x
and 'Test' in x, ALL_BUILDERS)
I think 43 & 44 are > 80

[borenet, 2013-08-20 15:24:01]

LGTM with a couple of comments.

https://codereview.chromium.org/23120002/diff/40001/buildbot/slave/skia_slave...
File buildbot/slave/skia_slave_scripts/postrender.py (right):

https://codereview.chromium.org/23120002/diff/40001/buildbot/slave/skia_slave...
buildbot/slave/skia_slave_scripts/postrender.py:16: self._builder_name),
I still think it would be better to set self._gm_image_subdir =
self._builder_name and then use that, rather than making the assumption that
self._gm_image_subdir is always a builder name.  I guess it doesn't really
matter though.

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/buildbot_glob...
File trunk/tools/buildbot_globals.py (right):

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/buildbot_glob...
trunk/tools/buildbot_globals.py:8: Provides read access to buildbot's
global_variables.json .
Thanks, this will be helpful!

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/rebaseline.py...
trunk/tools/rebaseline.py:44: TEST_BUILDERS = filter(lambda x: 'Trybot' not in x
and 'Test' in x, ALL_BUILDERS)
Nit: 80 chars

Eventually, it would be better to use the builder_name_schema rather than
depending on certain substrings being part of the builder name.

[epoger, 2013-08-20 15:46:19]

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/buildbot_glob...
File trunk/tools/buildbot_globals.py (right):

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/buildbot_glob...
trunk/tools/buildbot_globals.py:23: return _global_vars[var_name]['value']
On 2013/08/20 15:13:03, rmistry wrote:
> Optional: Catch KeyError and raise a better exception, maybe something like
> NoSuchGlobalVariable.

Done.

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/29001/trunk/tools/rebaseline.py...
trunk/tools/rebaseline.py:44: TEST_BUILDERS = filter(lambda x: 'Trybot' not in x
and 'Test' in x, ALL_BUILDERS)
On 2013/08/20 15:13:03, rmistry wrote:
> I think 43 & 44 are > 80

Done.

https://codereview.chromium.org/23120002/diff/40001/buildbot/slave/skia_slave...
File buildbot/slave/skia_slave_scripts/postrender.py (right):

https://codereview.chromium.org/23120002/diff/40001/buildbot/slave/skia_slave...
buildbot/slave/skia_slave_scripts/postrender.py:16: self._builder_name),
On 2013/08/20 15:24:01, borenet wrote:
> I still think it would be better to set self._gm_image_subdir =
> self._builder_name and then use that, rather than making the assumption that
> self._gm_image_subdir is always a builder name.  I guess it doesn't really
> matter though.

Right, as we discussed elsewhere in this CL.  As you mentioned there, we *need*
to specify builder_name here for now (because gm_image_subdir is still being
used as before for SKPs, for now).

So we can fight later about whether to change all of these back to
gm_image_subdir, once SKPs operate on the same directory structure as GMs again.

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/rebaseline.py...
trunk/tools/rebaseline.py:44: TEST_BUILDERS = filter(lambda x: 'Trybot' not in x
and 'Test' in x, ALL_BUILDERS)
On 2013/08/20 15:24:01, borenet wrote:
> Nit: 80 chars

Done.

> Eventually, it would be better to use the builder_name_schema rather than
> depending on certain substrings being part of the builder name.

Agreed.  To do that, I guess this code will need access to
buildbot/site_config/builder_name_schema.[json|py] , right?

[borenet, 2013-08-20 15:55:19]

https://codereview.chromium.org/23120002/diff/40001/buildbot/slave/skia_slave...
File buildbot/slave/skia_slave_scripts/postrender.py (right):

https://codereview.chromium.org/23120002/diff/40001/buildbot/slave/skia_slave...
buildbot/slave/skia_slave_scripts/postrender.py:16: self._builder_name),
On 2013/08/20 15:46:20, epoger wrote:
> On 2013/08/20 15:24:01, borenet wrote:
> > I still think it would be better to set self._gm_image_subdir =
> > self._builder_name and then use that, rather than making the assumption that
> > self._gm_image_subdir is always a builder name.  I guess it doesn't really
> > matter though.
> 
> Right, as we discussed elsewhere in this CL.  As you mentioned there, we
*need*
> to specify builder_name here for now (because gm_image_subdir is still being
> used as before for SKPs, for now).
> 
> So we can fight later about whether to change all of these back to
> gm_image_subdir, once SKPs operate on the same directory structure as GMs
again.

Got it.

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/rebaseline.py
File trunk/tools/rebaseline.py (right):

https://codereview.chromium.org/23120002/diff/40001/trunk/tools/rebaseline.py...
trunk/tools/rebaseline.py:44: TEST_BUILDERS = filter(lambda x: 'Trybot' not in x
and 'Test' in x, ALL_BUILDERS)
On 2013/08/20 15:46:20, epoger wrote:
> On 2013/08/20 15:24:01, borenet wrote:
> > Nit: 80 chars
> 
> Done.
> 
> > Eventually, it would be better to use the builder_name_schema rather than
> > depending on certain substrings being part of the builder name.
> 
> Agreed.  To do that, I guess this code will need access to
> buildbot/site_config/builder_name_schema.[json|py] , right?

Right.  I guess we'd need something that behaves just like buildbot_globals.py.

[epoger, 2013-08-20 16:02:40]

I committed the skia-autogen changes as
https://code.google.com/p/skia-autogen/source/detail?r=5788

Waiting for the tree to reopen, and then I will commit this CL!

[epoger, 2013-08-20 16:22:52]

Committed patchset #7 manually as r10821 (presubmit successful).