Bot update cleanup

Bot update cleanup

* Remove activation check
* Remove messages
* Remove deps2git
* Remove build_internal pointer by:
** Moving chrome svn url into bot_update.py (This isn't secret anyways.)
** Move patch.exe into depot_tools (No reason this should've been internal...)
** Default everything to active, so no need for checks anyways.

BUG=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298809

[hinoka, 2016-02-10 21:59:56]

hinoka@chromium.org changed reviewers:
+ iannucci@chromium.org

[iannucci, 2016-02-11 00:56:21]

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/api.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/api.py:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
can't comment on patch.exe directly, but can we make it a resource of this
recipe_module instead of a top-level file?

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/api.py:66: force=True, patch_root=None,
no_shallow=False,
this CL: docstring indicate that this option is ignored

followup CL: remove this option

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/example.expected/clobber.json (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/example.expected/clobber.json:27: "@@@STEP_TEXT@Some
step text@@@",
Is this change expected?

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/example.expected/no_shallow.json (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/example.expected/no_shallow.json:27:
"@@@STEP_TEXT@Some step text@@@",
same

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/example.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/example.py:120: slavename='somehost',
nothing should need these {master,builder,slave}name properties any more, right?

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/resources/bot_update.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:79: RECOGNIZED_PATHS = {
followup CL: what will it take to get rid of this?

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:184: bot_update.py is:"""
is: Active?

Should this line be removed?

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:201: PATCH_TOOL =
path.join(DEPOT_TOOLS_DIR, 'patch.EXE')
let's definitely change this to be a module resource, in which case this will be
join(SCRIPT_DIR, "patch.exe")

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:1172:
parse.add_option('--builder_name', help='DEPRECATED: Name of the builder, '
you can remove all deprecated options now: the only thing that uses this command
line interface is the recipe module, and this script is now pinned with the
recipe module itself.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:1240:
emit_json(options.output_json, did_run=True)
yeah let's definitely get rid of did_run if we can. If it's a huge change, let's
do it next.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:1376: 'recipe': 'Forced on by
default',
remove from here up

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/test_api.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/test_api.py:24: 'did_run': True,
can we remove did_run entirely? Or can this still be False at runtime somehow?

Maybe followup CL

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/test_api.py:68: def gen_revision(project, git_mode):
is this ever False?

[iannucci, 2016-02-11 01:00:56]

iannucci@chromium.org changed reviewers:
+ eseidel@chromium.org, estaab@chromium.org, martiniss@chromium.org

[iannucci, 2016-02-11 01:00:56]

Oh, also +interested parties

[iannucci, 2016-02-11 01:01:47]

(and this is a tremendous improvement :) Let's make sure to actually follow
through and land this (incl build/build_internal rolls))

[martiniss, 2016-02-11 10:31:48]

https://codereview.chromium.org/1686273002/diff/60001/recipe_modules/bot_upda...
File recipe_modules/bot_update/resources/bot_update.py (right):

https://codereview.chromium.org/1686273002/diff/60001/recipe_modules/bot_upda...
recipe_modules/bot_update/resources/bot_update.py:1240:
emit_json(options.output_json, did_run=True)
On 2016/02/11 at 00:56:20, iannucci wrote:
> yeah let's definitely get rid of did_run if we can. If it's a huge change,
let's do it next.

I think a fair amount of recipes still check this, so we should probably do this
after.

https://codereview.chromium.org/1686273002/diff/60001/tests/bot_update_covera...
File tests/bot_update_coverage_test.py (right):

https://codereview.chromium.org/1686273002/diff/60001/tests/bot_update_covera...
tests/bot_update_coverage_test.py:18: sys.platform = 'linux2'  # For
consistency, ya know?
wait what? is this actually needed?

https://codereview.chromium.org/1686273002/diff/60001/tests/bot_update_covera...
tests/bot_update_coverage_test.py:229: 'url':
'https://abc.googlesource.com/foo.git'
alphabet! :D

[hinoka, 2016-02-11 20:10:06]

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/api.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/api.py:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
On 2016/02/11 00:56:20, iannucci wrote:
> can't comment on patch.exe directly, but can we make it a resource of this
> recipe_module instead of a top-level file?

Done.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/api.py:66: force=True, patch_root=None,
no_shallow=False,
On 2016/02/11 00:56:20, iannucci wrote:
> this CL: docstring indicate that this option is ignored
> 
> followup CL: remove this option

Acknowledged.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/example.expected/clobber.json (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/example.expected/clobber.json:27: "@@@STEP_TEXT@Some
step text@@@",
On 2016/02/11 00:56:20, iannucci wrote:
> Is this change expected?

No.. but I guess the expectations were off before :/

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/example.expected/no_shallow.json (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/example.expected/no_shallow.json:27:
"@@@STEP_TEXT@Some step text@@@",
On 2016/02/11 00:56:20, iannucci wrote:
> same

#confusion

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/example.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/example.py:120: slavename='somehost',
On 2016/02/11 00:56:20, iannucci wrote:
> nothing should need these {master,builder,slave}name properties any more,
right?

Done.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/resources/bot_update.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:79: RECOGNIZED_PATHS = {
On 2016/02/11 00:56:21, iannucci wrote:
> followup CL: what will it take to get rid of this?

Remove all references to SVN in buildbot master configs.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:184: bot_update.py is:"""
On 2016/02/11 00:56:21, iannucci wrote:
> is: Active?
> 
> Should this line be removed?

Done.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:201: PATCH_TOOL =
path.join(DEPOT_TOOLS_DIR, 'patch.EXE')
On 2016/02/11 00:56:20, iannucci wrote:
> let's definitely change this to be a module resource, in which case this will
be
> join(SCRIPT_DIR, "patch.exe")

Done.

You mean THIS_DIR, right?

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:1172:
parse.add_option('--builder_name', help='DEPRECATED: Name of the builder, '
On 2016/02/11 00:56:21, iannucci wrote:
> you can remove all deprecated options now: the only thing that uses this
command
> line interface is the recipe module, and this script is now pinned with the
> recipe module itself.

Yesssssss. Ok

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:1240:
emit_json(options.output_json, did_run=True)
On 2016/02/11 10:31:47, martiniss wrote:
> On 2016/02/11 at 00:56:20, iannucci wrote:
> > yeah let's definitely get rid of did_run if we can. If it's a huge change,
> let's do it next.
> 
> I think a fair amount of recipes still check this, so we should probably do
this
> after.

Acknowledged.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/resources/bot_update.py:1376: 'recipe': 'Forced on by
default',
On 2016/02/11 00:56:21, iannucci wrote:
> remove from here up

Done.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
File recipe_modules/bot_update/test_api.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/test_api.py:24: 'did_run': True,
On 2016/02/11 00:56:21, iannucci wrote:
> can we remove did_run entirely? Or can this still be False at runtime somehow?
> 
> Maybe followup CL

Acknowledged.

I'm not sure didn't check but I expect we can remove it.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/recipe_modules/b...
recipe_modules/bot_update/test_api.py:68: def gen_revision(project, git_mode):
On 2016/02/11 00:56:21, iannucci wrote:
> is this ever False?

Ya, for commit positions.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/tests/bot_update...
File tests/bot_update_coverage_test.py (right):

https://chromiumcodereview.appspot.com/1686273002/diff/60001/tests/bot_update...
tests/bot_update_coverage_test.py:18: sys.platform = 'linux2'  # For
consistency, ya know?
On 2016/02/11 10:31:47, martiniss wrote:
> wait what? is this actually needed?

For determining gclient.bat / gclient and patch/patch.exe location.

https://chromiumcodereview.appspot.com/1686273002/diff/60001/tests/bot_update...
tests/bot_update_coverage_test.py:229: 'url':
'https://abc.googlesource.com/foo.git'
On 2016/02/11 10:31:48, martiniss wrote:
> alphabet! :D

kekeke

[iannucci, 2016-02-11 23:02:50]

lgtm

[Ryan Tseng, 2016-02-12 18:02:06]

hinoka@google.com changed reviewers:
+ hinoka@google.com

[Ryan Tseng, 2016-02-12 18:02:07]

I'll land this next monday

[Ryan Tseng, 2016-02-12 18:02:26]

I meant, Tuesday

[Ryan Tseng, 2016-02-16 18:09:00]

The CQ bit was checked by hinoka@google.com

[commit-bot: I haz the power, 2016-02-16 18:09:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1686273002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1686273002/80001

[commit-bot: I haz the power, 2016-02-16 18:11:19]

Description was changed from

==========
Bot update cleanup

* Remove activation check
* Remove messages
* Remove deps2git
* Remove build_internal pointer by:
** Moving chrome svn url into bot_update.py (This isn't secret anyways.)
** Move patch.exe into depot_tools (No reason this should've been internal...)
** Default everything to active, so no need for checks anyways.

BUG=
==========

to

==========
Bot update cleanup

* Remove activation check
* Remove messages
* Remove deps2git
* Remove build_internal pointer by:
** Moving chrome svn url into bot_update.py (This isn't secret anyways.)
** Move patch.exe into depot_tools (No reason this should've been internal...)
** Default everything to active, so no need for checks anyways.

BUG=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298809
==========

[commit-bot: I haz the power, 2016-02-16 18:11:20]

Committed patchset #5 (id:80001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298809

[Ryan Tseng, 2016-02-17 19:39:21]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1706893003/ by hinoka@google.com.

The reason for reverting is: Separating change into smaller changes..