Skip to content

Commit

Permalink
Use bazel's native json decode method
Browse files Browse the repository at this point in the history
This means that `rules_jvm_external` is only compatible with
Bazel 4 and later. That shipped on 2021-01-21, and so should
be widely adopted already. Older versions of this ruleset
will continue to work for repos using older versions of Bazel
so it's not removing any existing functionality, and the new
Bazel 5 release should have already shipped.

The main reason for doing this is that the native json
decoding is significantly faster than the starlark-based one.

The following options were explored as alternatives:

1. Making the `json_parse` method check the bazel version and
   optionally use the `json` module if it was present. This
   doesn't work because of #12735

2. Create a `json` repository rule that creates a repo that
   exposes a `json.bzl` file that delegates to the native
   `json` module if present, or the starlark one if not
   (as determined by looking at the major version number of
   bazel). This doesn't work because `maven_install` is used
   by RJE's own `repositories.bzl` file, so there's no chance
   to load the repository rule before it's needed.

Links:

12735: bazelbuild/bazel#12735
  • Loading branch information
shs96c committed Jan 28, 2022
1 parent 906875b commit 3e10de6
Show file tree
Hide file tree
Showing 11 changed files with 13 additions and 2,513 deletions.
1 change: 0 additions & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ bzl_library(
"//private/rules:maven_publish.bzl",
"//private/rules:pom_file.bzl",
"//settings:stamp_manifest.bzl",
"//third_party/bazel_json/lib:json_parser.bzl",
],
visibility = [
# This library is only visible to allow others who depend on
Expand Down
29 changes: 9 additions & 20 deletions coursier.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and

load("//third_party/bazel_json/lib:json_parser.bzl", _json_parse = "json_parse")
load("//private/rules:jetifier.bzl", "jetify_artifact_dependencies", "jetify_maven_coord")
load("//:specs.bzl", "parse", "utils")
load("//private:artifact_utilities.bzl", "deduplicate_and_sort_artifacts")
Expand Down Expand Up @@ -84,15 +83,6 @@ sh_binary(
)
"""

def json_parse(json_string, fail_on_invalid = True):
# Bazel has had a native JSON decoder since 4.0.0, but
# we need to support older versions of Bazel. In addition,
# we sometimes need to survive poorly formed JSON, so
# a fallback to the Starlark parser is provided.
if getattr(native, "json_decode", None) and not fail_on_invalid:
return native.json_decode(json_string)
return _json_parse(json_string, fail_on_invalid)

def _is_verbose(repository_ctx):
return bool(repository_ctx.os.environ.get("RJE_VERBOSE"))

Expand Down Expand Up @@ -243,7 +233,7 @@ def _compute_dependency_tree_signature(artifacts):
def compute_dependency_inputs_signature(artifacts):
artifact_inputs = []
for artifact in artifacts:
parsed = json_parse(artifact)
parsed = json.decode(artifact)

# Sort the keys to provide a stable order
keys = sorted(parsed.keys())
Expand Down Expand Up @@ -385,11 +375,11 @@ def _pinned_coursier_fetch_impl(repository_ctx):

repositories = []
for repository in repository_ctx.attr.repositories:
repositories.append(json_parse(repository))
repositories.append(json.decode(repository))

artifacts = []
for artifact in repository_ctx.attr.artifacts:
artifacts.append(json_parse(artifact))
artifacts.append(json.decode(artifact))

_check_artifacts_are_unique(artifacts, repository_ctx.attr.duplicate_version_warning)

Expand All @@ -398,11 +388,10 @@ def _pinned_coursier_fetch_impl(repository_ctx):
repository_ctx.path(repository_ctx.attr.maven_install_json),
repository_ctx.path("imported_maven_install.json"),
)
maven_install_json_content = json_parse(
maven_install_json_content = json.decode(
repository_ctx.read(
repository_ctx.path("imported_maven_install.json"),
),
fail_on_invalid = False,
)
)

# Validation steps for maven_install.json.
Expand Down Expand Up @@ -783,7 +772,7 @@ def make_coursier_dep_tree(
fail("Error while fetching artifact with coursier: " + exec_result.stderr)

return deduplicate_and_sort_artifacts(
json_parse(repository_ctx.read(repository_ctx.path("dep-tree.json"))),
json.decode(repository_ctx.read(repository_ctx.path("dep-tree.json"))),
artifacts,
excluded_artifacts,
_is_verbose(repository_ctx),
Expand Down Expand Up @@ -831,17 +820,17 @@ def _coursier_fetch_impl(repository_ctx):
# Deserialize the spec blobs
repositories = []
for repository in repository_ctx.attr.repositories:
repositories.append(json_parse(repository))
repositories.append(json.decode(repository))

artifacts = []
for artifact in repository_ctx.attr.artifacts:
artifacts.append(json_parse(artifact))
artifacts.append(json.decode(artifact))

_check_artifacts_are_unique(artifacts, repository_ctx.attr.duplicate_version_warning)

excluded_artifacts = []
for artifact in repository_ctx.attr.excluded_artifacts:
excluded_artifacts.append(json_parse(artifact))
excluded_artifacts.append(json.decode(artifact))

# Once coursier finishes a fetch, it generates a tree of artifacts and their
# transitive dependencies in a JSON file. We use that as the source of truth
Expand Down
8 changes: 4 additions & 4 deletions private/rules/maven_install.bzl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
load("//:coursier.bzl", "DEFAULT_AAR_IMPORT_LABEL", "coursier_fetch", "pinned_coursier_fetch")
load("//:specs.bzl", "json", "parse")
load("//:specs.bzl", _json = "json", "parse")
load("//private:constants.bzl", "DEFAULT_REPOSITORY_NAME")
load("//private:dependency_tree_parser.bzl", "JETIFY_INCLUDE_LIST_JETIFY_ALL")

Expand Down Expand Up @@ -77,15 +77,15 @@ def maven_install(
"""
repositories_json_strings = []
for repository in parse.parse_repository_spec_list(repositories):
repositories_json_strings.append(json.write_repository_spec(repository))
repositories_json_strings.append(_json.write_repository_spec(repository))

artifacts_json_strings = []
for artifact in parse.parse_artifact_spec_list(artifacts):
artifacts_json_strings.append(json.write_artifact_spec(artifact))
artifacts_json_strings.append(_json.write_artifact_spec(artifact))

excluded_artifacts_json_strings = []
for exclusion in parse.parse_exclusion_spec_list(excluded_artifacts):
excluded_artifacts_json_strings.append(json.write_exclusion_spec(exclusion))
excluded_artifacts_json_strings.append(_json.write_exclusion_spec(exclusion))

if additional_netrc_lines and maven_install_json == None:
fail("`additional_netrc_lines` is only supported with `maven_install_json` specified", "additional_netrc_lines")
Expand Down
20 changes: 0 additions & 20 deletions third_party/bazel_json/LICENSE

This file was deleted.

5 changes: 0 additions & 5 deletions third_party/bazel_json/README.md

This file was deleted.

5 changes: 0 additions & 5 deletions third_party/bazel_json/lib/BUILD

This file was deleted.

Loading

0 comments on commit 3e10de6

Please sign in to comment.