Check binary existence in gclient: 2nd try.

Check binary existence in gclient: 2nd try.

The first approach crrev.com/132446 was reverted due to
Windows build breaks. This approach is more robust because
it actually tests the binary by running the command actually.

BUG=114483
TEST=succeeded on Linux

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

[M-A Ruel, 2012-04-17 12:15:55]

http://codereview.chromium.org/10103024/diff/1/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/10103024/diff/1/gclient_scm.py#newcode114
gclient_scm.py:114: def BinaryExists(self):
This function may be called over 100 times for a gclient checkout. On windows,
this can be a significant slow down. As such, I recommend making it a
@classmethod, have a class level variable to know if it was detected, default it
to None, and if cls._binary_exist is None:
  try:
    cls._binary_exist = gclient_utils.CheckCall(...)
  except OSError:
    cls._binary_exist = False
return cls._binary_exist

Note that scm.py already has a function to get the tool's version,
SVN.AssertVersion() and GIT.AssertVersion. It'd probably be better to call this
function, since it's called eventually and it's already caching its return
value.

http://codereview.chromium.org/10103024/diff/1/tests/gclient_scm_test.py
File tests/gclient_scm_test.py (right):

http://codereview.chromium.org/10103024/diff/1/tests/gclient_scm_test.py#newc...
tests/gclient_scm_test.py:73: gclient_scm.SCMWrapper.BinaryExists =
_DummyBinaryExists
you can simplify to:
gclient_scm.SCMWrapper.BinaryExists = lambda _: True

[Jun Mukai, 2012-04-18 02:46:35]

http://codereview.chromium.org/10103024/diff/1/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/10103024/diff/1/gclient_scm.py#newcode114
gclient_scm.py:114: def BinaryExists(self):
On 2012/04/17 12:15:55, Marc-Antoine Ruel wrote:
> This function may be called over 100 times for a gclient checkout. On windows,
> this can be a significant slow down. As such, I recommend making it a
> @classmethod, have a class level variable to know if it was detected, default
it
> to None, and if cls._binary_exist is None:
>   try:
>     cls._binary_exist = gclient_utils.CheckCall(...)
>   except OSError:
>     cls._binary_exist = False
> return cls._binary_exist
> 
> Note that scm.py already has a function to get the tool's version,
> SVN.AssertVersion() and GIT.AssertVersion. It'd probably be better to call
this
> function, since it's called eventually and it's already caching its return
> value.

Oh, got it.  Modified to use AssertVersion.

http://codereview.chromium.org/10103024/diff/1/tests/gclient_scm_test.py
File tests/gclient_scm_test.py (right):

http://codereview.chromium.org/10103024/diff/1/tests/gclient_scm_test.py#newc...
tests/gclient_scm_test.py:73: gclient_scm.SCMWrapper.BinaryExists =
_DummyBinaryExists
On 2012/04/17 12:15:55, Marc-Antoine Ruel wrote:
> you can simplify to:
> gclient_scm.SCMWrapper.BinaryExists = lambda _: True

Done.

[M-A Ruel, 2012-04-18 12:37:01]

lgtm

http://codereview.chromium.org/10103024/diff/5001/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/10103024/diff/5001/gclient_scm.py#newcode797
gclient_scm.py:797: return scm.SVN.AssertVersion('0')[0]
We require svn 1.4.

[Jun Mukai, 2012-04-18 12:48:09]

http://codereview.chromium.org/10103024/diff/5001/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/10103024/diff/5001/gclient_scm.py#newcode797
gclient_scm.py:797: return scm.SVN.AssertVersion('0')[0]
On 2012/04/18 12:37:01, Marc-Antoine Ruel wrote:
> We require svn 1.4.

Done.

[commit-bot: I haz the power, 2012-04-18 12:48:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/10103024/6003

[commit-bot: I haz the power, 2012-04-18 12:55:45]

Change committed as 132783