Do not blindly assume that a .gclient file in a parent directory belongs to t...

Do not blindly assume that a .gclient file in a parent directory belongs to the current directory.

BUG=54238
TEST=GClientSmoke.testWrongConfig


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

[jochen (gone - plz use gerrit), 2010-09-02 13:19:28]

please review

[M-A Ruel, 2010-09-02 13:35:03]

It'd be interesting to use this code in trychange.py too. That would require
some significant refactoring that is outside the scope of this change.

http://codereview.chromium.org/3304007/diff/1/2
File depot_tools/gclient.py (right):

http://codereview.chromium.org/3304007/diff/1/2#newcode631
depot_tools/gclient.py:631: all_solutions = client.tree(False)
you should put that line just before 635.

http://codereview.chromium.org/3304007/diff/1/2#newcode634
depot_tools/gclient.py:634: cwd = cwd[len(path)+1:]
You should add a comment like:
# Validate the current directory we are in belongs to the .gclient file we
found.

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

http://codereview.chromium.org/3304007/diff/1/3#newcode151
depot_tools/tests/gclient_smoketest.py:151: if os.path.exists(conf):
The test directory is always deleted in setUp() so lines 151-155 aren't needed.

http://codereview.chromium.org/3304007/diff/1/3#newcode159
depot_tools/tests/gclient_smoketest.py:159: self.check(res,
self.gclient(['status'], other_src))
define res just above line 159, it makes it easier to read.

[jochen (gone - plz use gerrit), 2010-09-02 13:57:40]

http://codereview.chromium.org/3304007/diff/1/2
File depot_tools/gclient.py (right):

http://codereview.chromium.org/3304007/diff/1/2#newcode631
depot_tools/gclient.py:631: all_solutions = client.tree(False)
On 2010/09/02 13:35:03, Marc-Antoine Ruel wrote:
> you should put that line just before 635.

Done.

http://codereview.chromium.org/3304007/diff/1/2#newcode634
depot_tools/gclient.py:634: cwd = cwd[len(path)+1:]
On 2010/09/02 13:35:03, Marc-Antoine Ruel wrote:
> You should add a comment like:
> # Validate the current directory we are in belongs to the .gclient file we
> found.

Done.

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

http://codereview.chromium.org/3304007/diff/1/3#newcode151
depot_tools/tests/gclient_smoketest.py:151: if os.path.exists(conf):
On 2010/09/02 13:35:03, Marc-Antoine Ruel wrote:
> The test directory is always deleted in setUp() so lines 151-155 aren't
needed.

Done.

http://codereview.chromium.org/3304007/diff/1/3#newcode159
depot_tools/tests/gclient_smoketest.py:159: self.check(res,
self.gclient(['status'], other_src))
On 2010/09/02 13:35:03, Marc-Antoine Ruel wrote:
> define res just above line 159, it makes it easier to read.

Done.

[M-A Ruel, 2010-09-02 14:00:56]

lgtm with nit

http://codereview.chromium.org/3304007/diff/6001/7002
File depot_tools/tests/gclient_smoketest.py (right):

http://codereview.chromium.org/3304007/diff/6001/7002#newcode155
depot_tools/tests/gclient_smoketest.py:155: os.rmdir(other_src)
lines 155-157 aren't necessary either, tearDown() cleans off the temporary
directory. Isn't it awesome? :)