Switching nacl_deps_bump to git.

Switching nacl_deps_bump to git.

Switching to using the git client to do the deps bump.

BUG=None
TEST=manual
R=mseaborn@chromium.org, jfb@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=293844

[bradn, 2015-01-28 02:42:29]

bradnelson@google.com changed reviewers:
+ jfb@chromium.org

[JF, 2015-01-28 07:12:18]

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py
File nacl_deps_bump.py (right):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode143
nacl_deps_bump.py:143: subprocess.check_call(['git', 'fetch'], cwd=nacl_git_dir)
I'm not sure I understand what the NaCl cwd is expected to be. I assume we now
need a git checkout for this to work? Will the script warn if it's run from the
wrong place?

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode163
nacl_deps_bump.py:163: msg = 'NaCl: Update revision in DEPS, %s -> %s' %
(old_rev[:9], new_rev[:9])
Isn't the default short hash 7?

[Mark Seaborn, 2015-01-28 16:34:21]

Please set BUG=https://code.google.com/p/nativeclient/issues/detail?id=3928. 
LGTM.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py
File nacl_deps_bump.py (left):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#oldcode269
nacl_deps_bump.py:269: # TODO(mseaborn): Fix this automatic syncing.  Maybe this
should wait
I guess we can add back this automatic syncing if and when we need it.  These
revs rarely change.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py
File nacl_deps_bump.py (right):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode73
nacl_deps_bump.py:73: ['git', 'log', '--pretty=format:%h %ae %s', rev1 + '..' +
rev2],
Can you add "--reverse", so that the oldest changes are listed first, please? 
That would match the old behaviour and pnacl/deps_update.py.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode109
nacl_deps_bump.py:109: help='NaCl GIT revision to use (default is HEAD)')
Nit: capitalise as "Git"

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode143
nacl_deps_bump.py:143: subprocess.check_call(['git', 'fetch'], cwd=nacl_git_dir)
On 2015/01/28 07:12:17, JF wrote:
> I'm not sure I understand what the NaCl cwd is expected to be.

This just assumes it's run from a Chromium checkout with the usual native_client
subdir.  Brad is not changing this assumption.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode163
nacl_deps_bump.py:163: msg = 'NaCl: Update revision in DEPS, %s -> %s' %
(old_rev[:9], new_rev[:9])
On 2015/01/28 07:12:17, JF wrote:
> Isn't the default short hash 7?

Yes, JF is right.  Let's use 7.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py
File nacl_deps_bump_cronjob.py (right):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py#ne...
nacl_deps_bump_cronjob.py:52: # Returns the time that the given GIT revision was
committed, as a
"Git"

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py#ne...
nacl_deps_bump_cronjob.py:71: [(GetRevTime(nacl_git_dir, i), i) for i in
GetExistingJobs()])
"i" -> "rev"

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py#ne...
nacl_deps_bump_cronjob.py:83: nacl_git_dir, last_tried_rev, newest_rev)) - 1
I don't think the "- 1" is right.  Remove that?

[Mark Seaborn, 2015-01-28 16:44:15]

One more thing...

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py
File nacl_deps_bump.py (right):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode80
nacl_deps_bump.py:80: got.append('%s (%s) %s\n' % (h, author, message))
Can you add a colon, i.e.
  '%s: (%s) %s\n'
to match the old format and match pnacl/deps_update.py?

[bradn, 2015-01-28 17:37:33]

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py
File nacl_deps_bump.py (left):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#oldcode269
nacl_deps_bump.py:269: # TODO(mseaborn): Fix this automatic syncing.  Maybe this
should wait
On 2015/01/28 16:34:21, Mark Seaborn wrote:
> I guess we can add back this automatic syncing if and when we need it.  These
> revs rarely change.

Yeah I figure its problematic for the moment.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py
File nacl_deps_bump.py (right):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode143
nacl_deps_bump.py:143: subprocess.check_call(['git', 'fetch'], cwd=nacl_git_dir)
On 2015/01/28 07:12:17, JF wrote:
> I'm not sure I understand what the NaCl cwd is expected to be. I assume we now
> need a git checkout for this to work? Will the script warn if it's run from
the
> wrong place?

What mark said.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode143
nacl_deps_bump.py:143: subprocess.check_call(['git', 'fetch'], cwd=nacl_git_dir)
On 2015/01/28 16:34:21, Mark Seaborn wrote:
> On 2015/01/28 07:12:17, JF wrote:
> > I'm not sure I understand what the NaCl cwd is expected to be.
> 
> This just assumes it's run from a Chromium checkout with the usual
native_client
> subdir.  Brad is not changing this assumption.

Correct.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode163
nacl_deps_bump.py:163: msg = 'NaCl: Update revision in DEPS, %s -> %s' %
(old_rev[:9], new_rev[:9])
On 2015/01/28 16:34:21, Mark Seaborn wrote:
> On 2015/01/28 07:12:17, JF wrote:
> > Isn't the default short hash 7?
> 
> Yes, JF is right.  Let's use 7.

Done.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump.py#newcode163
nacl_deps_bump.py:163: msg = 'NaCl: Update revision in DEPS, %s -> %s' %
(old_rev[:9], new_rev[:9])
On 2015/01/28 07:12:17, JF wrote:
> Isn't the default short hash 7?

Done.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py
File nacl_deps_bump_cronjob.py (right):

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py#ne...
nacl_deps_bump_cronjob.py:52: # Returns the time that the given GIT revision was
committed, as a
On 2015/01/28 16:34:21, Mark Seaborn wrote:
> "Git"

Done.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py#ne...
nacl_deps_bump_cronjob.py:71: [(GetRevTime(nacl_git_dir, i), i) for i in
GetExistingJobs()])
On 2015/01/28 16:34:21, Mark Seaborn wrote:
> "i" -> "rev"

Done.

https://codereview.chromium.org/872813004/diff/1/nacl_deps_bump_cronjob.py#ne...
nacl_deps_bump_cronjob.py:83: nacl_git_dir, last_tried_rev, newest_rev)) - 1
On 2015/01/28 16:34:21, Mark Seaborn wrote:
> I don't think the "- 1" is right.  Remove that?

Ah, you're correct, didn't actually check the behavior.
Changed.

[bradn, 2015-01-28 17:37:42]

Committed patchset #2 (id:20001) manually as r293844 (presubmit successful).

[JF, 2015-01-31 22:35:04]

https://codereview.chromium.org/872813004/diff/20001/nacl_deps_bump.py
File nacl_deps_bump.py (right):

https://codereview.chromium.org/872813004/diff/20001/nacl_deps_bump.py#newcod...
nacl_deps_bump.py:145: new_rev = options.revision
This should be passed to `git rev-parse` so that it expands abbreviated hashes.

A revert was caused by this issue:
  https://codereview.chromium.org/889163003/