Add --workspace option, so deps2git.py may be run in a

Add --workspace option, so deps2git.py may be run in a
git-based checkout.

Gross, horrible hacks.  If you make me refactor them, I will
rewrite the whole damn thing.  I mean it.

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

[nsylvain, 2012-10-10 17:33:28]

A few comments/questions. Probably not worth refactoring the whole thing just
for this.. not that ugly ;)

https://codereview.chromium.org/11085027/diff/1/deps2git/deps2git.py
File deps2git/deps2git.py (right):

https://codereview.chromium.org/11085027/diff/1/deps2git/deps2git.py#newcode26
deps2git/deps2git.py:26: def SvnRevToGitHash(path, svn_rev, git_url, repos_path,
workspace, git_host):
I'd like to rename "path" to something more meaningful, but I can't find what.
"dep_path_in_checkout" is probably too long.  Also,  why making it the first
argument instead of after workspace?

https://codereview.chromium.org/11085027/diff/1/deps2git/git_tools.py
File deps2git/git_tools.py (right):

https://codereview.chromium.org/11085027/diff/1/deps2git/git_tools.py#newcode54
deps2git/git_tools.py:54: cmd = 'git clone %s %s' % (git_url, git_repo)
Why can't you do Git('/', 'clone %s %s' % ..) ?

https://codereview.chromium.org/11085027/diff/1/deps2git/git_tools.py#newcode138
deps2git/git_tools.py:138: def Search(git_repo, svn_rev, mirror):
Again i'm going to nitpick about the naming. It took me a while to understand
what that really meant.   What I think it means is more something like
"use_chrome_checkout=False".  It would probably be nice to use the word
"checkout" in the name so we see the link between both files.

Alternatively, could this be something like "bare=True" ?

[szager, 2012-10-10 18:09:14]

https://codereview.chromium.org/11085027/diff/1/deps2git/deps2git.py
File deps2git/deps2git.py (right):

https://codereview.chromium.org/11085027/diff/1/deps2git/deps2git.py#newcode26
deps2git/deps2git.py:26: def SvnRevToGitHash(path, svn_rev, git_url, repos_path,
workspace, git_host):
On 2012/10/10 17:33:28, nsylvain wrote:
> I'd like to rename "path" to something more meaningful, but I can't find what.
> "dep_path_in_checkout" is probably too long.  Also,  why making it the first
> argument instead of after workspace?

No reason; I reordered the arguments as you suggested and renamed 'path' to
'dep_path'.

https://codereview.chromium.org/11085027/diff/1/deps2git/git_tools.py
File deps2git/git_tools.py (right):

https://codereview.chromium.org/11085027/diff/1/deps2git/git_tools.py#newcode54
deps2git/git_tools.py:54: cmd = 'git clone %s %s' % (git_url, git_repo)
On 2012/10/10 17:33:28, nsylvain wrote:
> Why can't you do Git('/', 'clone %s %s' % ..) ?

The gotcha is that the directory git_repo might not exist, so we can't run a
command with cwd=git_repo, as Git() would do.

I changed it to explicitly os.mkdir(git_repo) and then call Git().

https://codereview.chromium.org/11085027/diff/1/deps2git/git_tools.py#newcode138
deps2git/git_tools.py:138: def Search(git_repo, svn_rev, mirror):
On 2012/10/10 17:33:28, nsylvain wrote:
> Again i'm going to nitpick about the naming. It took me a while to understand
> what that really meant.   What I think it means is more something like
> "use_chrome_checkout=False".  It would probably be nice to use the word
> "checkout" in the name so we see the link between both files.
> 
> Alternatively, could this be something like "bare=True" ?

I think 'use_chrome_checkout' is overly-specific.  The real issue is this: is
this a regular clone, or a mirror?  And the only reason you need to know that is
to figure out whether to use --git-dir= when invoking git.

I could change it to 'is_mirror', if you'd prefer; and maybe it would be more
intuitive if I used a default value to clarify that it's a boolean:

def Search(git_repo, svn_rev, is_mirror=False)

WDYT?

[nsylvain, 2012-10-10 18:28:29]

LGTM

[commit-bot: I haz the power, 2012-10-10 19:44:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szager@chromium.org/11085027/6001

[commit-bot: I haz the power, 2012-10-10 19:44:49]

Change committed as 161157