Some changes to allow recipes and modules to live noncentrally

Some changes to allow recipes and modules to live noncentrally

* This allows recipe modules to be loaded by path instead of by a global name.  When this is done, a local name is specified for the dependency.  
* The RECIPE_MODULES pseudopackage containing all modules is gone, replaced with a DEPS pseudopackage which only contains the dependencies of a module, identified by their local name.

This is a stepping stone.  We will eventually depend on repos, not paths, but this makes sure that things are decoupled enough from global names to do so.

Internal side changes: https://chromereviews.googleplex.com/187467013

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

[luqui, 2015-04-30 01:34:03]

luqui@chromium.org changed reviewers:
+ iannucci@chromium.org

[luqui, 2015-04-30 01:34:04]

Okay, time to get concrete. ptal

[iannucci, 2015-05-05 23:35:59]

lgtm % api nits

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/RE...
File scripts/slave/README.recipes.md (right):

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/RE...
scripts/slave/README.recipes.md:353: `CONFIG_CTX = DEPS['hello'].CONFIG_CTX`.
This will be useful for
import DEPS
CONFIG_CTX = DEPS['hello'].CONFIG_CTX

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/an...
File scripts/slave/annotated_run.py (right):

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/an...
scripts/slave/annotated_run.py:428: recipe_module =
recipe_loader.load_recipe(recipe, loader)
erm... shouldn't this be 

  recipe_module = loader.load(recipe)

?

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
File scripts/slave/recipe_config_types.py (right):

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_config_types.py:13: RECIPE_MODULE_PREFIX =
'\1RECIPE_MODULES'
\1? wat?

Maybe just don't put it in sys.modules?

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
File scripts/slave/recipe_loader.py (right):

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:1: # Copyright 2013 The Chromium Authors. All
rights reserved.
discussed:

A given recipe body of code (modules, recipes) belongs to a LUCI-cfg 'project'
(e.g. v8, chromium, etc.), and then the DEPS for a recipe/module would refer to
these as '<project>/module/<name>' (or something). As a special case, objects in
a given project can refer to each other with the project name of '.' e.g.
'./recipe/build' could depend on './module/foo'.

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:25: def __init__(self, recipe_dict, loader):
don't use it?

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:40: class PathDependency(object):
let's make a Dependency interface.

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:76: class ModuleLoader(object):
api nitpicks:

convenience methods below modify the state of this, and it's not clear that
constructing a new loader on a call-by-call basis would be broken

'loader' doesn't imply the permanent state tracking function of the object

'cache' is not really ephemeral, and maybe should be _modules?

or something?


personal pref: the convenience methods could really be methods of this object,
so it's implied that they modify its state.

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:238: a module, executing a hook ("instantiator")
for each module.
s/hook/callback

instantiator(module, deps_dict)

instantiator will only be called once per module load (not called if
DependencyMapper is reusing results from a previous call)

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:250: 
f = instantiator

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:269: deps_dict[name] = self.instantiate(dep)
dict comprehension?

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_loader.py:277:
test_data=test_data.get_module_test_data(mod.NAME))
NAME will need to be (eventually) a global name (probably ('<project>',
'modules', '<module name>'))

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
File scripts/slave/recipe_modules/bot_update/api.py (right):

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/re...
scripts/slave/recipe_modules/bot_update/api.py:8: import traceback
don't need this

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/un...
File scripts/slave/unittests/recipe_configs_test.py (left):

https://chromiumcodereview.appspot.com/1111413005/diff/40001/scripts/slave/un...
scripts/slave/unittests/recipe_configs_test.py:98: init_recipe_modules()
???????

