Allow passing in arbitrary root@revision to bot_update

Allow passing in arbitrary root@revision to bot_update

This is so that we can pin any DEPS to any arbitrary revision.
<root> can be specified as either a folder name containing a git repo, or
a git/svn DEPS url (But for SVN, only ones that bot_update explicitly recognizes).

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

[Ryan Tseng, 2014-05-07 05:00:58]

ptal!  This is needed for blink.

Tested locally with:
--revision src@268651
--revision src@268651 --revision src/third_party/WebKit@HEAD
--revision src@268651 --revision src/third_party/WebKit@173462
--revision src@HEAD --revision src/third_party/WebKit@173462
--revision src@HEAD --revision src/third_party/WebKit@HEAD

[iannucci, 2014-05-08 23:50:05]

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

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:481: searches backwards from the current commit.
s/current commit/HEAD

though you could specify that to be something else if you want

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:650: # TODO(hinoka): Prefer origin/git-svn if it
exists.
You should just pass origin/master to `git log` instead of doing an extra two
checkouts, right?

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:723: time.sleep(5)
ick :/

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:809: git_revision = git('log', '-1', '--format=%H',
cwd=dir_name).strip()
er... `git rev-parse HEAD` ?

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:894: We will always return a dict with {root:
something}.  By default if root
is 'root' like 'src' or 'src/tools/gyp' ?

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:913: print 'WARNING: SVN path %s not recognized,
ignoring' % current_root
in general I would recommend using the logging module for this sort of thing
instead of print.

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:929: print ('WARNING: %s is not recognized as a
valid revision specification,'
I would use %r (everywhere) to get automatic quoting (in case it has spaces or
something).

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:957: 'svn url. To specify Tip of Tree, set rev to
HEAD.')
Note that gclient would accept `--revision src@<rev>` as well, so we should
filter the 'root' portion based on the .gclient spec entries.

Where is the code that does that (I see in your examples you tried that already
:))

[hinoka, 2014-05-10 00:54:07]

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

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:481: searches backwards from the current commit.
On 2014/05/08 23:50:06, iannucci wrote:
> s/current commit/HEAD
> 
> though you could specify that to be something else if you want

Redid this to always start from origin/master

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:650: # TODO(hinoka): Prefer origin/git-svn if it
exists.
On 2014/05/08 23:50:06, iannucci wrote:
> You should just pass origin/master to `git log` instead of doing an extra two
> checkouts, right?

Done.

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:723: time.sleep(5)
On 2014/05/08 23:50:06, iannucci wrote:
> ick :/

As long as we are mirroring, we need a sleep somewhere :/

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:809: git_revision = git('log', '-1', '--format=%H',
cwd=dir_name).strip()
On 2014/05/08 23:50:06, iannucci wrote:
> er... `git rev-parse HEAD` ?

Done.

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:894: We will always return a dict with {root:
something}.  By default if root
On 2014/05/08 23:50:06, iannucci wrote:
> is 'root' like 'src' or 'src/tools/gyp' ?

both!
I'm expecting things like src and src/third_party/WebKit getting passed in.

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:913: print 'WARNING: SVN path %s not recognized,
ignoring' % current_root
On 2014/05/08 23:50:06, iannucci wrote:
> in general I would recommend using the logging module for this sort of thing
> instead of print.

I'll agree, but that sounds like a rabbit hole.  I've added a TODO.

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:929: print ('WARNING: %s is not recognized as a
valid revision specification,'
On 2014/05/08 23:50:06, iannucci wrote:
> I would use %r (everywhere) to get automatic quoting (in case it has spaces or
> something).

Done.

https://codereview.chromium.org/268393002/diff/20001/scripts/slave/bot_update...
scripts/slave/bot_update.py:957: 'svn url. To specify Tip of Tree, set rev to
HEAD.')
On 2014/05/08 23:50:06, iannucci wrote:
> Note that gclient would accept `--revision src@<rev>` as well, so we should
> filter the 'root' portion based on the .gclient spec entries.
> 
> Where is the code that does that (I see in your examples you tried that
already
> :))

Discussed offline - All revision handling is done by bot_update.
If --revision matches a spec, it enforces this at the git_checkout() level
(before gclient sync)
Otherwise, it enforces this after gclient sync.  The main reason we don't pass
it directly into "gclient sync" is because gclient wouldn't support
git_url@revision and we have no way to 100% get a correct url->folder mapping
before running gclient sync... besides rewriting a DEPS parser.

[iannucci, 2014-05-10 01:07:18]

lgtm

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

https://codereview.chromium.org/268393002/diff/60001/scripts/slave/bot_update...
scripts/slave/bot_update.py:692: if revision and revision.lower() != 'head':
nit, I would upper() since 'HEAD' is more greppable than 'head'

[Ryan Tseng, 2014-05-10 01:11:35]

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

https://codereview.chromium.org/268393002/diff/60001/scripts/slave/bot_update...
scripts/slave/bot_update.py:692: if revision and revision.lower() != 'head':
On 2014/05/10 01:07:18, iannucci wrote:
> nit, I would upper() since 'HEAD' is more greppable than 'head'

Done.

[Ryan Tseng, 2014-05-10 01:11:39]

The CQ bit was checked by hinoka@google.com

[commit-bot: I haz the power, 2014-05-10 01:12:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hinoka@chromium.org/268393002/80001

[commit-bot: I haz the power, 2014-05-10 01:13:39]

Change committed as 269470