Cleanup some of the depsets (#358)

Fix violations of --incompatible_depset_union

Fixes #345

closure/compiler/closure_js_binary.bzl

@@ -111,14 +111,17 @@ def _impl(ctx):
     # except unlike C++ there's no I/O operation penalty to using them since all
     # source paths that exist are being passed as flags.
     js_module_roots = sort_roots(
-        find_js_module_roots(
-            [ctx.outputs.bin],
-            ctx.workspace_name,
-            ctx.label,
-            getattr(ctx.attr, "includes", []),
-        ) +
-        js.js_module_roots,
+        depset(transitive = [
+            find_js_module_roots(
+                [ctx.outputs.bin],
+                ctx.workspace_name,
+                ctx.label,
+                getattr(ctx.attr, "includes", []),
+            ),
+            js.js_module_roots,
+        ]),
     )
+
     for root in js_module_roots:
         args.append("--js_module_root")
         args.append(root)
@@ -254,9 +257,11 @@ def _impl(ctx):
         ),
         runfiles = ctx.runfiles(
             files = files + ctx.files.data,
-            transitive_files = (collect_runfiles(deps) |
-                                collect_runfiles([ctx.attr.css]) |
-                                collect_runfiles(ctx.attr.data)),
+            transitive_files = depset(transitive = [
+                collect_runfiles(deps),
+                collect_runfiles([ctx.attr.css]),
+                collect_runfiles(ctx.attr.data),
+            ]),
         ),
     )
 

closure/compiler/closure_js_deps.bzl

@@ -60,9 +60,10 @@ def _impl(ctx):
         files = depset(outputs),
         runfiles = ctx.runfiles(
             files = outputs + ctx.files.data,
-            transitive_files = (depset(ctx.files._closure_library_base) |
-                                collect_runfiles(deps) |
-                                collect_runfiles(ctx.attr.data)),
+            transitive_files = depset(
+                ctx.files._closure_library_base,
+                transitive = [collect_runfiles(deps), collect_runfiles(ctx.attr.data)],
+            ),
         ),
     )
 

closure/compiler/closure_js_library.bzl