[luqui, 2015-05-06 22:18:39]

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/README.re...
File scripts/slave/README.recipes.md (right):

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/README.re...
scripts/slave/README.recipes.md:353: `CONFIG_CTX = DEPS['hello'].CONFIG_CTX`.
This will be useful for
On 2015/05/05 23:35:58, iannucci wrote:
> import DEPS
> CONFIG_CTX = DEPS['hello'].CONFIG_CTX

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/annotated...
File scripts/slave/annotated_run.py (right):

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/annotated...
scripts/slave/annotated_run.py:428: recipe_module =
recipe_loader.load_recipe(recipe, loader)
On 2015/05/05 23:35:59, iannucci wrote:
> erm... shouldn't this be 
> 
>   recipe_module = loader.load(recipe)
> 
> ?

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_co...
File scripts/slave/recipe_config_types.py (right):

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_co...
scripts/slave/recipe_config_types.py:13: RECIPE_MODULE_PREFIX =
'\1RECIPE_MODULES'
On 2015/05/05 23:35:59, iannucci wrote:
> \1? wat?
> 
> Maybe just don't put it in sys.modules?

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_lo...
File scripts/slave/recipe_loader.py (right):

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_lo...
scripts/slave/recipe_loader.py:25: def __init__(self, recipe_dict, loader):
On 2015/05/05 23:35:59, iannucci wrote:
> don't use it?

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_lo...
scripts/slave/recipe_loader.py:40: class PathDependency(object):
On 2015/05/05 23:35:59, iannucci wrote:
> let's make a Dependency interface.

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_lo...
scripts/slave/recipe_loader.py:76: class ModuleLoader(object):
On 2015/05/05 23:35:59, iannucci wrote:
> api nitpicks:
> 
> convenience methods below modify the state of this, and it's not clear that
> constructing a new loader on a call-by-call basis would be broken
> 
> 'loader' doesn't imply the permanent state tracking function of the object
> 
> 'cache' is not really ephemeral, and maybe should be _modules?
> 
> or something?
> 
> 
> personal pref: the convenience methods could really be methods of this object,
> so it's implied that they modify its state.

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_lo...
scripts/slave/recipe_loader.py:238: a module, executing a hook ("instantiator")
for each module.
On 2015/05/05 23:35:59, iannucci wrote:
> s/hook/callback
> 
> instantiator(module, deps_dict)
> 
> instantiator will only be called once per module load (not called if
> DependencyMapper is reusing results from a previous call)

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_lo...
scripts/slave/recipe_loader.py:269: deps_dict[name] = self.instantiate(dep)
On 2015/05/05 23:35:59, iannucci wrote:
> dict comprehension?

Done.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_lo...
scripts/slave/recipe_loader.py:277:
test_data=test_data.get_module_test_data(mod.NAME))
On 2015/05/05 23:35:59, iannucci wrote:
> NAME will need to be (eventually) a global name (probably ('<project>',
> 'modules', '<module name>'))

Acknowledged.

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/bot_update/api.py (right):

https://codereview.chromium.org/1111413005/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/bot_update/api.py:8: import traceback
On 2015/05/05 23:35:59, iannucci wrote:
> don't need this

Done.

[luqui, 2015-05-06 22:18:45]

The CQ bit was checked by luqui@chromium.org

[luqui, 2015-05-06 22:18:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/1111413005/#ps60001
(title: "Review comments")

[commit-bot: I haz the power, 2015-05-06 22:19:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1111413005/60001

[commit-bot: I haz the power, 2015-05-06 22:25:19]

Committed patchset #4 (id:60001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=295160

[Michael Achenbach, 2015-05-07 07:26:19]

On 2015/05/06 22:25:19, I haz the power (commit-bot) wrote:
> Committed patchset #4 (id:60001) as
> http://src.chromium.org/viewvc/chrome?view=rev&revision=295160

This broke our auto-roller recipes, e.g. here:
http://build.chromium.org/p/client.v8/builders/Auto-roll%20-%20v8rel/builds/2...

[Michael Achenbach, 2015-05-07 07:43:12]

On 2015/05/07 07:26:19, Michael Achenbach wrote:
> On 2015/05/06 22:25:19, I haz the power (commit-bot) wrote:
> > Committed patchset #4 (id:60001) as
> > http://src.chromium.org/viewvc/chrome?view=rev&revision=295160
> 
> This broke our auto-roller recipes, e.g. here:
>
http://build.chromium.org/p/client.v8/builders/Auto-roll%20-%20v8rel/builds/2...

Furthermore
scripts/slave/unittests/recipe_simulation_test.py list
appears to hang now.

scripts/slave/unittests/recipe_simulation_test.py train v8/auto_roll_v8rel
errors out with the same errors as on the production bot. Why did the CQ not
catch this?

[Michael Achenbach, 2015-05-07 07:55:09]

Fix for the latter: https://codereview.chromium.org/1134443004/

[Michael Achenbach, 2015-05-07 08:36:22]

That recipe list hangs was a false alarm. Did some more updating locally and now
that works. The only remaining question for me is why the changes that got
necessary in https://codereview.chromium.org/1134443004/ weren't shown by
presubmit.