[recipe_engine] Fix a couple path-related bugs.

recipe_engine/loader.py

 # Copyright 2016 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 import collections
 import contextlib
 import imp
 import inspect
 import os
 import sys
@@ skipping 565 matching lines @@
     # Replace class-level Requirements placeholders in the recipe API with
     # their instance-level real values.
     map(lambda (k, v): setattr(mod_api, k, _resolve_requirement(v, engine)),
         ((k, v) for k, v in type(mod_api).__dict__.iteritems()
          if isinstance(v, _UnresolvedRequirement)))
 
     mod_api.initialize()
     return mod_api
 
   mapper = DependencyMapper(instantiator)
   # Provide a fake module to the ScriptApi so that recipes can use:

[martiniss, 2017/02/02 21:12:18]

should this comment be moved? It doesn't seem to apply to the name, that I can
tell.

[iannucci, 2017/02/02 22:29:13]

no, it still applies; this whole section is in order to construct fakeModule

   #   * .name
   #   * .resource
   #   * .package_repo_resource
   # This is obviously a hack, however it homogenizes the api and removes the
   # need for some ugly workarounds in user code. A better way to do this would
   # be to migrate all recipes to be members of modules.
-  name = os.path.join(
-    toplevel_package.name,
-    os.path.relpath(
-      os.path.splitext(recipe_script_path)[0],
-      toplevel_package.recipe_dirs[0]))
+  is_module = recipe_script_path.startswith(toplevel_package.module_dirs[0])
+  rel_to = toplevel_package.recipe_dirs[0]

[martiniss, 2017/02/02 21:12:18]

I don't like all the [0] we have everywhere with module_dirs and recipe_dirs,
which we should technically iterate over, but never actually do.

[iannucci, 2017/02/02 22:29:13]

agree. I'll take a stab at cleaning this up in a subsequent CL.

+  if is_module:
+    rel_to = toplevel_package.module_dirs[0]
+  rel_path = os.path.relpath(os.path.splitext(recipe_script_path)[0], rel_to)
+  if is_module:
+    mod_name, recipe = rel_path.split(os.path.sep, 1)
+    rel_path = "%s:%s" % (mod_name, recipe)
+  name = "%s::%s" % (toplevel_package.name, rel_path)
   fakeModule = collections.namedtuple(
     "fakeModule", "PACKAGE_REPO_ROOT NAME RESOURCE_DIRECTORY")(
       Path(PackageRepoBasePath(toplevel_package)),
       name,
       Path(RecipeScriptBasePath(
         name, os.path.splitext(recipe_script_path)[0]+".resources")))
   api = RecipeScriptApi(module=fakeModule, engine=engine,
                   test_data=test_data.get_module_test_data(None))
   for k, v in toplevel_deps.iteritems():
     setattr(api, k, mapper.instantiate(v))
+
+  # Always instantiate the path module at least once so that string functions on
+  # Path objects work. This extra load doesn't actually attach the loaded path
+  # module to the api return, so if recipes want to use the path module, they
+  # still need to import it. If the recipe already loaded the path module
+  # (somewhere, could be transitively), then this extra load is a no-op.
+  # TODO(iannucci): The way paths work need to be reimplemented sanely :/
+  path_spec = engine._universe_view.deps_from_spec(['recipe_engine/path'])
+  mapper.instantiate(path_spec['path'])
+
   return api
 
 
 def create_test_api(toplevel_deps, universe):
   def instantiator(mod, deps):
     modapi = (getattr(mod, 'TEST_API', None) or RecipeTestApi)(module=mod)
     for k,v in deps.iteritems():
       setattr(modapi.m, k, v)
     return modapi
 
   mapper = DependencyMapper(instantiator)
   api = RecipeTestApi(module=None)
   for k,v in toplevel_deps.iteritems():
     setattr(api, k, mapper.instantiate(v))
   return api
 
 
 def _resolve_requirement(req, engine):
   if req._typ == 'client':
     return engine._get_client(req._name)
   else:
     raise ValueError('Unknown requirement type [%s]' % (req._typ,))