Apply patch for bot_update

Apply patch for bot_update

Does the following:
1. Download patch
2. Strip off the a/ prefixes in the patch, if existant
3. git apply --index -p0 --3way < [patch data]

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

[hinoka, 2014-02-18 22:02:48]

Apply patch, in like 5 lines

[agable, 2014-02-18 22:36:10]

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update.py
File scripts/slave/bot_update.py (right):

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update...
scripts/slave/bot_update.py:345: attempts_left = 3
This should use the same RETRIES mechanism as call().

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update...
scripts/slave/bot_update.py:363: patch_data = re.sub(r'^(---|\+\+\+) a/', r'\1
', patch_data, re.MULTILINE)
So we are supporting svn-style patches? I guess we have to, until flag day. That
sucks a bit though. This is going to need thorough testing... for a feature that
goes out the window on flag day.

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update...
scripts/slave/bot_update.py:366: git('apply', '--index', '-p0', '--3way',
stdin=patch_data)
No need to specify --index when --3way is passed.

[Ryan Tseng, 2014-02-19 22:05:40]

Dependent on: https://codereview.chromium.org/171763003/

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update.py
File scripts/slave/bot_update.py (right):

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update...
scripts/slave/bot_update.py:345: attempts_left = 3
On 2014/02/18 22:36:10, agable wrote:
> This should use the same RETRIES mechanism as call().

Done.

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update...
scripts/slave/bot_update.py:363: patch_data = re.sub(r'^(---|\+\+\+) a/', r'\1
', patch_data, re.MULTILINE)
On 2014/02/18 22:36:10, agable wrote:
> So we are supporting svn-style patches? I guess we have to, until flag day.
That
> sucks a bit though. This is going to need thorough testing... for a feature
that
> goes out the window on flag day.

Copypasta from here:
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/git_...

https://codereview.chromium.org/170843003/diff/40001/scripts/slave/bot_update...
scripts/slave/bot_update.py:366: git('apply', '--index', '-p0', '--3way',
stdin=patch_data)
On 2014/02/18 22:36:10, agable wrote:
> No need to specify --index when --3way is passed.

Also copypasta'ed, probably from a more simpler time
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/git_...

--index removed.

[agable, 2014-02-20 00:13:05]

https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
File scripts/slave/bot_update.py (right):

https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
scripts/slave/bot_update.py:308: def create_less_than_or_equal_regex(number):
Nooooooooooooo.

At the very least can we update this to use
7[0-7][0-9]{3}
style synatx?

But also I'm confused as to why this is actually necessary. Why don't the
revisions line up exactly so we can grep for an exact number?

https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
scripts/slave/bot_update.py:463: apply_issue_bin = ('apply_issue.bat' if
sys.platform.startswith('win')
It's really truly necessary to be using apply_issue? It does scary things like
commit the patch and stuff. Why was the switch away from 'git apply' necessary?

[Ryan Tseng, 2014-02-20 01:04:37]

Cleaned up the regex

A number of 251718 will yield the expression:
(251718|25171[0-7]|25170[0-9]|251[0-6][0-9]{2}|250[0-9]{3}|2[0-4][0-9]{4}|[0-1][0-9]{5}|[0-9]{1,5})

https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
File scripts/slave/bot_update.py (right):

https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
scripts/slave/bot_update.py:308: def create_less_than_or_equal_regex(number):
On 2014/02/20 00:13:06, agable wrote:
> Nooooooooooooo.
> 
> At the very least can we update this to use
> 7[0-7][0-9]{3}
> style synatx?
> 
> But also I'm confused as to why this is actually necessary. Why don't the
> revisions line up exactly so we can grep for an exact number?

Imagine if you set chrome's src repo to some pinned revision x, and revision x
has files in /chrome/src/base, but you reuse that revision for
/chrome/src/tools, but the last revision checked into the /chrome/src/tools repo
is y (where y = x - 6), without this ref resolution in /chrome/src/tools will
fail.

https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
scripts/slave/bot_update.py:463: apply_issue_bin = ('apply_issue.bat' if
sys.platform.startswith('win')
On 2014/02/20 00:13:06, agable wrote:
> It's really truly necessary to be using apply_issue? It does scary things like
> commit the patch and stuff. Why was the switch away from 'git apply'
necessary?

apply_issue has a crapton of logic to deal with every corner case that rietveld
throws at us, including svn patches, git patches, binary files that don't fit in
patches so need to be downloaded individually, etc.  The intent is to not
reimplement all of that here, and just match the current behavior.

[agable, 2014-02-20 04:03:37]

On 2014/02/20 01:04:37, Ryan T. wrote:
> Cleaned up the regex
> 
> A number of 251718 will yield the expression:
>
(251718|25171[0-7]|25170[0-9]|251[0-6][0-9]{2}|250[0-9]{3}|2[0-4][0-9]{4}|[0-1][0-9]{5}|[0-9]{1,5})
> 
>
https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
> File scripts/slave/bot_update.py (right):
> 
>
https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
> scripts/slave/bot_update.py:308: def create_less_than_or_equal_regex(number):
> On 2014/02/20 00:13:06, agable wrote:
> > Nooooooooooooo.
> > 
> > At the very least can we update this to use
> > 7[0-7][0-9]{3}
> > style synatx?
> > 
> > But also I'm confused as to why this is actually necessary. Why don't the
> > revisions line up exactly so we can grep for an exact number?
> 
> Imagine if you set chrome's src repo to some pinned revision x, and revision x
> has files in /chrome/src/base, but you reuse that revision for
> /chrome/src/tools, but the last revision checked into the /chrome/src/tools
repo
> is y (where y = x - 6), without this ref resolution in /chrome/src/tools will
> fail.
> 
>
https://codereview.chromium.org/170843003/diff/190001/scripts/slave/bot_updat...
> scripts/slave/bot_update.py:463: apply_issue_bin = ('apply_issue.bat' if
> sys.platform.startswith('win')
> On 2014/02/20 00:13:06, agable wrote:
> > It's really truly necessary to be using apply_issue? It does scary things
like
> > commit the patch and stuff. Why was the switch away from 'git apply'
> necessary?
> 
> apply_issue has a crapton of logic to deal with every corner case that
rietveld
> throws at us, including svn patches, git patches, binary files that don't fit
in
> patches so need to be downloaded individually, etc.  The intent is to not
> reimplement all of that here, and just match the current behavior.

LGTM

[Ryan Tseng, 2014-02-20 07:06:09]

The CQ bit was checked by hinoka@google.com

[commit-bot: I haz the power, 2014-02-20 07:06:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hinoka@google.com/170843003/250001

[commit-bot: I haz the power, 2014-02-20 07:08:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-20 07:08:13]

Presubmit check for 170843003-250001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found line ending with white spaces in:

***************
build/scripts/slave/bot_update.py:369
***************

** Presubmit ERRORS **
Pylint (427 files) (108.08s) failed
************* Module slave.bot_update
W0107:458,6:_download: Unnecessary pass statement
W0611: 13,0: Unused import re
[8119352 refs]


Presubmit checks took 109.5s to calculate.

[88405 refs]

[Ryan Tseng, 2014-02-21 02:30:25]

The CQ bit was checked by hinoka@google.com

[commit-bot: I haz the power, 2014-02-21 02:30:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hinoka@google.com/170843003/320001

[commit-bot: I haz the power, 2014-02-21 02:31:40]

Change committed as 252469

[Ryan Tseng, 2014-02-21 02:49:31]

A revert of this CL has been created in
https://codereview.chromium.org/169173009/ by hinoka@google.com.

The reason for reverting is: Arg changes breaks existing bots..