Add repo-managed checkout support to trychange.py

Add repo-managed checkout support to trychange.py

BUG=chromium-os:21286
TEST=Ran 'git-try' with/without --root=src, and with/without .gclient file

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

[M-A Ruel, 2011-10-10 18:16:06]

http://codereview.chromium.org/8174009/diff/1/trychange.py
File trychange.py (right):

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode101
trychange.py:101: while path != '/':
I'd like it to be windows-compatible, so I'd use something like
gclient_utils.FindFileUpwards('.repo') instead.

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode176
trychange.py:176: logging.info('Found .gclient at %s' % self.toplevel_root)
If you want, add:
assert os.path.abspath(self.toplevel_root) == self.toplevel_root

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode182
trychange.py:182: chromium_root = os.path.join(repo_root,
weird alignment, also I've refrained from hardcoding path like 'chromium' and
I'd rather to keep it this way.

Just generalize to:
1. use override if set.
2. find .gclient
3. find .repo

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode196
trychange.py:196: root = cur = os.path.abspath(self.checkout_root)
self.checkout_root is guaranteed to be abspath so no need to do it again.

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode198
trychange.py:198: root = os.path.abspath(self.toplevel_root)
If you also abspath toplevel_root, then you can do:
root = self.toplevel_root or self.checkout_root

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode337
trychange.py:337: 
bof, I'd revert this line

[rcui1, 2011-10-11 19:42:34]

Incorporated feedback.  PTAL.

http://codereview.chromium.org/8174009/diff/1/trychange.py
File trychange.py (right):

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode101
trychange.py:101: while path != '/':
On 2011/10/10 18:16:06, Marc-Antoine Ruel wrote:
> I'd like it to be windows-compatible, so I'd use something like
> gclient_utils.FindFileUpwards('.repo') instead.

Done.

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode176
trychange.py:176: logging.info('Found .gclient at %s' % self.toplevel_root)
On 2011/10/10 18:16:06, Marc-Antoine Ruel wrote:
> If you want, add:
> assert os.path.abspath(self.toplevel_root) == self.toplevel_root

Done.

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode182
trychange.py:182: chromium_root = os.path.join(repo_root,
On 2011/10/10 18:16:06, Marc-Antoine Ruel wrote:
> weird alignment, also I've refrained from hardcoding path like 'chromium' and
> I'd rather to keep it this way.
> 
> Just generalize to:
> 1. use override if set.
> 2. find .gclient
> 3. find .repo

Done.

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode196
trychange.py:196: root = cur = os.path.abspath(self.checkout_root)
On 2011/10/10 18:16:06, Marc-Antoine Ruel wrote:
> self.checkout_root is guaranteed to be abspath so no need to do it again.

Done.

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode198
trychange.py:198: root = os.path.abspath(self.toplevel_root)
I think the logic is pretty clear as it is.

On 2011/10/10 18:16:06, Marc-Antoine Ruel wrote:
> If you also abspath toplevel_root, then you can do:
> root = self.toplevel_root or self.checkout_root

http://codereview.chromium.org/8174009/diff/1/trychange.py#newcode337
trychange.py:337: 
On 2011/10/10 18:16:06, Marc-Antoine Ruel wrote:
> bof, I'd revert this line

Done.

[M-A Ruel, 2011-10-11 19:50:36]

And as I said over IM, use:
FindFileUpwards(os.path.join('..', '.repo'))

and make it look for directories too. I'm fine with simply using
os.path.exists() instead of os.path.isfile()

http://codereview.chromium.org/8174009/diff/7001/trychange.py
File trychange.py (right):

http://codereview.chromium.org/8174009/diff/7001/trychange.py#newcode177
trychange.py:177: if not self.options.no_gclient:
I feel like --no_gclient should also affect the search for .repo. Maybe the flag
should be renamed to be more generic.

http://codereview.chromium.org/8174009/diff/7001/trychange.py#newcode191
trychange.py:191: self.options.root =
gclient_utils.PathDifference(self.toplevel_root,
It's needed? I feel like it's a bug. :/

http://codereview.chromium.org/8174009/diff/7001/trychange.py#newcode198
trychange.py:198: if self.toplevel_root:
cur = self.checkout_root
root = self.toplevel_root or self.checkout_root

[rcui1, 2011-10-11 21:26:48]

http://codereview.chromium.org/8174009/diff/7001/trychange.py
File trychange.py (right):

http://codereview.chromium.org/8174009/diff/7001/trychange.py#newcode198
trychange.py:198: if self.toplevel_root:
On 2011/10/11 19:50:36, Marc-Antoine Ruel wrote:
> cur = self.checkout_root
> root = self.toplevel_root or self.checkout_root

Done.

[M-A Ruel, 2011-10-11 21:32:06]

lgtm

[M-A Ruel, 2011-10-11 21:42:14]

thanks! lgtm

[commit-bot: I haz the power, 2011-10-13 01:18:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rcui@google.com/8174009/6003

[commit-bot: I haz the power, 2011-10-13 01:25:08]

Change committed as 105229