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 (bazelbuild/bazel#13043).

    PiperOrigin-RevId: 364973576
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 032d1b4 commit bc83c1f
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
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.rules.java.JavaConfiguration.OneVersionEnforcementLevel;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.OutputJar;
import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.Pair;
Expand Down Expand Up @@ -205,7 +204,10 @@ public ConfiguredTarget create(RuleContext ruleContext)
JavaRuleOutputJarsProvider.Builder ruleOutputJarsProviderBuilder =
JavaRuleOutputJarsProvider.builder()
.addOutputJar(
OutputJar.builder().fromJavaCompileOutputs(outputs).addSourceJar(srcJar).build());
/* classJar= */ classJar,
/* iJar= */ null,
/* manifestProto= */ outputs.manifestProto(),
/* sourceJars= */ ImmutableList.of(srcJar));

JavaTargetAttributes attributes = attributesBuilder.build();
List<Artifact> nativeLibraries = attributes.getNativeLibraries();
Expand Down Expand Up @@ -236,6 +238,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleOutputJarsProviderBuilder,
javaSourceJarsProviderBuilder);
javaArtifactsBuilder.setCompileTimeDependencies(outputs.depsProto());
ruleOutputJarsProviderBuilder.setJdeps(outputs.depsProto());

JavaCompilationArtifacts javaArtifacts = javaArtifactsBuilder.build();
common.setJavaCompilationArtifacts(javaArtifacts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,12 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.java;

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
Expand All @@ -44,11 +40,9 @@
import com.google.devtools.build.lib.packages.Target;
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.rules.java.JavaPluginInfo.JavaPluginData;
import com.google.devtools.build.lib.rules.java.JavaPluginInfoProvider.JavaPluginInfo;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
Expand All @@ -59,7 +53,6 @@
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;

/** A helper class to create configured targets for Java rules. */
Expand All @@ -79,7 +72,7 @@ public class JavaCommon {
targetsTreatedAsDeps;

private final ImmutableList<Artifact> sources;
private JavaPluginInfo activePlugins = JavaPluginInfo.empty();
private JavaPluginInfoProvider activePlugins = JavaPluginInfoProvider.empty();

private final RuleContext ruleContext;
private final JavaSemantics semantics;
Expand Down Expand Up @@ -696,62 +689,6 @@ public void addTransitiveInfoProviders(

javaInfoBuilder.addProvider(JavaExportsProvider.class, exportsProvider);
javaInfoBuilder.addProvider(JavaCompilationInfoProvider.class, compilationInfoProvider);

addCcRelatedProviders(builder, javaInfoBuilder);
}

/** Adds Cc related providers to a Java target. */
private void addCcRelatedProviders(
RuleConfiguredTargetBuilder ruleBuilder, JavaInfo.Builder javaInfoBuilder) {
Iterable<? extends TransitiveInfoCollection> deps = targetsTreatedAsDeps(ClasspathType.BOTH);


ImmutableList<CcInfo> ccInfos =
Streams.concat(
AnalysisUtils.getProviders(deps, CcInfo.PROVIDER).stream(),
AnalysisUtils.getProviders(deps, JavaCcLinkParamsProvider.PROVIDER).stream()
.map(JavaCcLinkParamsProvider::getCcInfo),
JavaInfo.getProvidersFromListOfTargets(JavaCcInfoProvider.class, deps).stream()
.map(JavaCcInfoProvider::getCcInfo))
.collect(toImmutableList());

CcInfo mergedCcInfo = CcInfo.merge(ccInfos);

// Collect library paths from all attributes (including data)
Iterable<? extends TransitiveInfoCollection> data;
if (ruleContext.getRule().isAttrDefined("data", BuildType.LABEL_LIST)) {
data = ruleContext.getPrerequisites("data");
} else {
data = ImmutableList.of();
}
CcNativeLibraryInfo mergedCcNativeLibraryInfo =
CcNativeLibraryInfo.merge(
Streams.concat(
Stream.of(mergedCcInfo.getCcNativeLibraryInfo()),
AnalysisUtils.getProviders(
Iterables.concat(deps, data), JavaNativeLibraryInfo.PROVIDER)
.stream()
.map(JavaNativeLibraryInfo::getTransitiveJavaNativeLibraries)
.map(CcNativeLibraryInfo::new),
JavaInfo.getProvidersFromListOfTargets(JavaCcInfoProvider.class, data).stream()
.map(JavaCcInfoProvider::getCcInfo)
.map(CcInfo::getCcNativeLibraryInfo),
AnalysisUtils.getProviders(data, CcInfo.PROVIDER).stream()
.map(CcInfo::getCcNativeLibraryInfo))
.collect(toImmutableList()));

CcInfo filteredCcInfo =
CcInfo.builder()
.setCcLinkingContext(mergedCcInfo.getCcLinkingContext())
.setCcNativeLibraryInfo(mergedCcNativeLibraryInfo)
.build();

if (ruleContext
.getFragment(JavaConfiguration.class)
.experimentalPublishJavaCcLinkParamsInfo()) {
ruleBuilder.addNativeDeclaredProvider(new JavaCcLinkParamsProvider(filteredCcInfo));
}
javaInfoBuilder.addProvider(JavaCcInfoProvider.class, new JavaCcInfoProvider(filteredCcInfo));
}

private InstrumentedFilesInfo getInstrumentationFilesProvider(
Expand Down Expand Up @@ -826,32 +763,28 @@ private void addPlugins(JavaTargetAttributes.Builder attributes) {
* added to the given attributes. Plugins having repetitive names/paths will be added only once.
*/
public static void addPlugins(
JavaTargetAttributes.Builder attributes, JavaPluginInfo activePlugins) {
JavaTargetAttributes.Builder attributes, JavaPluginInfoProvider activePlugins) {
attributes.addPlugin(activePlugins);
}

private JavaPluginInfo collectPlugins() {
List<JavaPluginInfo> result = new ArrayList<>();
Iterables.addAll(result, getJavaPluginInfoForAttribute(ruleContext, ":java_plugins"));
Iterables.addAll(result, getJavaPluginInfoForAttribute(ruleContext, "plugins"));
Iterables.addAll(result, getJavaPluginInfoForAttribute(ruleContext, "deps"));
return JavaPluginInfo.merge(result);
private JavaPluginInfoProvider collectPlugins() {
List<JavaPluginInfoProvider> result = new ArrayList<>();
Iterables.addAll(result, getPluginInfoProvidersForAttribute(ruleContext, ":java_plugins"));
Iterables.addAll(result, getPluginInfoProvidersForAttribute(ruleContext, "plugins"));
Iterables.addAll(result, getPluginInfoProvidersForAttribute(ruleContext, "deps"));
return JavaPluginInfoProvider.merge(result);
}

private static Iterable<JavaPluginInfo> getJavaPluginInfoForAttribute(
private static Iterable<JavaPluginInfoProvider> getPluginInfoProvidersForAttribute(
RuleContext ruleContext, String attribute) {
if (ruleContext.attributes().has(attribute, BuildType.LABEL_LIST)) {
return ruleContext.getPrerequisites(attribute).stream()
.map(JavaInfo::getJavaInfo)
.filter(Objects::nonNull)
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList());
return JavaInfo.getProvidersFromListOfTargets(
JavaPluginInfoProvider.class, ruleContext.getPrerequisites(attribute));
}
return ImmutableList.of();
}

JavaPluginInfo createJavaPluginInfo(RuleContext ruleContext) {
JavaPluginInfoProvider getJavaPluginInfoProvider(RuleContext ruleContext) {
NestedSet<String> processorClasses =
NestedSetBuilder.wrap(Order.NAIVE_LINK_ORDER, getProcessorClasses(ruleContext));
NestedSet<Artifact> processorClasspath = getRuntimeClasspath();
Expand All @@ -860,8 +793,8 @@ JavaPluginInfo createJavaPluginInfo(RuleContext ruleContext) {
dataProvider != null
? dataProvider.getFilesToBuild()
: NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
return JavaPluginInfo.create(
JavaPluginData.create(processorClasses, processorClasspath, data),
return JavaPluginInfoProvider.create(
JavaPluginInfo.create(processorClasses, processorClasspath, data),
ruleContext.attributes().get("generates_api", Type.BOOLEAN));
}

Expand All @@ -875,11 +808,11 @@ private static ImmutableSet<String> getProcessorClasses(RuleContext ruleContext)
: ImmutableSet.of();
}

public static JavaPluginInfo getTransitivePlugins(RuleContext ruleContext) {
return JavaPluginInfo.merge(
public static JavaPluginInfoProvider getTransitivePlugins(RuleContext ruleContext) {
return JavaPluginInfoProvider.merge(
Iterables.concat(
getJavaPluginInfoForAttribute(ruleContext, "exported_plugins"),
getJavaPluginInfoForAttribute(ruleContext, "exports")));
getPluginInfoProvidersForAttribute(ruleContext, "exported_plugins"),
getPluginInfoProvidersForAttribute(ruleContext, "exports")));
}

public static Runfiles getRunfiles(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.common.base.Ascii;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
Expand All @@ -26,8 +27,12 @@
import com.google.devtools.build.lib.analysis.config.RequiresOptions;
import com.google.devtools.build.lib.analysis.starlark.annotations.StarlarkConfigurationField;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.starlarkbuildapi.java.JavaConfigurationApi;
import com.google.devtools.common.options.TriState;
import java.util.Map;
import java.util.Objects;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -91,6 +96,8 @@ public enum ImportDepsCheckingLevel {
private final String fixDepsTool;
private final Label proguardBinary;
private final ImmutableList<Label> extraProguardSpecs;
private final TriState bundleTranslations;
private final ImmutableList<Label> translationTargets;
private final NamedLabel bytecodeOptimizer;
private final boolean splitBytecodeOptimizationPass;
private final boolean enforceProguardFileExtension;
Expand Down Expand Up @@ -127,6 +134,7 @@ public JavaConfiguration(BuildOptions buildOptions) throws InvalidConfigurationE
this.extraProguardSpecs = ImmutableList.copyOf(javaOptions.extraProguardSpecs);
this.splitBytecodeOptimizationPass = javaOptions.splitBytecodeOptimizationPass;
this.enforceProguardFileExtension = javaOptions.enforceProguardFileExtension;
this.bundleTranslations = javaOptions.bundleTranslations;
this.toolchainLabel = javaOptions.javaToolchain;
this.runtimeLabel = javaOptions.javaBase;
this.useLegacyBazelJavaTest = javaOptions.legacyBazelJavaTest;
Expand All @@ -143,6 +151,22 @@ public JavaConfiguration(BuildOptions buildOptions) throws InvalidConfigurationE
this.runAndroidLint = javaOptions.runAndroidLint;
this.limitAndroidLintToAndroidCompatible = javaOptions.limitAndroidLintToAndroidCompatible;

ImmutableList.Builder<Label> translationsBuilder = ImmutableList.builder();
for (String s : javaOptions.translationTargets) {
try {
Label label = Label.parseAbsolute(s, ImmutableMap.of());
translationsBuilder.add(label);
} catch (LabelSyntaxException e) {
throw new InvalidConfigurationException(
"Invalid translations target '"
+ s
+ "', make "
+ "sure it uses correct absolute path syntax.",
e);
}
}
this.translationTargets = translationsBuilder.build();

Map<String, Label> optimizers = javaOptions.bytecodeOptimizers;
if (optimizers.size() > 1) {
throw new InvalidConfigurationException(
Expand Down Expand Up @@ -197,6 +221,16 @@ public String getStrictJavaDepsName() {
return Ascii.toLowerCase(strictJavaDeps.name());
}

@Override
public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
if ((bundleTranslations == TriState.YES) && translationTargets.isEmpty()) {
reporter.handle(
Event.error(
"Translations enabled, but no message translations specified. "
+ "Use '--message_translations' to select the message translations to use"));
}
}

/** Returns true iff Java compilation should use ijars. */
public boolean getUseIjars() {
return useIjars;
Expand Down Expand Up @@ -277,6 +311,21 @@ public boolean enforceProguardFileExtension() {
return enforceProguardFileExtension;
}

/** Returns the raw translation targets. */
public ImmutableList<Label> getTranslationTargets() {
return translationTargets;
}

/** Returns true if the we should build translations. */
public boolean buildTranslations() {
return (bundleTranslations != TriState.NO) && !translationTargets.isEmpty();
}

/** Returns whether translations were explicitly disabled. */
public boolean isTranslationsDisabled() {
return bundleTranslations == TriState.NO;
}

/** Returns the label of the default java_toolchain rule */
@StarlarkConfigurationField(
name = "java_toolchain",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.TriState;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -426,6 +427,25 @@ public ImportDepsCheckingLevelConverter() {
+ " *.pgcfg file extension.")
public boolean enforceProguardFileExtension;

@Option(
name = "translations",
defaultValue = "auto",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Translate Java messages; bundle all translations into the jar "
+ "for each affected rule.")
public TriState bundleTranslations;

@Option(
name = "message_translations",
defaultValue = "null",
allowMultiple = true,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "The message translations used for translating messages in Java targets.")
public List<String> translationTargets;

@Deprecated
@Option(
name = "check_constraint",
Expand Down Expand Up @@ -727,4 +747,5 @@ public FragmentOptions getHost() {

return host;
}

}
Loading

0 comments on commit bc83c1f

Please sign in to comment.