Skip to content

Commit

Permalink
Add fine-grained Closure Library build rules (#269)
Browse files Browse the repository at this point in the history
This change introduced a fine-grained build graph for the Closure Library. This
is most useful when the Closure Compiler isn't being used. For example, it's
now possible to depend on `@io_bazel_rules_closure//closure/library/object`
which is 11mB smaller (in terms of naïvely concatenated JavaScript sources)
than getting the same API from `@io_bazel_rules_closure//closure/library`.
The latency of tools like Clutz should also be improved.

The following additional changes needed to be made:

- `deps.js` no longer needs to be an implicit dependency thanks to
  `transitionalforwarddeclarations.js`.

- A `lenient` attribute is now available for `closure_js_library` which makes
  the compiler more easy-going.

- `goog.labs`, `goog.ui`, and third party APIs may no longer be exported from
  `//closure/library` and `//closure/library:testing` by default.
  • Loading branch information
jart committed May 9, 2018
1 parent 6403db4 commit dea93c4
Show file tree
Hide file tree
Showing 131 changed files with 13,768 additions and 1,232 deletions.
5 changes: 1 addition & 4 deletions closure/compiler/closure_js_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
load("//closure/private:defs.bzl",
"CLOSURE_WORKER_ATTR",
"CLOSURE_LIBRARY_BASE_ATTR",
"CLOSURE_LIBRARY_DEPS_ATTR",
"JS_LANGUAGES",
"JS_LANGUAGE_IN",
"JS_LANGUAGE_OUT_DEFAULT",
Expand All @@ -43,8 +42,7 @@ def _impl(ctx):
ctx.attr.language, ", ".join(JS_LANGUAGES)))

deps = unfurl(ctx.attr.deps, provider="closure_js_library")
js = collect_js(deps, ctx.file._closure_library_base,
ctx.file._closure_library_deps, css=ctx.attr.css)
js = collect_js(deps, ctx.files._closure_library_base, css=ctx.attr.css)
if not js.srcs:
fail("There are no JS source files in the transitive closure")

