Skip to content

Commit

Permalink
Flip and remove incompatible_dont_collect_so_artifacts flag.
Browse files Browse the repository at this point in the history
Downstream tests were run https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1911#b7a47544-7f5a-440b-ab15-c64c8b264bb8
The code for those tests was modified to throw a failure on each uncollected .so library. In all cases the library did not to appear on -Djava.library.path.

RELNOTES[INC]: Flip and remove incompatible_dont_collect_so_artifacts (#13043).

PiperOrigin-RevId: 364973576
  • Loading branch information
comius authored and Copybara-Service committed Mar 25, 2021
1 parent c981413 commit 144a62a
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
Lists.newArrayList(common.targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY));
helper.addLibrariesToAttributes(deps);
attributesBuilder.addNativeLibraries(
collectNativeLibraries(ruleContext, common.targetsTreatedAsDeps(ClasspathType.BOTH)));
collectNativeLibraries(common.targetsTreatedAsDeps(ClasspathType.BOTH)));

// deploy_env is valid for java_binary, but not for java_test.
if (ruleContext.getRule().isAttrDefined("deploy_env", BuildType.LABEL_LIST)) {
Expand Down Expand Up @@ -677,9 +677,9 @@ private void collectDefaultRunfiles(
* @return the native libraries found in the transitive closure of the deps.
*/
public static Collection<Artifact> collectNativeLibraries(
RuleContext ruleContext, Iterable<? extends TransitiveInfoCollection> deps) {
Iterable<? extends TransitiveInfoCollection> deps) {
NestedSet<LibraryToLink> linkerInputs =
new NativeLibraryNestedSetBuilder(ruleContext).addJavaTargets(deps).build();
new NativeLibraryNestedSetBuilder().addJavaTargets(deps).build();
return LibraryToLink.getDynamicLibrariesForLinking(linkerInputs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public static void checkRuntimeDeps(
* @see JavaNativeLibraryInfo
*/
protected NestedSet<LibraryToLink> collectTransitiveJavaNativeLibraries() {
NativeLibraryNestedSetBuilder builder = new NativeLibraryNestedSetBuilder(ruleContext);
NativeLibraryNestedSetBuilder builder = new NativeLibraryNestedSetBuilder();
builder.addJavaTargets(targetsTreatedAsDeps(ClasspathType.BOTH));

if (ruleContext.getRule().isAttrDefined("data", BuildType.LABEL_LIST)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ public enum ImportDepsCheckingLevel {
private final boolean disallowResourceJars;
private final boolean disallowLegacyJavaToolchainFlags;
private final boolean experimentalTurbineAnnotationProcessing;
private final boolean dontCollectSoArtifacts;

// TODO(dmarting): remove once we have a proper solution for #2539
private final boolean useLegacyBazelJavaTest;
Expand Down Expand Up @@ -151,7 +150,6 @@ public JavaConfiguration(BuildOptions buildOptions) throws InvalidConfigurationE
this.addTestSupportToCompileTimeDeps = javaOptions.addTestSupportToCompileTimeDeps;
this.runAndroidLint = javaOptions.runAndroidLint;
this.limitAndroidLintToAndroidCompatible = javaOptions.limitAndroidLintToAndroidCompatible;
this.dontCollectSoArtifacts = javaOptions.dontCollectSoArtifacts;

ImmutableList.Builder<Label> translationsBuilder = ImmutableList.builder();
for (String s : javaOptions.translationTargets) {
Expand Down Expand Up @@ -371,7 +369,6 @@ public boolean useLegacyBazelJavaTest() {
return useLegacyBazelJavaTest;
}


/**
* Make it mandatory for java_test targets to explicitly declare any JUnit or Hamcrest
* dependencies instead of accidentally obtaining them from the TestRunner's dependencies.
Expand Down Expand Up @@ -439,8 +436,4 @@ public boolean disallowResourceJars() {
public boolean experimentalTurbineAnnotationProcessing() {
return experimentalTurbineAnnotationProcessing;
}

public boolean dontCollectSoArtifacts() {
return dontCollectSoArtifacts;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ public ImportDepsCheckingLevelConverter() {
help = "The Java language version used to execute the tools that are needed during a build")
public String hostJavaLanguageVersion;

// TODO(b/180107817): delete flag after removing from global .blazerc
@Option(
name = "incompatible_dont_collect_so_artifacts",
defaultValue = "false",
Expand All @@ -659,9 +660,7 @@ public ImportDepsCheckingLevelConverter() {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"Disables collection of .so libraries as artifact (produced by filegroup or genrule); "
+ " depend on cc_binary or cc_library directly.")
help = "This flag is a noop and scheduled for removal.")
public boolean dontCollectSoArtifacts;

Label defaultJavaBase() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,16 @@

package com.google.devtools.build.lib.rules.java;

import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.rules.cpp.CcLinkingOutputs;
import com.google.devtools.build.lib.rules.cpp.CcNativeLibraryProvider;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
import com.google.devtools.build.lib.util.FileType;

/** A builder that helps construct nested sets of native libraries. */
public final class NativeLibraryNestedSetBuilder {

private final NestedSetBuilder<LibraryToLink> builder = NestedSetBuilder.linkOrder();
private final RuleContext ruleContext;

public NativeLibraryNestedSetBuilder(RuleContext ruleContext) {
this.ruleContext = ruleContext;
}

/** Build a nested set of native libraries. */
public NestedSet<LibraryToLink> build() {
Expand Down Expand Up @@ -70,8 +59,6 @@ public NativeLibraryNestedSetBuilder addJavaTarget(TransitiveInfoCollection dep)
return this;
}

addTarget(dep);

return this;
}

Expand All @@ -89,25 +76,6 @@ private void addCcTarget(TransitiveInfoCollection dep) {
CcNativeLibraryProvider provider = dep.get(CcNativeLibraryProvider.PROVIDER);
if (provider != null) {
builder.addTransitive(provider.getTransitiveCcNativeLibraries());
} else {
addTarget(dep);
}
}

/** Include files and genrule artifacts. */
private void addTarget(TransitiveInfoCollection dep) {
if (ruleContext.getFragment(JavaConfiguration.class).dontCollectSoArtifacts()) {
return;
}
for (Artifact artifact :
FileType.filterList(
dep.getProvider(FileProvider.class).getFilesToBuild().toList(),
CppFileTypes.SHARED_LIBRARY)) {
builder.add(
LibraryToLink.builder()
.setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(artifact))
.setDynamicLibrary(artifact)
.build());
}
}
}

0 comments on commit 144a62a

Please sign in to comment.