Added `gclient hookinfo`. This will be used to convert hooks into

Added `gclient hookinfo`.  This will be used to convert hooks into
repo post-sync hooks.

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

[M-A Ruel, 2012-01-28 02:15:18]

https://chromiumcodereview.appspot.com/9232068/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/1/gclient.py#newcode1095
gclient.py:1095: gclient_utils.CheckCallAndFilterAndHeader = _f
:/ I'd rather have you pass an argument to RunHooksRecursively(), then
_RunHookAction() than monkey patch here.

[szager, 2012-01-30 18:47:07]

https://chromiumcodereview.appspot.com/9232068/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/1/gclient.py#newcode1095
gclient.py:1095: gclient_utils.CheckCallAndFilterAndHeader = _f
On 2012/01/28 02:15:18, Marc-Antoine Ruel wrote:
> :/ I'd rather have you pass an argument to RunHooksRecursively(), then
> _RunHookAction() than monkey patch here.

OK, less monkey more patch.

[M-A Ruel, 2012-01-30 19:23:07]

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py#newcode589
gclient.py:589: """Evaluates all hooks, running actions as needed. run()
... running |action| for each hook. or something like that.

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py#newcode592
gclient.py:592: if not action:
action = action or self._RunHookAction

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py#newcode620
gclient.py:620: s.RunHooksRecursively(options, action=action)
My idea was to still call _RunHookAtion but add an argument to it. The new code
duplicate some logic. :/

What about refactoring it:
1. Rename RunHooksRecursively() to GetHooksToRun() to grab all the hooks that
should be run.
2. Have the caller RunHooks() or HookINfo() to run/print the hooks as wanted.

I think it'd be saner to maintain. wdyt?

[szager, 2012-01-31 00:22:20]

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py#newcode589
gclient.py:589: """Evaluates all hooks, running actions as needed. run()
On 2012/01/30 19:23:07, Marc-Antoine Ruel wrote:
> ... running |action| for each hook. or something like that.

Done.

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py#newcode592
gclient.py:592: if not action:
On 2012/01/30 19:23:07, Marc-Antoine Ruel wrote:
> action = action or self._RunHookAction

Done.

https://chromiumcodereview.appspot.com/9232068/diff/4001/gclient.py#newcode620
gclient.py:620: s.RunHooksRecursively(options, action=action)
On 2012/01/30 19:23:07, Marc-Antoine Ruel wrote:
> My idea was to still call _RunHookAtion but add an argument to it. The new
code
> duplicate some logic. :/
> 
> What about refactoring it:
> 1. Rename RunHooksRecursively() to GetHooksToRun() to grab all the hooks that
> should be run.
> 2. Have the caller RunHooks() or HookINfo() to run/print the hooks as wanted.
> 
> I think it'd be saner to maintain. wdyt?

OK.  Patch is now much bigger...

[M-A Ruel, 2012-01-31 01:15:25]

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode591
gclient.py:591: logging.debug(hook_dict)
FTR, you'll have to remove these before committing.

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode604
gclient.py:604: def GetHooks(self, options, result=None):
I think it'd be simpler if you simply returned the list instead of accepting it
as a parameter and extend() as needed.

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode609
gclient.py:609: result = [] if result is None else result
result = result or []
?

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode1440
gclient.py:1440: print client.GetHooks(options)
for hook in client.GetHooks(options):
  print ' '.join(hook)
?

[szager, 2012-01-31 06:47:39]

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode591
gclient.py:591: logging.debug(hook_dict)
On 2012/01/31 01:15:25, Marc-Antoine Ruel wrote:
> FTR, you'll have to remove these before committing.

I don't mind removing them (done), but why?  These lines were present in the old
code.

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode604
gclient.py:604: def GetHooks(self, options, result=None):
On 2012/01/31 01:15:25, Marc-Antoine Ruel wrote:
> I think it'd be simpler if you simply returned the list instead of accepting
it
> as a parameter and extend() as needed.

Done.

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode609
gclient.py:609: result = [] if result is None else result
On 2012/01/31 01:15:25, Marc-Antoine Ruel wrote:
> result = result or []
> ?

