Download hooks only in "git cl config"

Download hooks only in "git cl config"

also, use http download instead of scp,
because scp won't work well on Windows.

BUG=chromium:113153
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=121820

[M-A Ruel, 2012-02-09 14:57:19]

https://chromiumcodereview.appspot.com/9369023/diff/1/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/9369023/diff/1/git_cl.py#newcode753
git_cl.py:753: fileobj = urllib2.urlopen(src)
1. use urllib.urlretrieve()
2. Do not fetch if the file exist.
3. If the request fails, write an empty file?
4. Have this run automatically all the time, we don't require users to git cl
config if codereview.settings is present.
5. A quick mocked test?

[ukai, 2012-02-10 06:58:57]

On 2012/02/09 14:57:19, Marc-Antoine Ruel wrote:
> https://chromiumcodereview.appspot.com/9369023/diff/1/git_cl.py
> File git_cl.py (right):
> 
> https://chromiumcodereview.appspot.com/9369023/diff/1/git_cl.py#newcode753
> git_cl.py:753: fileobj = urllib2.urlopen(src)
> 1. use urllib.urlretrieve()
> 2. Do not fetch if the file exist.
> 3. If the request fails, write an empty file?

> config if codereview.settings is present.
> 5. A quick mocked test?

Done.

> 4. Have this run automatically all the time, we don't require users to git cl

hooks/commit-msg should be installed before "git commit", "git cl upload" or so.
 so, i need to request users to git cl config.
(or should I add hooks in DEPS?)

[M-A Ruel, 2012-02-10 15:50:04]

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode159
git_cl.py:159: LoadCodereviewSettingsFromFile(cr_settings_file)
That's where it should load the file if not present. But not delete when it is
present, unlike when git cl config is run.

DownloadHooks(False)

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode748
git_cl.py:748: def DownloadHooks():
Add force argument

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode750
git_cl.py:750: if settings.GetIsGerrit():
if not settings.GetIsGerrit():
  return

reduces indentation.

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode755
git_cl.py:755: if os.path.exists(dst):
if os.path.exists(dst):
  if not force:
    return
  os.remove(dst)
...

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode774
git_cl.py:774: DownloadHooks()
True

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode783
git_cl.py:783: DownloadHooks()
True

[ukai, 2012-02-13 02:57:43]

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode159
git_cl.py:159: LoadCodereviewSettingsFromFile(cr_settings_file)
On 2012/02/10 15:50:05, Marc-Antoine Ruel wrote:
> That's where it should load the file if not present. But not delete when it is
> present, unlike when git cl config is run.
> 
> DownloadHooks(False)

Done.

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode748
git_cl.py:748: def DownloadHooks():
On 2012/02/10 15:50:05, Marc-Antoine Ruel wrote:
> Add force argument

Done.

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode750
git_cl.py:750: if settings.GetIsGerrit():
On 2012/02/10 15:50:05, Marc-Antoine Ruel wrote:
> if not settings.GetIsGerrit():
>   return
> 
> reduces indentation.

Done.

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode755
git_cl.py:755: if os.path.exists(dst):
On 2012/02/10 15:50:05, Marc-Antoine Ruel wrote:
> if os.path.exists(dst):
>   if not force:
>     return
>   os.remove(dst)
> ...

Done.

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode774
git_cl.py:774: DownloadHooks()
On 2012/02/10 15:50:05, Marc-Antoine Ruel wrote:
> True

Done.

https://chromiumcodereview.appspot.com/9369023/diff/2001/git_cl.py#newcode783
git_cl.py:783: DownloadHooks()
On 2012/02/10 15:50:05, Marc-Antoine Ruel wrote:
> True

Done.

[M-A Ruel, 2012-02-13 06:48:07]

lgtm

[commit-bot: I haz the power, 2012-02-13 07:03:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ukai@chromium.org/9369023/8001

[commit-bot: I haz the power, 2012-02-13 07:09:04]

Presubmit check for 9369023-8001 failed and returned exit status 1.

warning: code.google.com certificate with fingerprint
ce:3e:bd:64:4d:e5:66:1c:e8:63:eb:99:3d:7e:75:36:a0:86:0e:03 not verified (check
hostfingerprints or web.cacerts config setting)
Running presubmit commit checks ...
************* Module git_cl_test
R0201: 49,2:CodereviewSettingsFileMock.read: Method could be a function
Checking out rietveld...
Running push-basic.sh
Running save-description-on-failure.sh
Running basic.sh
Running upload-stale.sh
Running upload-local-tracking-branch.sh
Running patch.sh
Running submit-from-new-dir.sh
Running post-dcommit-hook-test.sh
Running hooks.sh
Running abandon.sh
Running upstream.sh

** Presubmit Messages **
You should install python 2.5 and run ln -s $(which python2.5) python.
A great place to put this symlink is in depot_tools.
Otherwise, you break depot_tools on python 2.5, you get to keep the pieces.

** Presubmit ERRORS **
Fix pylint errors first.

Presubmit checks took 352.9s to calculate.

[Marc-Antoine Ruel (Google), 2012-02-13 20:47:52]

still lgtm

http://codereview.chromium.org/9369023/diff/8001/tests/git_cl_test.py
File tests/git_cl_test.py (right):

http://codereview.chromium.org/9369023/diff/8001/tests/git_cl_test.py#newcode49
tests/git_cl_test.py:49: def read(self):
You can disable the warning at file level. It's not an interesting one in tests;

# pylint: disable=R0201