Add a git-drover.

Add a git-drover.

This uses the same trick as git-new-workdir to reuse an existing git
checkout without interfering with it. However, this makes it only usable
on platforms where os.symlink exists.

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

[Sam McNally, 2015-09-16 04:38:42]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2015-09-16 04:38:58]

Patchset #1 (id:20001) has been deleted

[Sam McNally, 2015-09-22 01:21:03]

Patchset #1 (id:40001) has been deleted

[Sam McNally, 2015-09-22 01:21:11]

Patchset #1 (id:60001) has been deleted

[Sam McNally, 2015-09-22 01:21:19]

Patchset #1 (id:80001) has been deleted

[Sam McNally, 2015-09-22 01:21:26]

Patchset #1 (id:100001) has been deleted

[Sam McNally, 2015-09-22 03:28:36]

Patchset #1 (id:120001) has been deleted

[Sam McNally, 2015-09-22 03:46:32]

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

[iannucci, 2015-09-22 04:18:11]

https://codereview.chromium.org/1342383002/diff/140001/git-drover
File git-drover (right):

https://codereview.chromium.org/1342383002/diff/140001/git-drover#newcode6
git-drover:6: . $(type -P python_git_runner.sh)
you'll want to update the help pages in docs
(https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/man/src...
and related 'drover' files) too so that `git help drover` and `git drover
--help` work.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py
File git_drover.py (right):

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode62
git_drover.py:62: self._run_git_command(['cherry-pick', '--abort'])
you could also just run this always and swallow the error

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode66
git_drover.py:66: os.chdir(self._cwd)
I'd very strongly advise to avoid chdir'ing (except possibly once in main()). It
can introduce an entire class of ugly bugs that you don't want :(

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode108
git_drover.py:108: ['rev-parse', 'branch-heads/%s' % self._branch],
let's use absolute refs (so: `refs/branch-heads/___`). Unless git sees the
`refs/` portion, it will 'search' for the most likely branch. This can end up
picking something weird if the user has more than one remote, or has copied refs
somewhere, or has ever run 'filter-branch', or....

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode164
git_drover.py:164: parent_git_dir = os.path.join(self._parent_repo, '.git')
you can use `git rev-parse --git-dir` to get this in case they're not in the
root of the repo.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode178
git_drover.py:178: ['checkout', '-b', branch_name, 'branch-heads/%s' %
self._branch],
absolute refs

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode180
git_drover.py:180: self._branch_name = branch_name
Is there any way we can use the actual git workdir script for this? Or share
this code in git_common.py or something?

Also, have you considered using `git clone --shared` or `git clone --local` to
do this operation?

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode182
git_drover.py:182: def _prepare_merge(self):
s/merge/cherry_pick/ everywhere (mentioned below)

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode187
git_drover.py:187: self._cherry_pick_in_progress = False
fwiw, git tracks this bit of information in a file in .git... maybe good to use
that instead of duplicating it (thinking that it could get out of sync on an
edge case error)?

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode206
git_drover.py:206: self._dev_null_file = open(os.devnull, 'w')
I'd probably do this in __init__, since it's essentially zero-cost, and if quiet
is true (which it is for the very first git invocation), then you'll end up
doing it anyway.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode226
git_drover.py:226: branch: A string containing the release branch number to
which to merge.
Does it work with 'mini' branches?

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode227
git_drover.py:227: cl: A string containing the git hash of the patch to merge.
maybe 'revision'?

Is it required to be the full git hash?

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode246
git_drover.py:246: help='the name of the branch to which to merge')
full name or just the bit after `refs/branch-heads/`? May want to supply an
example, and make sure that if they provide `1234` it works as well as
`refs/branch-heads/1234` (or errors, or something... just so it's well-defined).

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode251
git_drover.py:251: help='the hash of the change to merge')
s/merge/cherry-pick/ since 'merge' means something very specific in git, and
we're not using it that way here.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode257
git_drover.py:257: help=('the path to the chromium checkout to use; '
definitely want to explain what 'use' means here (perhaps in the man page?). I'm
assuming it's just for git objects?

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode263
git_drover.py:263: 'Just check that merging would succeed'))
mixed ' and " is weird... I see why you did it, but maybe switch both of them on
this 
line to " just to avoid the weirdness :)

[Sam McNally, 2015-09-22 08:30:55]

Patchset #2 (id:160001) has been deleted

[Sam McNally, 2015-09-23 01:10:28]

Patchset #3 (id:200001) has been deleted

[Sam McNally, 2015-09-23 01:10:40]

Patchset #2 (id:180001) has been deleted

[Sam McNally, 2015-09-23 01:14:09]

Patchset #2 (id:220001) has been deleted

[Sam McNally, 2015-09-23 01:16:07]

https://codereview.chromium.org/1342383002/diff/140001/git-drover
File git-drover (right):