@@ -249,9 +249,6 @@ def _closure_js_library_impl(
     # collisions exist for any particular transitive closure. By making it
     # canonical, we can use it to propagate suppressions up to closure_js_binary.
     # TODO(davido): Find out how to avoid that hack
-    srcs_it = srcs
-    if type(srcs) == "depset":
-        srcs_it = srcs.to_list()
     modules = [
         convert_path_to_es6_module_name(
             f.path if not f.is_directory else f.path + "/*.js",
@@ -309,6 +306,9 @@ def _closure_js_library_impl(
         internal_expect_failure = internal_expect_failure,
     )
 
+    if type(internal_descriptors) == "list":
+        internal_descriptors = depset(internal_descriptors)
+
     # We now export providers to any parent Target. This is considered a public
     # interface because other Skylark rules can be designed to do things with
     # this data. Other Skylark rules can even export their own provider with the
@@ -343,35 +343,35 @@ def _closure_js_library_impl(
             # NestedSet<File> of all info files in the transitive closure. This
             # is used by JsCompiler to apply error suppression on a file-by-file
             # basis.
-            infos = js.infos + [info_file],
+            infos = depset([info_file], transitive = [js.infos]),
             ijs = ijs_file,
-            ijs_files = js.ijs_files + [ijs_file],
+            ijs_files = depset([ijs_file], transitive = [js.ijs_files]),
             # NestedSet<File> of all JavaScript source File artifacts in the
             # transitive closure. These files MUST be JavaScript.
-            srcs = js.srcs + srcs,
+            srcs = depset(srcs_it, transitive = [js.srcs]),
             # NestedSet<String> of all execroot path prefixes in the transitive
             # closure. For very simple projects, it will be empty. It is useful
             # for getting rid of Bazel generated directories, workspace names,
             # etc. out of module paths.  It contains the cartesian product of
             # generated roots, external repository roots, and includes
             # prefixes. This is passed to JSCompiler via the --js_module_root
             # flag. See find_js_module_roots() in defs.bzl.
-            js_module_roots = js.js_module_roots + js_module_roots,
+            js_module_roots = depset(js_module_roots, transitive = [js.js_module_roots]),
             # NestedSet<String> of all ES6 module name strings in the transitive
             # closure. These are generated from the source file path relative to
             # the longest matching root prefix. It is used to guarantee that
             # within any given transitive closure, no namespace collisions
             # exist. These MUST NOT begin with "/" or ".", or contain "..".
-            modules = js.modules + modules,
+            modules = depset(modules, transitive = [js.modules]),
             # NestedSet<File> of all protobuf definitions in the transitive
             # closure. It is used so Closure Templates can have information about
             # the structure of protobufs so they can be easily rendered in .soy
             # files with type safety. See closure_js_template_library.bzl.
-            descriptors = js.descriptors + internal_descriptors,
+            descriptors = depset(transitive = [js.descriptors, internal_descriptors]),
             # NestedSet<Label> of all closure_css_library rules in the transitive
             # closure. This is used by closure_js_binary can guarantee the
             # completeness of goog.getCssName() substitutions.
-            stylesheets = js.stylesheets + stylesheets,
+            stylesheets = depset(stylesheets, transitive = [js.stylesheets]),
             # Boolean indicating indicating if Closure Library's base.js is part
             # of the srcs subprovider. This field exists for optimization.
             has_closure_library = js.has_closure_library,
@@ -427,14 +427,13 @@ def _closure_js_library(ctx):
         # The usual suspects are exported as runfiles, in addition to raw source.
         runfiles = ctx.runfiles(
             files = srcs + ctx.files.data,
-            transitive_files = (depset([] if ctx.attr.no_closure_library else ctx.files._closure_library_base) |
-                                collect_runfiles(
-                                    unfurl(
-                                        ctx.attr.deps,
-                                        provider = "closure_js_library",
-                                    ),
-                                ) |
-                                collect_runfiles(ctx.attr.data)),
+            transitive_files = depset(
+                [] if ctx.attr.no_closure_library else ctx.files._closure_library_base,
+                transitive = [
+                    collect_runfiles(unfurl(ctx.attr.deps, provider = "closure_js_library")),
+                    collect_runfiles(ctx.attr.data),
+                ],
+            ),
         ),
     )
 

closure/private/defs.bzl

@@ -69,22 +69,23 @@ def collect_js(
         no_closure_library = False,
         css = None):
     """Aggregates transitive JavaScript source files from unfurled deps."""
-    srcs = depset()
-    ijs_files = depset()
-    infos = depset()
-    modules = depset()
-    descriptors = depset()
-    stylesheets = depset()
-    js_module_roots = depset()
+    srcs = []
+    direct_srcs = []
+    ijs_files = []
+    infos = []
+    modules = []
+    descriptors = []
+    stylesheets = []
+    js_module_roots = []
     has_closure_library = False
     for dep in deps:
-        srcs += getattr(dep.closure_js_library, "srcs", [])
-        ijs_files += getattr(dep.closure_js_library, "ijs_files", [])
-        infos += getattr(dep.closure_js_library, "infos", [])
-        modules += getattr(dep.closure_js_library, "modules", [])
-        descriptors += getattr(dep.closure_js_library, "descriptors", [])
-        stylesheets += getattr(dep.closure_js_library, "stylesheets", [])
-        js_module_roots += getattr(dep.closure_js_library, "js_module_roots", [])
+        srcs += [getattr(dep.closure_js_library, "srcs", depset())]
+        ijs_files += [getattr(dep.closure_js_library, "ijs_files", depset())]
+        infos += [getattr(dep.closure_js_library, "infos", depset())]
+        modules += [getattr(dep.closure_js_library, "modules", depset())]
+        descriptors += [getattr(dep.closure_js_library, "descriptors", depset())]
+        stylesheets += [getattr(dep.closure_js_library, "stylesheets", depset())]
+        js_module_roots += [getattr(dep.closure_js_library, "js_module_roots", depset())]
         has_closure_library = (
             has_closure_library or
             getattr(dep.closure_js_library, "has_closure_library", False)
@@ -94,21 +95,19 @@ def collect_js(
             fail("no_closure_library can't be used when Closure Library is " +
                  "already part of the transitive closure")
     elif has_direct_srcs and not has_closure_library:
-        srcs = depset(closure_library_base, transitive = [srcs])
+        direct_srcs += closure_library_base
         has_closure_library = True
     if css:
-        srcs = depset(
-            closure_library_base + [css.closure_css_binary.renaming_map],
-            transitive = [srcs],
-        )
+        direct_srcs += closure_library_base + [css.closure_css_binary.renaming_map]
+
     return struct(
-        srcs = srcs,
-        js_module_roots = js_module_roots,
-        ijs_files = ijs_files,
-        infos = infos,
-        modules = modules,
-        descriptors = descriptors,
-        stylesheets = stylesheets,
+        srcs = depset(direct_srcs, transitive = srcs),
+        js_module_roots = depset(transitive = js_module_roots),
+        ijs_files = depset(transitive = ijs_files),
+        infos = depset(transitive = infos),
+        modules = depset(transitive = modules),
+        descriptors = depset(transitive = descriptors),
+        stylesheets = depset(transitive = stylesheets),
         has_closure_library = has_closure_library,
     )
 
@@ -160,6 +159,7 @@ def find_js_module_roots(srcs, workspace_name, label, includes):
     relative to the root of a monolithic Bazel repository. Also, unlike the C++
     rules, there is no penalty for using includes in JavaScript compilation.
     """
+
     # TODO(davido): Find out how to avoid that hack
     srcs_it = srcs
     if type(srcs) == "depset":
@@ -264,6 +264,7 @@ def library_level_checks(
     for f in ijs_deps.to_list():
         args.append("--externs=%s" % f.path)
         inputs.append(f)
+
     # TODO(davido): Find out how to avoid that hack
     srcs_it = srcs
     if type(srcs) == "depset":

closure/testing/phantomjs_test.bzl

@@ -29,14 +29,17 @@ def _impl(ctx):
     if not ctx.attr.deps:
         fail("phantomjs_rule needs at least one dep")
     files = [ctx.outputs.executable]
-    srcs = depset()
+    srcs = []
+    direct_srcs = []
     deps = unfurl(ctx.attr.deps, provider = "closure_js_library")
     deps.append(ctx.attr.runner)
     for dep in deps:
         if hasattr(dep, "closure_js_binary"):
-            srcs += [dep.closure_js_binary.bin]
+            direct_srcs += [dep.closure_js_binary.bin]
         else:
-            srcs += dep.closure_js_library.srcs
+            srcs += [dep.closure_js_library.srcs]
+    srcs = depset(direct_srcs, transitive = srcs)
+
     args = [
         "#!/bin/sh\nexec " + ctx.executable._phantomjs.short_path,
         ctx.attr.harness.closure_js_binary.bin.short_path,
@@ -52,13 +55,15 @@ def _impl(ctx):
         files = depset(files),
         runfiles = ctx.runfiles(
             files = files + ctx.attr.data + [ctx.file.html],
-            transitive_files = (collect_runfiles(deps) |
-                                collect_runfiles(ctx.attr.data) |
-                                collect_runfiles([
-                                    ctx.attr._phantomjs,
-                                    ctx.attr.runner,
-                                    ctx.attr.harness,
-                                ])),
+            transitive_files = depset(transitive = [
+                collect_runfiles(deps),
+                collect_runfiles(ctx.attr.data),
+                collect_runfiles([
+                    ctx.attr._phantomjs,
+                    ctx.attr.runner,
+                    ctx.attr.harness,
+                ]),
+            ]),
         ),
     )
 

closure/webfiles/web_library.bzl

@@ -43,10 +43,10 @@ def _web_library(ctx):
     # process what came before
     deps = unfurl(ctx.attr.deps, provider = "webfiles")
     webpaths = []
-    manifests = depset(order = "postorder")
+    manifests = []
     for dep in deps:
         webpaths.append(dep.webfiles.webpaths)
-        manifests += dep.webfiles.manifests
+        manifests += [dep.webfiles.manifests]
 
     # process what comes now
     new_webpaths = []
@@ -85,11 +85,11 @@ def _web_library(ctx):
             src = manifest_srcs,
         ).to_proto(),
     )
-    manifests += [manifest]
+    manifests = depset([manifest], transitive = manifests, order = "postorder")
 
     # perform strict dependency checking
     inputs = [manifest]
-    direct_manifests = depset([manifest])
+    direct_manifests = [manifest]
     args = [
         "WebfilesValidator",
         "--dummy",
@@ -109,7 +109,7 @@ def _web_library(ctx):
         inputs.append(dep.webfiles.manifest)
         args.append("--direct_dep")
         args.append(dep.webfiles.manifest.path)
-    for man in difference(manifests, direct_manifests):
+    for man in difference(manifests, depset(direct_manifests)):
         inputs.append(man)
         args.append("--transitive_dep")
         args.append(man.path)
@@ -146,11 +146,11 @@ def _web_library(ctx):
         ),
     )
 
-    # export data to parent rules
-    transitive_runfiles = depset()
-    transitive_runfiles += ctx.attr._WebfilesServer.data_runfiles.files
-    for dep in deps:
-        transitive_runfiles += dep.data_runfiles.files
+    transitive_runfiles = depset(
+        transitive = [ctx.attr._WebfilesServer.data_runfiles.files] +
+                     [dep.data_runfiles.files for dep in deps],
+    )
+
     return struct(
         files = depset([ctx.outputs.executable, ctx.outputs.dummy]),
         exports = unfurl(ctx.attr.exports),