Obsolete now.

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode1440
gclient.py:1440: print client.GetHooks(options)
On 2012/01/31 01:15:25, Marc-Antoine Ruel wrote:
> for hook in client.GetHooks(options):
>   print ' '.join(hook)
> ?

I'd like the output to be parse-able by python, so I prefer to leave them in a
list.

[M-A Ruel, 2012-01-31 15:18:33]

Can you add a unit test to tests/gclient_test.py? No need for a smoke test, just
use adhoc mocks as needed. Then you're done!

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/8001/gclient.py#newcode591
gclient.py:591: logging.debug(hook_dict)
On 2012/01/31 06:47:39, szager wrote:
> On 2012/01/31 01:15:25, Marc-Antoine Ruel wrote:
> > FTR, you'll have to remove these before committing.
> 
> I don't mind removing them (done), but why?  These lines were present in the
old
> code.

I'm stupid. You can add these back.

[szager, 2012-03-12 23:12:30]

Added a test.

[M-A Ruel, 2012-03-12 23:52:06]

https://chromiumcodereview.appspot.com/9232068/diff/11001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/11001/gclient.py#newcode1468
gclient.py:1468: work_queue = gclient_utils.ExecutionQueue(options.jobs, None)
I'd prefer you to use RunOnDeps(None, [])

https://chromiumcodereview.appspot.com/9232068/diff/11001/gclient.py#newcode1472
gclient.py:1472: print '; '.join([' '.join(hook) for hook in
client.GetHooks(options)])
print '; '.join(' '.join(hook) for hook in client.GetHooks(options))

https://chromiumcodereview.appspot.com/9232068/diff/11001/tests/gclient_test.py
File tests/gclient_test.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/11001/tests/gclient_test....
tests/gclient_test.py:251: def testHooks(self):
Use self.root_dir, trial_dir.TestCase already manages temporary directory
creation and deletion.

[szager, 2012-03-13 00:00:17]

https://chromiumcodereview.appspot.com/9232068/diff/11001/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/11001/gclient.py#newcode1468
gclient.py:1468: work_queue = gclient_utils.ExecutionQueue(options.jobs, None)
On 2012/03/12 23:52:06, Marc-Antoine Ruel wrote:
> I'd prefer you to use RunOnDeps(None, [])

Done.

https://chromiumcodereview.appspot.com/9232068/diff/11001/gclient.py#newcode1472
gclient.py:1472: print '; '.join([' '.join(hook) for hook in
client.GetHooks(options)])
On 2012/03/12 23:52:06, Marc-Antoine Ruel wrote:
> print '; '.join(' '.join(hook) for hook in client.GetHooks(options))

Done.

https://chromiumcodereview.appspot.com/9232068/diff/11001/tests/gclient_test.py
File tests/gclient_test.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/11001/tests/gclient_test....
tests/gclient_test.py:251: def testHooks(self):
On 2012/03/12 23:52:06, Marc-Antoine Ruel wrote:
> Use self.root_dir, trial_dir.TestCase already manages temporary directory
> creation and deletion.

Done.

[M-A Ruel, 2012-03-13 00:05:11]

lgtm with nits

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py#newcode609
gclient.py:609: """Turn a parsed 'hook' dict into an executable command."""
Turns

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py#newcode624
gclient.py:624: """Evaluate all hooks, and return them in a flat list.
Evaluates

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py#newcode626
gclient.py:626: run() must have been called before to load the DEPS.
s/run/RunOnDeps/

[szager, 2012-03-13 00:08:46]

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py#newcode609
gclient.py:609: """Turn a parsed 'hook' dict into an executable command."""
On 2012/03/13 00:05:11, Marc-Antoine Ruel wrote:
> Turns

Done.

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py#newcode624
gclient.py:624: """Evaluate all hooks, and return them in a flat list.
On 2012/03/13 00:05:11, Marc-Antoine Ruel wrote:
> Evaluates

Done.

https://chromiumcodereview.appspot.com/9232068/diff/12003/gclient.py#newcode626
gclient.py:626: run() must have been called before to load the DEPS.
On 2012/03/13 00:05:11, Marc-Antoine Ruel wrote:
> s/run/RunOnDeps/

Done.

[commit-bot: I haz the power, 2012-03-13 18:39:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szager@google.com/9232068/17003

[commit-bot: I haz the power, 2012-03-13 18:46:23]

Change committed as 126426