From 21b757480a1e3a67f1a25a8f27a404fc751e1477 Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Tue, 10 Apr 2018 22:55:55 -0700 Subject: [PATCH] Fix depsets in web_library and update README Adjusting the ordering in this manner makes the webfiles manifests easier to work with. It will also ensure the development webserver displays them in the correct order again. --- README.md | 60 +++++-------------- closure/webfiles/web_library.bzl | 2 +- .../closure/webfiles/server/Metadata.java | 11 ++-- 3 files changed, 22 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index d9b8a4ae16..dc943033da 100644 --- a/README.md +++ b/README.md @@ -70,15 +70,18 @@ notes. ## Setup -First you must [install][bazel-install] Bazel ≥0.3.1. Then you must add the -following to your `WORKSPACE` file: +First you must [install][bazel-install] Bazel. Then you add the following to +your `WORKSPACE` file: ```python http_archive( name = "io_bazel_rules_closure", - strip_prefix = "rules_closure-0.4.1", - sha256 = "ba5e2e10cdc4027702f96e9bdc536c6595decafa94847d08ae28c6cb48225124", - url = "https://mirror.bazel.build/github.com/bazelbuild/rules_closure/archive/0.4.1.tar.gz", + sha256 = "6691c58a2cd30a86776dd9bb34898b041e37136f2dc7e24cadaeaf599c95c657", + strip_prefix = "rules_closure-08039ba8ca59f64248bb3b6ae016460fe9c9914f", + urls = [ + "https://mirror.bazel.build/github.com/bazelbuild/rules_closure/archive/08039ba8ca59f64248bb3b6ae016460fe9c9914f.tar.gz", + "https://github.com/bazelbuild/rules_closure/archive/08039ba8ca59f64248bb3b6ae016460fe9c9914f.tar.gz", # 2018-01-16 + ], ) load("@io_bazel_rules_closure//closure:defs.bzl", "closure_repositories") @@ -136,8 +139,8 @@ Please see the test directories within this project for concrete examples of usa ```python load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library") -closure_js_library(name, srcs, data, deps, language, exports, suppress, - convention, no_closure_library) +closure_js_library(name, srcs, data, deps, exports, suppress, convention, + no_closure_library) ``` Defines a set of JavaScript sources. @@ -181,39 +184,6 @@ This rule can be referenced as though it were the following: point to [closure_js_library], [closure_js_template_library], [closure_css_library] and [closure_js_proto_library] rules. -- **language:** (String; optional; default is `"ECMASCRIPT5_STRICT"`) Variant of - JavaScript in which `srcs` are written. The following are valid options: - - - `ECMASCRIPT6_STRICT`: Nitpicky, shiny new JavaScript. - - `ECMASCRIPT5_STRICT`: Nitpicky, traditional JavaScript. - - `ECMASCRIPT6`: Shiny new JavaScript. - - `ECMASCRIPT5`: Traditional JavaScript. - - `ECMASCRIPT3`: 90's JavaScript. - - `ANY`: Indicates sources are compatible with any variant of JavaScript. - - Maintaining this attribute for your library rules is important because - [closure_js_binary] checks the `language` attribute of dependencies to - determine if it's a legal combination that's safe to compile. - - ![ECMAScript Language Combinations Diagram](https://i.imgur.com/xNZ9FAr.png) - - Combinations that traverse a red line cause strictness to decay and a warning - will be emitted. For example, if just one library is unstrict, then strictness - will be removed for your entire binary. Therefore we *strongly* recommend - that you use strict variants. - - **ProTip:** You are not required to put `"use strict"` at the tops of your - files. The Closure Compiler generates that in the output for you. - - The default language is `ECMASCRIPT5_STRICT` for three reasons. First, we want - to make the most conservative recommendation possible. Some ES6 features have - not yet been implemented in the Closure Compiler. We're working on - that. Secondly, it upgrades easily into `ECMASCRIPT6_STRICT`, should you - choose to use it later. Thirdly, PhantomJS only supports `ECMASCRIPT5_STRICT`, - so your unit tests will be able to run lightning fast in raw sources mode if - you write your code exclusively in that language. (XXX: Unfortunately a - [bug][phantomjs-bug] in PhantomJS is blocking this at the moment.) - - **exports:** (List of [labels]; optional) Listing dependencies here will cause them to become *direct* dependencies in parent rules. This functions similarly to [java_library.exports]. This can be used to create aliases for rules in @@ -624,9 +594,8 @@ This rule can be referenced as though it were the following: contain all transitive JS sources and data. - [closure_js_library]: `srcs` will be the generated JS output files, `data` - will contain the transitive data, `language` will be `ECMASCRIPT5_STRICT`, - `deps` will contain necessary libraries, and `no_closure_library` will be - `False`. + will contain the transitive data, `deps` will contain necessary libraries, and + `no_closure_library` will be `False`. ### Arguments @@ -776,7 +745,7 @@ This rule can be referenced as though it were the following: contain all transitive CSS/GSS sources and data. - [closure_js_library]: `srcs` is empty, `data` is the transitive CSS sources - and data, `language` is `ANY`, and `no_closure_library` is `True`. However the + and data, and `no_closure_library` is `True`. However the closure\_css\_library rule does pass special information along when used as a dep in closure\_js\_library. See its documentation to learn more. @@ -955,8 +924,7 @@ This rule can be referenced as though it were the following: sources and data. - [closure_js_library]: `srcs` will be the generated JS output files, `data` - will contain the transitive data, `language` will be `ECMASCRIPT5_STRICT`, and - `deps` will contain necessary libraries. + will contain the transitive data, and `deps` will contain necessary libraries. ### Arguments diff --git a/closure/webfiles/web_library.bzl b/closure/webfiles/web_library.bzl index f96d10768e..5d0973b84c 100644 --- a/closure/webfiles/web_library.bzl +++ b/closure/webfiles/web_library.bzl @@ -41,7 +41,7 @@ def _web_library(ctx): # process what came before deps = unfurl(ctx.attr.deps, provider="webfiles") webpaths = depset() - manifests = depset(order="topological") + manifests = depset(order="postorder") for dep in deps: webpaths += dep.webfiles.webpaths manifests += dep.webfiles.manifests diff --git a/java/io/bazel/rules/closure/webfiles/server/Metadata.java b/java/io/bazel/rules/closure/webfiles/server/Metadata.java index b9e2b0815f..21b57fee72 100644 --- a/java/io/bazel/rules/closure/webfiles/server/Metadata.java +++ b/java/io/bazel/rules/closure/webfiles/server/Metadata.java @@ -153,7 +153,7 @@ void loadMetadataIntoObjectGraph() throws IOException { manifestPaths.add(config.getPath()); WebfilesServerInfo params = config.get(); List manifests = new ArrayList<>(); - for (String longPath : Lists.reverse(params.getManifestList())) { + for (String longPath : params.getManifestList()) { Path manifestPath = runfiles.getPath(longPath); manifestPaths.add(manifestPath); Webfiles.Builder builder = Webfiles.newBuilder(); @@ -164,11 +164,14 @@ void loadMetadataIntoObjectGraph() throws IOException { for (AssetInfo asset : params.getExternalAssetList()) { assets.put(Webpath.get(asset.getWebpath()), runfiles.getPath(asset.getPath())); } - Set webpaths = new LinkedHashSet<>(); for (Webfiles manifest : manifests) { for (WebfilesSource src : manifest.getSrcList()) { - Webpath webpath = Webpath.get(src.getWebpath()); - assets.put(webpath, runfiles.getPath(src.getLongpath())); + assets.put(Webpath.get(src.getWebpath()), runfiles.getPath(src.getLongpath())); + } + } + Set webpaths = new LinkedHashSet<>(); + for (Webfiles manifest : Lists.reverse(manifests)) { + for (WebfilesSource src : manifest.getSrcList()) { webpaths.add(Webpath.get(src.getWebpath())); } }