Skip to content

Commit

Permalink
Inline the only usage of and delete JavaCommon.collectNativeLibraries
Browse files Browse the repository at this point in the history
Also avoids creating a potentially large list of artifacts by computing the set of directories in the same stream that filters for shared libraries[^1]

PiperOrigin-RevId: 546081142
Change-Id: I613df2d0e5b47deae12e0f771b6b96e8d950d539
  • Loading branch information
hvadehra authored and copybara-github committed Jul 6, 2023
1 parent 3e800a7 commit 27d158b
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CcNativeLibraryInfo;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Pair;
Expand Down Expand Up @@ -112,27 +110,6 @@ public JavaCommon(
ClasspathType.BOTH, bothDeps);
}

/**
* Collects the native libraries in the transitive closure of the deps.
*
* @param deps the dependencies to be included as roots of the transitive closure.
* @return the native libraries found in the transitive closure of the deps.
*/
public static ImmutableList<Artifact> collectNativeLibraries(
Collection<? extends TransitiveInfoCollection> deps) throws RuleErrorException {
NestedSet<LibraryToLink> linkerInputs =
NestedSetBuilder.fromNestedSets(
Streams.concat(
JavaInfo.transitiveCcNativeLibraries(deps).stream(),
AnalysisUtils.getProviders(deps, CcInfo.PROVIDER).stream()
.map(CcInfo::getCcNativeLibraryInfo)
.map(CcNativeLibraryInfo::getTransitiveCcNativeLibraries))
.collect(toImmutableList()))
.build();

return LibraryToLink.getDynamicLibrariesForLinking(linkerInputs);
}

public JavaSemantics getJavaSemantics() {
return semantics;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.configuredtargets.AbstractConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
Expand All @@ -42,13 +41,11 @@
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.TransitiveInfoCollectionApi;
import com.google.devtools.build.lib.starlarkbuildapi.java.JavaCommonApi;
import com.google.devtools.build.lib.starlarkbuildapi.java.JavaToolchainStarlarkApiProviderApi;
import java.util.LinkedHashSet;
import java.util.Objects;
import java.util.Set;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Module;
import net.starlark.java.eval.Sequence;
Expand Down Expand Up @@ -294,14 +291,13 @@ public boolean getExperimentalJavaProtoLibraryDefaultHasServices(
}

@Override
public Sequence<String> collectNativeLibsDirs(
Sequence<? extends TransitiveInfoCollectionApi> deps, StarlarkThread thread)
throws EvalException, RuleErrorException {
public Sequence<String> collectNativeLibsDirs(Depset libraries, StarlarkThread thread)
throws EvalException, TypeException {
checkPrivateAccess(thread);
ImmutableList<Artifact> nativeLibs =
JavaCommon.collectNativeLibraries(
Sequence.cast(deps, TransitiveInfoCollection.class, "deps"))
.stream()
ImmutableList<Artifact> nativeLibraries =
LibraryToLink.getDynamicLibrariesForLinking(libraries.getSet(LibraryToLink.class));
ImmutableList<String> uniqueDirs =
nativeLibraries.stream()
.filter(
nativeLibrary -> {
String name = nativeLibrary.getFilename();
Expand All @@ -315,12 +311,9 @@ public Sequence<String> collectNativeLibsDirs(
}
return true;
})
.map(artifact -> artifact.getRootRelativePath().getParentDirectory().getPathString())
.distinct()
.collect(toImmutableList());

Set<String> uniqueDirs = new LinkedHashSet<>();
for (Artifact nativeLib : nativeLibs) {
uniqueDirs.add(nativeLib.getRootRelativePath().getParentDirectory().getPathString());
}
return StarlarkList.immutableCopyOf(uniqueDirs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkActionFactoryApi;
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkRuleContextApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.TransitiveInfoCollectionApi;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcInfoApi;
import com.google.devtools.build.lib.starlarkbuildapi.platform.ConstraintValueInfoApi;
import net.starlark.java.annot.Param;
Expand Down Expand Up @@ -427,12 +426,11 @@ boolean getExperimentalJavaProtoLibraryDefaultHasServices(StarlarkSemantics star

@StarlarkMethod(
name = "collect_native_deps_dirs",
parameters = {@Param(name = "deps")},
parameters = {@Param(name = "libraries")},
useStarlarkThread = true,
documented = false)
Sequence<String> collectNativeLibsDirs(
Sequence<? extends TransitiveInfoCollectionApi> deps, StarlarkThread thread)
throws EvalException, RuleErrorException;
Sequence<String> collectNativeLibsDirs(Depset libraries, StarlarkThread thread)
throws EvalException, RuleErrorException, TypeException;

@StarlarkMethod(
name = "get_runtime_classpath_for_archive",
Expand Down
8 changes: 7 additions & 1 deletion src/main/starlark/builtins_bzl/common/java/java_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,13 @@ def basic_java_binary(

jvm_flags.extend(launcher_info.jvm_flags)

native_libs_dirs = collect_native_deps_dirs(runtime_deps)
native_libs_depsets = []
for dep in runtime_deps:
if JavaInfo in dep:
native_libs_depsets.append(dep[JavaInfo].transitive_native_libraries)
if CcInfo in dep:
native_libs_depsets.append(dep[CcInfo].transitive_native_libraries())
native_libs_dirs = collect_native_deps_dirs(depset(transitive = native_libs_depsets))
if native_libs_dirs:
prefix = "${JAVA_RUNFILES}/" + ctx.workspace_name + "/"
jvm_flags.append("-Djava.library.path=%s" % (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,16 +204,16 @@ def get_build_info(ctx, is_stamping_enabled):
"""
return _java_common_internal.get_build_info(ctx, is_stamping_enabled)

def collect_native_deps_dirs(deps):
def collect_native_deps_dirs(libraries):
"""Collect the set of root-relative paths containing native libraries
Args:
deps: [Target] list of targets
libraries: (depset[LibraryToLink]) set of native libraries
Returns:
([String]) A set of root-relative paths as a list
"""
return _java_common_internal.collect_native_deps_dirs(deps)
return _java_common_internal.collect_native_deps_dirs(libraries)

def get_runtime_classpath_for_archive(jars, excluded_jars):
"""Filters a classpath to remove certain entries
Expand Down

0 comments on commit 27d158b

Please sign in to comment.