Return unicode commit hashes in checkout.GitCheckout

Return unicode commit hashes in checkout.GitCheckout.

The motivation for this CL is:
When a project is turned on to use GitCheckout with try job verifiers, then the CQ crashes because of line 64 in http://src.chromium.org/viewvc/chrome/trunk/tools/commit-queue/pending_manager.py

This is because revision can only be one of (None, int, unicode). This crash only happens when try jobs are used (which is why I did not see it before).
This CL changes the return type to be unicode instead of string.

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

[iannucci, 2013-10-02 18:24:36]

https://codereview.chromium.org/25686005/diff/3001/tests/checkout_test.py
File tests/checkout_test.py (right):

https://codereview.chromium.org/25686005/diff/3001/tests/checkout_test.py#new...
tests/checkout_test.py:383: self.assertEquals(unicode, type(git_rev))
hmm... can't we just change the expected return type to str? Since it's actually
hex-encoded binary, I don't see any benefit to casting to unicode (unless it's
expecting to be able to get a refname, but even then, I'm pretty sure git
doesn't support unicode refs).

[rmistry, 2013-10-02 19:02:12]

https://codereview.chromium.org/25686005/diff/3001/tests/checkout_test.py
File tests/checkout_test.py (right):

https://codereview.chromium.org/25686005/diff/3001/tests/checkout_test.py#new...
tests/checkout_test.py:383: self.assertEquals(unicode, type(git_rev))
On 2013/10/02 18:24:36, iannucci wrote:
> hmm... can't we just change the expected return type to str? Since it's
actually
> hex-encoded binary, I don't see any benefit to casting to unicode (unless it's
> expecting to be able to get a refname, but even then, I'm pretty sure git
> doesn't support unicode refs).

We can, it will need a few changes in the CQ.
Since svn revs are expected to be ints I figured it would be ok if git revs are
expected to be unicodes, but if you prefer the other approach then that SGTM.
CL coming soon..

[M-A Ruel, 2013-10-03 01:04:05]

At the beginning I didn't like this CL but I don't mind much. It gets the job
done.

lgtm if you prefer.

[iannucci, 2013-10-03 01:05:37]

On 2013/10/03 01:04:05, M-A Ruel wrote:
> At the beginning I didn't like this CL but I don't mind much. It gets the job
> done.
> 
> lgtm if you prefer.

Yeah if the other way is going to be a lot more painful, then this lgtm. It was
more of a 'Lets do the right thing if possible'...

[rmistry, 2013-10-03 12:40:47]

On 2013/10/03 01:05:37, iannucci wrote:
> On 2013/10/03 01:04:05, M-A Ruel wrote:
> > At the beginning I didn't like this CL but I don't mind much. It gets the
job
> > done.
> > 
> > lgtm if you prefer.
> 
> Yeah if the other way is going to be a lot more painful, then this lgtm. It
was
> more of a 'Lets do the right thing if possible'...

Gave it a shot in https://codereview.chromium.org/25536018/ but ran into
problems getting the tests to pass in tests/model_test.py

Going ahead and submitting this CL since there maybe more complications involved
with having PersistentMixIn support 'str'.

[commit-bot: I haz the power, 2013-10-03 12:41:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rmistry@google.com/25686005/3001

[commit-bot: I haz the power, 2013-10-03 12:43:41]

Change committed as 226743