https://codereview.chromium.org/1342383002/diff/140001/git-drover#newcode6
git-drover:6: . $(type -P python_git_runner.sh)
On 2015/09/22 04:18:11, iannucci wrote:
> you'll want to update the help pages in docs
>
(https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/man/src...
> and related 'drover' files) too so that `git help drover` and `git drover
> --help` work.

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py
File git_drover.py (right):

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode62
git_drover.py:62: self._run_git_command(['cherry-pick', '--abort'])
On 2015/09/22 04:18:11, iannucci wrote:
> you could also just run this always and swallow the error

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode66
git_drover.py:66: os.chdir(self._cwd)
On 2015/09/22 04:18:11, iannucci wrote:
> I'd very strongly advise to avoid chdir'ing (except possibly once in main()).
It
> can introduce an entire class of ugly bugs that you don't want :(

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode108
git_drover.py:108: ['rev-parse', 'branch-heads/%s' % self._branch],
On 2015/09/22 04:18:11, iannucci wrote:
> let's use absolute refs (so: `refs/branch-heads/___`). Unless git sees the
> `refs/` portion, it will 'search' for the most likely branch. This can end up
> picking something weird if the user has more than one remote, or has copied
refs
> somewhere, or has ever run 'filter-branch', or....

Done. I had to use refs/remotes/branch-heads/...

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode164
git_drover.py:164: parent_git_dir = os.path.join(self._parent_repo, '.git')
On 2015/09/22 04:18:11, iannucci wrote:
> you can use `git rev-parse --git-dir` to get this in case they're not in the
> root of the repo.

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode178
git_drover.py:178: ['checkout', '-b', branch_name, 'branch-heads/%s' %
self._branch],
On 2015/09/22 04:18:11, iannucci wrote:
> absolute refs

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode180
git_drover.py:180: self._branch_name = branch_name
On 2015/09/22 04:18:11, iannucci wrote:
> Is there any way we can use the actual git workdir script for this? Or share
> this code in git_common.py or something?

Moved it from gclient-new-workdir.py. This has to be slightly different; it sets
the workdir to do a sparse checkout and we don't want this to leak into the
parent checkout so .git/config and .git/info/sparse-checkout aren't shared.
> 
> Also, have you considered using `git clone --shared` or `git clone --local` to
> do this operation?

I hadn't, but I think this might be simpler than those. Doing a clone would
require copying across the right remote and creating a sparse clone is a bit
fiddly.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode182
git_drover.py:182: def _prepare_merge(self):
On 2015/09/22 04:18:11, iannucci wrote:
> s/merge/cherry_pick/ everywhere (mentioned below) 

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode187
git_drover.py:187: self._cherry_pick_in_progress = False
On 2015/09/22 04:18:11, iannucci wrote:
> fwiw, git tracks this bit of information in a file in .git... maybe good to
use
> that instead of duplicating it (thinking that it could get out of sync on an
> edge case error)?

Removed this.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode206
git_drover.py:206: self._dev_null_file = open(os.devnull, 'w')
On 2015/09/22 04:18:11, iannucci wrote:
> I'd probably do this in __init__, since it's essentially zero-cost, and if
quiet
> is true (which it is for the very first git invocation), then you'll end up
> doing it anyway.

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode226
git_drover.py:226: branch: A string containing the release branch number to
which to merge.
On 2015/09/22 04:18:11, iannucci wrote:
> Does it work with 'mini' branches?

I'm not sure what 'mini' branches are.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode227
git_drover.py:227: cl: A string containing the git hash of the patch to merge.
On 2015/09/22 04:18:11, iannucci wrote:
> maybe 'revision'?

Done.
> 
> Is it required to be the full git hash?

No, it can be anything that git-rev-parse likes.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode246
git_drover.py:246: help='the name of the branch to which to merge')
On 2015/09/22 04:18:11, iannucci wrote:
> full name or just the bit after `refs/branch-heads/`? May want to supply an
> example, and make sure that if they provide `1234` it works as well as
> `refs/branch-heads/1234` (or errors, or something... just so it's
well-defined).

Just the bit after.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode251
git_drover.py:251: help='the hash of the change to merge')
On 2015/09/22 04:18:11, iannucci wrote:
> s/merge/cherry-pick/ since 'merge' means something very specific in git, and
> we're not using it that way here.

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode257
git_drover.py:257: help=('the path to the chromium checkout to use; '
On 2015/09/22 04:18:11, iannucci wrote:
> definitely want to explain what 'use' means here (perhaps in the man page?).
I'm
> assuming it's just for git objects?

Done.

https://codereview.chromium.org/1342383002/diff/140001/git_drover.py#newcode263
git_drover.py:263: 'Just check that merging would succeed'))
On 2015/09/22 04:18:11, iannucci wrote:
> mixed ' and " is weird... I see why you did it, but maybe switch both of them
on
> this 
> line to " just to avoid the weirdness :)

Done.

[Sam McNally, 2015-09-29 05:45:23]

ping

[iannucci, 2015-09-29 06:46:01]

lgtm

[iannucci, 2015-09-29 06:46:15]

On 2015/09/29 at 06:46:01, iannucci wrote:
> lgtm

Sorry for the huge lag on that last review >_<

[Sam McNally, 2015-09-29 06:54:56]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2015-09-29 06:55:07]

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

[commit-bot: I haz the power, 2015-09-29 06:57:11]

Committed patchset #2 (id:240001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=296920