Expand Down Expand Up @@ -272,7 +270,6 @@ closure_js_binary = rule(
"internal_expect_warnings": attr.bool(default=False),
"_ClosureWorker": CLOSURE_WORKER_ATTR,
"_closure_library_base": CLOSURE_LIBRARY_BASE_ATTR,
"_closure_library_deps": CLOSURE_LIBRARY_DEPS_ATTR,
},
outputs={
"bin": "%{name}.js",
Expand Down
4 changes: 2 additions & 2 deletions closure/compiler/closure_js_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ load("//closure/private:defs.bzl",
def _impl(ctx):
deps = unfurl(ctx.attr.deps, provider="closure_js_library")
js = collect_js(deps)
closure_root = _dirname(long_path(ctx, ctx.file._closure_library_base))
closure_root = _dirname(long_path(ctx, ctx.files._closure_library_base[0]))
closure_rel = '/'.join(['..' for _ in range(len(closure_root.split('/')))])
outputs = [ctx.outputs.out]
# XXX: Other files in same directory will get schlepped in w/o sandboxing.
Expand All @@ -44,7 +44,7 @@ def _impl(ctx):
files=depset(outputs),
runfiles=ctx.runfiles(
files=outputs + ctx.files.data,
transitive_files=(depset([ctx.file._closure_library_base]) |
transitive_files=(depset(ctx.files._closure_library_base) |
collect_runfiles(deps) |
collect_runfiles(ctx.attr.data))))

Expand Down
36 changes: 25 additions & 11 deletions closure/compiler/closure_js_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
load("//closure/private:defs.bzl",
"CLOSURE_WORKER_ATTR",
"CLOSURE_LIBRARY_BASE_ATTR",
"CLOSURE_LIBRARY_DEPS_ATTR",
"JS_FILE_TYPE",
"JS_LANGUAGE_IN",
"library_level_checks",
Expand All @@ -41,8 +40,8 @@ def _maybe_declare_file(actions, file, name):
def closure_js_library_impl(
actions, label, workspace_name,

srcs, deps, testonly, suppress,
closure_library_base, closure_library_deps, _ClosureWorker,
srcs, deps, testonly, suppress, lenient,
closure_library_base, _ClosureWorker,

includes=(),
exports=depset(),
Expand All @@ -60,6 +59,20 @@ def closure_js_library_impl(
# TODO(yannic): Figure out how to modify |find_js_module_roots|
# so that we won't need |workspace_name| anymore.

if lenient:
suppress = suppress + [
"analyzerChecks",
"analyzerChecksInternal",
"deprecated",
"legacyGoogScopeRequire",
"lintChecks",
"missingOverride",
"reportUnknownTypes",
"strictCheckTypes",
"superfluousSuppress",
"unnecessaryEscape",
]

# TODO(yannic): Always use |actions.declare_file()|.
info_file = _maybe_declare_file(
actions, deprecated_info_file, '%s.pbtxt' % label.name)
Expand All @@ -74,8 +87,7 @@ def closure_js_library_impl(

# Collect all the transitive stuff the child rules have propagated. Bazel has
# a special nested set data structure that makes this efficient.
js = collect_js(deps, closure_library_base, closure_library_deps,
bool(srcs), no_closure_library)
js = collect_js(deps, closure_library_base, bool(srcs), no_closure_library)

# If closure_js_library depends on closure_css_library, that means
# goog.getCssName() is being used in srcs to reference CSS names in the
Expand Down Expand Up @@ -176,7 +188,7 @@ def closure_js_library_impl(
# The list of flags could potentially be very long. So we're going to write
# them all to a file which gets loaded automatically by our BazelWorker
# middleware.
argfile = create_argfile(actions, label.name, args)
argfile = create_argfile(actions, label.name, args)
inputs.append(argfile)

# Add a JsChecker edge to the build graph. The command itself will only be
Expand All @@ -199,6 +211,7 @@ def closure_js_library_impl(
output=_maybe_declare_file(
actions, deprecated_typecheck_file, '%s_typecheck' % label.name),
suppress=suppress,
lenient=lenient,
)

# We now export providers to any parent Target. This is considered a public
Expand Down Expand Up @@ -274,6 +287,8 @@ def _closure_js_library(ctx):
fail("Either 'srcs' or 'exports' must be specified")
if not ctx.files.srcs and ctx.attr.deps:
fail("'srcs' must be set when using 'deps', otherwise consider 'exports'")
if not ctx.files.srcs and (ctx.attr.suppress or ctx.attr.lenient):
fail("'srcs' must be set when using 'suppress' or 'lenient'")
if ctx.attr.language:
print("The closure_js_library 'language' attribute is now removed and " +
"is always set to " + JS_LANGUAGE_IN)
Expand All @@ -287,9 +302,9 @@ def _closure_js_library(ctx):
library = closure_js_library_impl(
ctx.actions, ctx.label, ctx.workspace_name,
srcs, ctx.attr.deps, ctx.attr.testonly, ctx.attr.suppress,
ctx.attr.lenient,

ctx.file._closure_library_base,
ctx.file._closure_library_deps,
ctx.files._closure_library_base,
ctx.executable._ClosureWorker,

getattr(ctx.attr, "includes", []),
Expand All @@ -311,8 +326,7 @@ def _closure_js_library(ctx):
runfiles=ctx.runfiles(
files=srcs + ctx.files.data,
transitive_files=(depset([] if ctx.attr.no_closure_library
else [ctx.file._closure_library_base,
ctx.file._closure_library_deps]) |
else ctx.files._closure_library_base) |
collect_runfiles(
unfurl(ctx.attr.deps,
provider="closure_js_library")) |
Expand All @@ -337,6 +351,7 @@ closure_js_library = rule(
"no_closure_library": attr.bool(),
"srcs": attr.label_list(allow_files=JS_FILE_TYPE),
"suppress": attr.string_list(),
"lenient": attr.bool(),

# deprecated
"externs": attr.label_list(allow_files=JS_FILE_TYPE),
Expand All @@ -347,7 +362,6 @@ closure_js_library = rule(
"internal_expect_failure": attr.bool(default=False),
"_ClosureWorker": CLOSURE_WORKER_ATTR,
"_closure_library_base": CLOSURE_LIBRARY_BASE_ATTR,
"_closure_library_deps": CLOSURE_LIBRARY_DEPS_ATTR,
},
# TODO(yannic): Deprecate.
# https://docs.bazel.build/versions/master/skylark/lib/globals.html#rule.outputs
Expand Down
2 changes: 1 addition & 1 deletion closure/compiler/test/closure_js_deps/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ file_test(
closure_js_library(
name = "goblin",
srcs = ["goblin.js"],
deps = ["//closure/library"],
deps = ["//closure/library/dom"],
)

closure_js_deps(
Expand Down
Loading

0 comments on commit dea93c4

Please sign in to comment.