Make gclient_scm.py use cache_dir

Make gclient_scm.py use cache_dir

Instead of having custom logic for dealing with cache directories, use
git_cache.py to populate caches.

Also fixes a bug in git_cache.py where it was looking for lockfiles in cwd rather than the cache dir.

Other changes:
* _Run now returns output.
* Always print to stdout in CheckCallAndFilterOutput, even if it gets a carriage return.  This is done because git progress report are carriage returns and not newlines and we don't want everything on the same line and not strip out the CRs.
* Removed members changed tests, its not very useful to know a new import is added.

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

[agable, 2014-02-26 22:43:11]

https://codereview.chromium.org/180243006/diff/10001/gclient.py
File gclient.py (left):

https://codereview.chromium.org/180243006/diff/10001/gclient.py#oldcode1832
gclient.py:1832: if os.environ.get('CHROME_HEADLESS') == '1':
This seems dangerous until we're entirely off of svn?

https://codereview.chromium.org/180243006/diff/10001/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/180243006/diff/10001/gclient_scm.py#newcode693
gclient_scm.py:693: return subprocess2.check_output([git_bin, 'cache', 'exists',
'--cache-dir',
could also make _Run able to return output?

https://codereview.chromium.org/180243006/diff/10001/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/180243006/diff/10001/git_cache.py#newcode242
git_cache.py:242: if os.path.isdir(os.path.join(options.cache_dir, path))]
d'oh

[Ryan Tseng, 2014-02-27 23:31:40]

Re-upped

+Robbie FYI for gclient_utils_test.py

https://codereview.chromium.org/180243006/diff/10001/gclient.py
File gclient.py (left):

https://codereview.chromium.org/180243006/diff/10001/gclient.py#oldcode1832
gclient.py:1832: if os.environ.get('CHROME_HEADLESS') == '1':
On 2014/02/26 22:43:12, agable wrote:
> This seems dangerous until we're entirely off of svn?

Added a condition to skip this if .update.flag is found

https://codereview.chromium.org/180243006/diff/10001/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/180243006/diff/10001/gclient_scm.py#newcode693
gclient_scm.py:693: return subprocess2.check_output([git_bin, 'cache', 'exists',
'--cache-dir',
On 2014/02/26 22:43:12, agable wrote:
> could also make _Run able to return output?

Done.

[agable, 2014-02-28 02:06:32]

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py#newcode150
gclient_scm.py:150: cache_dir = None
...I don't see *any* code in this file that assigns to this. Do we *ever* hit
the cache_dir codepaths?

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py#newcode153
gclient_scm.py:153: cache_locks = collections.defaultdict(threading.Lock)
This can go away, I believe.

https://codereview.chromium.org/180243006/diff/30001/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/180243006/diff/30001/gclient_utils.py#newcode486
gclient_utils.py:486: return output.getvalue()
Just straight up doing this seems like it could break stuff. The return value of
CheckCallAndFilter just switched from False to True for successful runs. There's
probably some code somewhere that says "if CheckCallAndFilterfoo)...". It should
only return output if given a special flag or something.

https://codereview.chromium.org/180243006/diff/30001/tests/gclient_utils_test.py
File tests/gclient_utils_test.py (left):

https://codereview.chromium.org/180243006/diff/30001/tests/gclient_utils_test...
tests/gclient_utils_test.py:27: class GclientUtilsUnittest(GclientUtilBase):
You could fix the expectation in the test instead of removing the test entirely.

But honestly it's a horrible test so I'm okay with this.

[Ryan Tseng, 2014-02-28 02:22:50]

Removed more membership changed tests, this time in gclient_scm.py.  They're
silly tests that don't test functionality, but instead tests "did the code
change?"

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py#newcode150
gclient_scm.py:150: cache_dir = None
On 2014/02/28 02:06:32, agable wrote:
> ...I don't see *any* code in this file that assigns to this. Do we *ever* hit
> the cache_dir codepaths?

Well 691 does, since it passes it into git cache populate.

Its sent by bot update using the .gclient file, and probably injected in this
way:
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/gcli...

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py#newcode153
gclient_scm.py:153: cache_locks = collections.defaultdict(threading.Lock)
On 2014/02/28 02:06:32, agable wrote:
> This can go away, I believe.

Done.

https://codereview.chromium.org/180243006/diff/30001/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/180243006/diff/30001/gclient_utils.py#newcode486
gclient_utils.py:486: return output.getvalue()
On 2014/02/28 02:06:32, agable wrote:
> Just straight up doing this seems like it could break stuff. The return value
of
> CheckCallAndFilter just switched from False to True for successful runs.
There's
> probably some code somewhere that says "if CheckCallAndFilterfoo)...". It
should
> only return output if given a special flag or something.

If you check
https://code.google.com/p/chromium/codesearch#search/&q=CheckCallAndFilter&sq...
and
https://code.google.com/p/chromium/codesearch#search/&q=CheckCallAndFilterAnd...

No one actually uses the result of this function.

https://codereview.chromium.org/180243006/diff/30001/tests/gclient_utils_test.py
File tests/gclient_utils_test.py (left):

https://codereview.chromium.org/180243006/diff/30001/tests/gclient_utils_test...
tests/gclient_utils_test.py:27: class GclientUtilsUnittest(GclientUtilBase):
On 2014/02/28 02:06:32, agable wrote:
> You could fix the expectation in the test instead of removing the test
entirely.
> 
> But honestly it's a horrible test so I'm okay with this.

I got a verbal OK from Robbie already :)

[agable, 2014-02-28 02:38:24]

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/180243006/diff/30001/gclient_scm.py#newcode150
gclient_scm.py:150: cache_dir = None
On 2014/02/28 02:22:50, Ryan T. wrote:
> On 2014/02/28 02:06:32, agable wrote:
> > ...I don't see *any* code in this file that assigns to this. Do we *ever*
hit
> > the cache_dir codepaths?
> 
> Well 691 does, since it passes it into git cache populate.

No, 691 (thankfully) only uses self.cache_dir if it is already set, but it
doesn't set it to anything.

> Its sent by bot update using the .gclient file, and probably injected in this
> way:
>
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/gcli...

OH GOD that's so terrifying. Can we please make it a input parameter to __init__
or something?

[agable, 2014-02-28 21:57:56]

lgtmshipitshipit

[Ryan Tseng, 2014-02-28 22:00:31]

The CQ bit was checked by hinoka@google.com

[commit-bot: I haz the power, 2014-02-28 22:00:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hinoka@google.com/180243006/50001

[commit-bot: I haz the power, 2014-02-28 22:02:32]

Change committed as 254248