Refactor a copy of dartium/buildbot_annotated_steps.py

Refactor a copy of dartium/buildbot_annotated_steps.py

The buildbot script buildbot_annotated_steps.py has been copied from
dartium_tools in the dartium repository to tools/dartium in the dart repository.
We plan to switch over to using this version, and removing the original.

This CL refactors it, to make the uploading code available to
multivm_archive.py, which uploads the builds from multivm and dartium perf
builders, so they can be downloaded by the performance testers.

BUG=
R=ricow@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36211

[ricow1, 2014-05-14 17:28:18]

LGTM, thanks for doing this

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/buildbot_a...
File tools/dartium/buildbot_annotated_steps.py (right):

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/buildbot_a...
tools/dartium/buildbot_annotated_steps.py:24: SRC_PATH = os.path.dirname(
I think it makes sense to have the path at where we expect to be relative to src
in a comment, that makes reading this line much easier, something like:
# We are in src/dart/tools/dartium/

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/buildbot_a...
tools/dartium/buildbot_annotated_steps.py:31: DRT_FILTER = 'html'
I have been meaning to update that to the browser controller, I really should do
that

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/buildbot_a...
tools/dartium/buildbot_annotated_steps.py:87: info =
upload_steps.BuildInfo(version, dart_revision)
does that function belong somewhere else, the info is not only for uploading
right

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
File tools/dartium/multivm_archive.py (right):

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
tools/dartium/multivm_archive.py:7: """Archive builds from the multivm and
dartium perf builders
does this really get called for dartium perf builders?

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
tools/dartium/multivm_archive.py:22: SRC_PATH = os.path.dirname(
how about putting this in utils and using it here and in the
buildbot_annotated_steps instead of replicating it?

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
tools/dartium/multivm_archive.py:40: # Archive to the revision bucket unless
integration build
what does integration build mean here?

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/upload_ste...
File tools/dartium/upload_steps.py (right):

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/upload_ste...
tools/dartium/upload_steps.py:37: SRC_PATH = os.path.dirname(
as mentioned elsewhere, this should be in utils I think

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/upload_ste...
tools/dartium/upload_steps.py:100: if info.channel == 'be':
that is true, we upload bleeding edge to the old buckets, and dev/stable to the
new ones, I forgot about this when we discussed this earlier today

[ricow1, 2014-05-14 17:28:31]

+vsm

[Bill Hesse, 2014-05-15 10:46:37]

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/buildbot_a...
File tools/dartium/buildbot_annotated_steps.py (right):

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/buildbot_a...
tools/dartium/buildbot_annotated_steps.py:24: SRC_PATH = os.path.dirname(
On 2014/05/14 17:28:18, ricow1 wrote:
> I think it makes sense to have the path at where we expect to be relative to
src
> in a comment, that makes reading this line much easier, something like:
> # We are in src/dart/tools/dartium/

Done.  Moved to utils and comment added.

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/buildbot_a...
tools/dartium/buildbot_annotated_steps.py:87: info =
upload_steps.BuildInfo(version, dart_revision)
On 2014/05/14 17:28:18, ricow1 wrote:
> does that function belong somewhere else, the info is not only for uploading
> right

I did have the class here, and in multivm-upload, but it was duplicate code.  I
considered making a separate library (file) just for this class, but since it is
bound so tightly to (used heavily by) upload_steps, and upload_steps is imported
by both callers, I decided to put it there.

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
File tools/dartium/multivm_archive.py (right):

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
tools/dartium/multivm_archive.py:7: """Archive builds from the multivm and
dartium perf builders
On 2014/05/14 17:28:18, ricow1 wrote:
> does this really get called for dartium perf builders?

Yes, since the setup is identical for multivm and dartium, it will be easier to
maintain, and the download code for both will be identical.  If we know we have
a different builder for dartium, that uploads the binary we want, then we can
drop this later.

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
tools/dartium/multivm_archive.py:22: SRC_PATH = os.path.dirname(
On 2014/05/14 17:28:18, ricow1 wrote:
> how about putting this in utils and using it here and in the
> buildbot_annotated_steps instead of replicating it?

Done.  But:

The file utils.py contains only totally general python utility functions, not
specific to dart or dartium at all.  If we add a function to it that is specific
to the location of the file, we should at least make a new section at the bottom
of "local site-specific changes".
Also, this function is only valid inside a dart checkout that is inside a
dartium checkout.  If the function is called inside a regular dart checkout, it
will return the directory containing the dart checkout.

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/multivm_ar...
tools/dartium/multivm_archive.py:40: # Archive to the revision bucket unless
integration build
On 2014/05/14 17:28:18, ricow1 wrote:
> what does integration build mean here?

Stray comment - fixed.

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/upload_ste...
File tools/dartium/upload_steps.py (right):

https://codereview.chromium.org/270663007/diff/20001/tools/dartium/upload_ste...
tools/dartium/upload_steps.py:100: if info.channel == 'be':
On 2014/05/14 17:28:18, ricow1 wrote:
> that is true, we upload bleeding edge to the old buckets, and dev/stable to
the
> new ones, I forgot about this when we discussed this earlier today

We also upload bleeding-edge to the new buckets.

[Bill Hesse, 2014-05-15 10:49:34]

Committed patchset #5 manually as r36211 (presubmit successful).

[vsm, 2014-05-19 06:54:22]

lgtm