Skip to content

Commit

Permalink
Use private native method to tokenize javacopts
Browse files Browse the repository at this point in the history
We use this only when we don't have a `ctx`, such as in an `args.map_each`. The pure Starlark implementation in `cc_helper.bzl`, as currently implemented, is too slow[^1].

PiperOrigin-RevId: 587638256
Change-Id: I4aa6b9c2009323e9c4adc7f6e8b5e335a637ed1c
  • Loading branch information
hvadehra authored and copybara-github committed Dec 4, 2023
1 parent 840f440 commit fb446e6
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ public Sequence<?> expandJavaOpts(
}
}

@Override
public Sequence<?> tokenizeJavacOpts(Sequence<?> opts) throws EvalException {
return StarlarkList.immutableCopyOf(
JavaHelper.tokenizeJavaOptions(Sequence.noneableCast(opts, String.class, "opts")));
}

static boolean isInstanceOfProvider(Object obj, Provider provider) {
if (obj instanceof NativeInfo) {
return ((NativeInfo) obj).getProvider().getKey().equals(provider.getKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -734,4 +734,10 @@ JavaInfoT wrapJavaInfo(Info javaInfo, StarlarkThread thread)
Sequence<?> expandJavaOpts(
StarlarkRuleContextT ctx, String attr, boolean tokenize, boolean execPaths)
throws EvalException, InterruptedException;

@StarlarkMethod(
name = "tokenize_javacopts",
documented = false,
parameters = {@Param(name = "opts")})
Sequence<?> tokenizeJavacOpts(Sequence<?> opts) throws EvalException, InterruptedException;
}
10 changes: 3 additions & 7 deletions src/main/starlark/builtins_bzl/common/java/java_helper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ load(":common/java/java_semantics.bzl", "semantics")
load(":common/paths.bzl", "paths")

testing = _builtins.toplevel.testing
_java_common_internal = _builtins.internal.java_common_internal_do_not_use

def _collect_all_targets_as_deps(ctx, classpath_type = "all"):
deps = []
Expand Down Expand Up @@ -402,13 +403,8 @@ def _tokenize_javacopts(ctx = None, opts = []):
for token in ctx.tokenize(opt)
]
else:
# slow, but pure Starlark implementation
result = []
for opt in opts:
tokens = []
cc_helper.tokenize(tokens, opt)
result.extend(tokens)
return result
# TODO: optimize and use the pure Starlark implementation in cc_helper
return _java_common_internal.tokenize_javacopts(opts)

def _detokenize_javacopts(opts):
"""Detokenizes a list of options to a depset.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3700,6 +3700,7 @@ public void testNoArgsPrivateAPIsAreIndeedPrivate(String module, String api) thr
"{api: _check_java_toolchain_is_declared_on_rule}",
"{api: wrap_java_info}",
"{api: intern_javac_opts}",
"{api: tokenize_javacopts}",
})
public void testJavaCommonPrivateApis_areNotVisibleToPublicStarlark(String api) throws Exception {
// validate that this api is present on the module, so this test fails when the API is deleted
Expand Down

0 comments on commit fb446e6

Please sign in to comment.