Skip to content

Commit

Permalink
Allow bindings from include_defs to be loaded into a namespace.
Browse files Browse the repository at this point in the history
Summary:
Extend include_defs to allow defs from a file to be loaded into a separate
namespace instead of being merged into the globals of the current file.

For example, suppose define bar `bar` in a defs file, and include it via
`include_defs('//path/to/DEFS', 'foo')`. This will make the binding accessible
via `foo.bar` rather than `bar`.

This is implemented as an optional parameter so the legacy behavior is
unchanged, you can also pass `None` as the 2nd parameter to explicitly import
into global scope.

Test Plan: Added a test case. CI should ensure no regressions.

Reviewed By: mjpieters

fbshipit-source-id: a3f69d1
  • Loading branch information
yiding authored and facebook-github-bot committed Mar 13, 2017
1 parent 3956756 commit 1ef6123
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 8 deletions.
22 changes: 18 additions & 4 deletions docs/function/include_defs.soy
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ macros for creating more complex build rules.

{param args}

{call buck.functionArg}
{call buck.arg}
{param name: 'path' /}
{param desc}
The first and only argument is a path, of sorts, to a file containing
A path, of sorts, to a file containing
{sp}<a href="{ROOT}extending/macros.html"><code>macros</code></a> and
constants. It looks similar to a build target because it starts with
{sp}<code>//</code> (indicating the root of the project), but is not a
Expand All @@ -44,6 +45,19 @@ macros for creating more complex build rules.
{/param}
{/call}

{call buck.arg}
{param name: 'namespace' /}
{param default: 'None' /}
{param desc}
A string representing a namespace in which the bindings from the other file will be stored.
When set, an object of the given name that holds the bindings from the other file is put into the
global scope of the current file. If unset, the bindings from the other file are injected directly
into global scope of the current file.
<p>
If the name is already in scope, it is simply overwritten.
{/param}
{/call}

{/param}

{param examples}
Expand All @@ -62,12 +76,12 @@ Then another build file could include the array using
copy-and-paste definitions across build files:

{literal}<pre class="prettyprint lang-py">
include_defs('//core/DEFS')
include_defs('//core/DEFS', 'core')

android_binary(
name = 'example',
# ...
no_dx = JARS_TO_EXCLUDE_FROM_DX,
no_dx = core.JARS_TO_EXCLUDE_FROM_DX,
)
</pre>{/literal}

Expand Down
16 changes: 12 additions & 4 deletions src/com/facebook/buck/json/buck_parser/buck.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,8 +609,8 @@ def with_env_interceptors(self):
yield

def _merge_globals(self, mod, dst):
"""
Copy the global definitions from one globals dict to another.
# type: (types.ModuleType, Dict[str, Any]) -> None
"""Copy the global definitions from one globals dict to another.
Ignores special attributes and attributes starting with '_', which
typically denote module-level private attributes.
Expand Down Expand Up @@ -735,7 +735,7 @@ def _called_from_project_file(self):
filename = inspect.getframeinfo(frame).filename
return is_in_dir(filename, self._project_root)

def _include_defs(self, is_implicit_include, name):
def _include_defs(self, is_implicit_include, name, namespace=None):
# type: (bool, str) -> None
"""Pull the named include into the current caller's context.
Expand All @@ -753,7 +753,14 @@ def _include_defs(self, is_implicit_include, name):
# Look up the caller's stack frame and merge the include's globals
# into it's symbol table.
frame = get_caller_frame(skip=['_functools', __name__])
self._merge_globals(mod, frame.f_globals)
if namespace is not None:
# If using a fresh namespace, create a fresh module to populate.
fresh_module = imp.new_module(namespace)
fresh_module.__file__ = mod.__file__
self._merge_globals(mod, fresh_module.__dict__)
frame.f_globals[namespace] = fresh_module
else:
self._merge_globals(mod, frame.f_globals)

# Pull in the include's accounting of its own referenced includes
# into the current build context.
Expand Down Expand Up @@ -1064,6 +1071,7 @@ def _parse_autodeps(self, autodeps_file):
format(autodeps_file))

def process(self, watch_root, project_prefix, path, diagnostics):
# type: (str, Optional[str], str, List[Diagnostic]) -> List[Dict[str, Any]]
"""Process a build file returning a dict of its rules and includes."""
build_env, mod = self._process_build_file(watch_root, project_prefix,
os.path.join(self._project_root, path))
Expand Down
48 changes: 48 additions & 0 deletions src/com/facebook/buck/json/buck_parser/processor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@


def foo_rule(name, srcs=[], visibility=[], build_env=None):
"""A dummy build rule."""
add_rule({
'buck.type': 'foo',
'name': name,
Expand Down Expand Up @@ -65,6 +66,13 @@ def with_envs(envs):
class ProjectFile(object):

def __init__(self, root, path, contents):
# type: (str, str, Sequence[str]) -> None
"""Record of a file that can be written to disk.
:param root: root.
:param path: path to write the file, relative to the root.
:param contents: lines of file context
"""
self.path = path
self.name = '//{0}'.format(path)
self.root = root
Expand All @@ -87,10 +95,12 @@ def tearDown(self):
shutil.rmtree(self.project_root, True)

def write_file(self, pfile):
# type: (ProjectFile) -> None
with open(os.path.join(self.project_root, pfile.path), 'w') as f:
f.write(pfile.contents)

def write_files(self, *pfiles):
# type: (*ProjectFile) -> None
for pfile in pfiles:
self.write_file(pfile)

Expand Down Expand Up @@ -801,6 +811,44 @@ def test_bser_encoding_failure(self):
'parse',
decoded_result['diagnostics'][0]['source'])

def test_values_from_namespaced_includes_accessible_only_via_namespace(self):
defs_file = ProjectFile(
root=self.project_root,
path='DEFS',
contents=(
'value = 2',
)
)
build_file = ProjectFile(
self.project_root,
path='BUCK',
contents=(
'include_defs("//DEFS", "defs")',
'foo_rule(name="foo" + str(defs.value), srcs=[])',
)
)
self.write_files(defs_file, build_file)
processor = self.create_build_file_processor(extra_funcs=[foo_rule])
with processor.with_builtins(__builtin__.__dict__):
result = processor.process(self.project_root, None, 'BUCK', [])
self.assertTrue(
[x for x in result if x.get('name', '') == 'foo2'],
"result should contain rule with name derived from a value in namespaced defs",
)
# should not be in global scope
self.write_file(ProjectFile(
self.project_root,
path='BUCK_fail',
contents=(
'include_defs("//DEFS", "defs")',
'foo_rule(name="foo" + str(value), srcs=[])',
)
))
with processor.with_builtins(__builtin__.__dict__):
self.assertRaises(
NameError,
lambda: processor.process(self.project_root, None, 'BUCK_fail', []))


if __name__ == '__main__':
unittest.main()

0 comments on commit 1ef6123

Please sign in to comment.