Add a --deps-file flag to gclient to allow using a deps file other than DEPS.

Add a --deps-file flag to gclient to allow using a deps file other than DEPS.

TEST=covered by gclient_smoketest.py
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=87367

[nsylvain, 2011-05-26 20:56:23]

running the smoke tests manually work :

depot_tools/tests$ python gclient_smoketest.py
........................................
----------------------------------------------------------------------
Ran 40 tests in 33.447s

OK


But running them part of the presubmit fail:


CalledProcessError: Command git svn clone --prefix origin/ -T trunk
svn://127.0.0.1:10000/svn/
/tmp/trial2DvPeD/__main__.GitSvnCheckout.testSvnProps/foo.git --quiet returned
non-zero exit status 1 in /tmp/trial2DvPeD/__main__.GitSvnCheckout.testSvnProps


I don't think my code caused this error though.

[cmp, 2011-05-26 21:04:20]

http://codereview.chromium.org/7062029/diff/1/gclient.py
File gclient.py (right):

http://codereview.chromium.org/7062029/diff/1/gclient.py#newcode974
gclient.py:974: help='overrides the default name for the DEPS file')
Can we add '--git-deps'?  It would be a boolean flag, default false.  When set,
below it would imply setting options.deps_file to .DEPS.git.

http://codereview.chromium.org/7062029/diff/1/tests/gclient_smoketest.py
File tests/gclient_smoketest.py (right):

http://codereview.chromium.org/7062029/diff/1/tests/gclient_smoketest.py#newc...
tests/gclient_smoketest.py:202: '    "deps_file"   : "DEPS",\n'
I don't see a test to cover today's case of a missing "deps_file".  It is
probably worth adding it.

[nsylvain, 2011-05-26 21:13:28]

http://codereview.chromium.org/7062029/diff/1/gclient.py
File gclient.py (right):

http://codereview.chromium.org/7062029/diff/1/gclient.py#newcode974
gclient.py:974: help='overrides the default name for the DEPS file')
On 2011/05/26 21:04:20, cmp wrote:
> Can we add '--git-deps'?  It would be a boolean flag, default false.  When
set,
> below it would imply setting options.deps_file to .DEPS.git.

Done.

http://codereview.chromium.org/7062029/diff/1/tests/gclient_smoketest.py
File tests/gclient_smoketest.py (right):

http://codereview.chromium.org/7062029/diff/1/tests/gclient_smoketest.py#newc...
tests/gclient_smoketest.py:202: '    "deps_file"   : "DEPS",\n'
On 2011/05/26 21:04:20, cmp wrote:
> I don't see a test to cover today's case of a missing "deps_file".  It is
> probably worth adding it.

It's covered by the tests passing "--deps" for example. Like testMultiSolutions.

[cmp, 2011-05-26 21:14:56]

lgtm with nit

http://codereview.chromium.org/7062029/diff/1005/gclient.py
File gclient.py (right):

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode975
gclient.py:975: parser.add_option('--git-deps', action='store_true',
Is "default=False," needed here?

[Dirk Pranke, 2011-05-26 22:09:22]

drive-by nit: it might be nice to note in the change description why you're
adding this (e.g., different deps for git-based checkouts, etc.).

[M-A Ruel, 2011-05-27 12:37:34]

I noted a few things if you want the setting to propagate. If you don't want it
to propagate, it'd be nice to be clearer with some comments at the relevant
places about why not propagating this setting.

http://codereview.chromium.org/7062029/diff/1005/gclient.py
File gclient.py (right):

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode141
gclient.py:141: DEPS_FILE = 'DEPS'
I recommend deleting this variable.

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode157
gclient.py:157: self.deps_file = deps_file or self.DEPS_FILE
self.deps_file = deps_file

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode335
gclient.py:335: None, should_process))
self.deps_file, should_process))

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode648
gclient.py:648: s.get('deps_file', None),
'DEPS' ?

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode975
gclient.py:975: parser.add_option('--git-deps', action='store_true',
On 2011/05/26 21:14:56, cmp wrote:
> Is "default=False," needed here?

No.

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode997
gclient.py:997: deps_file = '.DEPS.git'
.DEPS? Why try to hide the file?

[nsylvain, 2011-05-27 16:25:08]

http://codereview.chromium.org/7062029/diff/1005/gclient.py
File gclient.py (right):

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode141
gclient.py:141: DEPS_FILE = 'DEPS'
On 2011/05/27 12:37:34, Marc-Antoine Ruel wrote:
> I recommend deleting this variable.

Done.

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode157
gclient.py:157: self.deps_file = deps_file or self.DEPS_FILE
On 2011/05/27 12:37:34, Marc-Antoine Ruel wrote:
> self.deps_file = deps_file

Done.

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode335
gclient.py:335: None, should_process))
On 2011/05/27 12:37:34, Marc-Antoine Ruel wrote:
> self.deps_file, should_process))

Done.

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode648
gclient.py:648: s.get('deps_file', None),
On 2011/05/27 12:37:34, Marc-Antoine Ruel wrote:
> 'DEPS' ?
makes sense. Done

http://codereview.chromium.org/7062029/diff/1005/gclient.py#newcode997
gclient.py:997: deps_file = '.DEPS.git'
On 2011/05/27 12:37:34, Marc-Antoine Ruel wrote:
> .DEPS? Why try to hide the file?
Generated file. Should not be edited manually. Once we move to git for real, we
can get rid of this, and DEPS will be the real thing.

[M-A Ruel, 2011-05-28 22:58:23]

lgtm

http://codereview.chromium.org/7062029/diff/8001/gclient.py
File gclient.py (right):

http://codereview.chromium.org/7062029/diff/8001/gclient.py#newcode648
gclient.py:648: s.get('deps_file', 'DEPS'),
self.deps_file?