diff options
8 files changed, 154 insertions, 94 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java index 6f2fbff664..f6573077c7 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java @@ -21,7 +21,9 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; 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.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; @@ -42,9 +44,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.TargetUtils; -import com.google.devtools.build.lib.rules.cpp.AbstractCcLinkParamsStore; -import com.google.devtools.build.lib.rules.cpp.CcLinkParams; -import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore; import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression; @@ -562,20 +561,20 @@ public class BazelJavaSemantics implements JavaSemantics { Artifact gensrcJar, RuleConfiguredTargetBuilder ruleBuilder) { // TODO(plf): Figure out whether we can remove support for C++ dependencies in Bazel. - CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create(); - ccLinkingInfoBuilder.setCcLinkParamsStore( - new CcLinkParamsStore( - new AbstractCcLinkParamsStore() { - @Override - protected void collect( - CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) { - builder.addTransitiveTargets( - javaCommon.targetsTreatedAsDeps(ClasspathType.BOTH), - JavaCcLinkParamsProvider.TO_LINK_PARAMS, - CcLinkParamsStore.TO_LINK_PARAMS); - } - })); - ruleBuilder.addNativeDeclaredProvider(ccLinkingInfoBuilder.build()); + ImmutableList<? extends TransitiveInfoCollection> deps = + javaCommon.targetsTreatedAsDeps(ClasspathType.BOTH); + ImmutableList<CcLinkingInfo> ccLinkingInfos = + ImmutableList.<CcLinkingInfo>builder() + .addAll(AnalysisUtils.getProviders(deps, CcLinkingInfo.PROVIDER)) + .addAll( + Streams.stream(AnalysisUtils.getProviders(deps, JavaCcLinkParamsProvider.class)) + .map(JavaCcLinkParamsProvider::getCcLinkingInfo) + .collect(ImmutableList.toImmutableList())) + .build(); + + // TODO(plf): return empty CcLinkingInfo because deps= in Java targets should not contain C++ + // targets. We need to make sure that no one uses this functionality, though. + ruleBuilder.addNativeDeclaredProvider(CcLinkingInfo.merge(ccLinkingInfos)); } // TODO(dmarting): simplify that logic when we remove the legacy Bazel java_test behavior. diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java index 51fe643793..9a71ae9f55 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java @@ -18,9 +18,11 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Streams; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile; +import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; @@ -40,9 +42,6 @@ import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.In import com.google.devtools.build.lib.cmdline.Label; 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.AbstractCcLinkParamsStore; -import com.google.devtools.build.lib.rules.cpp.CcLinkParams; -import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore; import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo; import com.google.devtools.build.lib.rules.python.PyCcLinkParamsProvider; import com.google.devtools.build.lib.rules.python.PyCommon; @@ -365,18 +364,16 @@ public class BazelPythonSemantics implements PythonSemantics { @Override public CcLinkingInfo buildCcLinkingInfoProvider( Iterable<? extends TransitiveInfoCollection> deps) { - CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create(); - AbstractCcLinkParamsStore ccLinkParamsStore = - new AbstractCcLinkParamsStore() { - @Override - protected void collect( - CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) { - builder.addTransitiveTargets( - deps, CcLinkParamsStore.TO_LINK_PARAMS, PyCcLinkParamsProvider.TO_LINK_PARAMS); - } - }; + ImmutableList<CcLinkingInfo> ccLinkingInfos = + ImmutableList.<CcLinkingInfo>builder() + .addAll(AnalysisUtils.getProviders(deps, CcLinkingInfo.PROVIDER)) + .addAll( + Streams.stream(AnalysisUtils.getProviders(deps, PyCcLinkParamsProvider.PROVIDER)) + .map(PyCcLinkParamsProvider::getCcLinkingInfo) + .collect(ImmutableList.toImmutableList())) + .build(); + // TODO(plf): return empty CcLinkingInfo. - ccLinkingInfoBuilder.setCcLinkParamsStore(new CcLinkParamsStore(ccLinkParamsStore)); - return ccLinkingInfoBuilder.build(); + return CcLinkingInfo.merge(ccLinkingInfos); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java index 5703def576..af205eb4d9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.android; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.analysis.AnalysisUtils; @@ -45,9 +46,8 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.android.ZipFilterBuilder.CheckHashMismatchMode; -import com.google.devtools.build.lib.rules.cpp.AbstractCcLinkParamsStore; import com.google.devtools.build.lib.rules.cpp.CcLinkParams; -import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore; +import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo; import com.google.devtools.build.lib.rules.java.ClasspathConfiguredFragment; import com.google.devtools.build.lib.rules.java.JavaCcLinkParamsProvider; import com.google.devtools.build.lib.rules.java.JavaCommon; @@ -795,28 +795,39 @@ public class AndroidCommon { return asNeverLink; } - public AbstractCcLinkParamsStore getCcLinkParamsStore() { - return getCcLinkParamsStore( + public CcLinkingInfo getCcLinkingInfo() { + return getCcLinkingInfo( javaCommon.targetsTreatedAsDeps(ClasspathType.BOTH), ImmutableList.<String>of()); } - public static AbstractCcLinkParamsStore getCcLinkParamsStore( + public static CcLinkingInfo getCcLinkingInfo( final Iterable<? extends TransitiveInfoCollection> deps, final Collection<String> linkOpts) { - return new AbstractCcLinkParamsStore() { - @Override - protected void collect( - CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) { - builder.addTransitiveTargets( - deps, - // Link in Java-specific C++ code in the transitive closure - JavaCcLinkParamsProvider.TO_LINK_PARAMS, - // Link in Android-specific C++ code (e.g., android_libraries) in the transitive closure - AndroidCcLinkParamsProvider.TO_LINK_PARAMS, - // Link in non-language-specific C++ code in the transitive closure - CcLinkParamsStore.TO_LINK_PARAMS); - builder.addLinkOpts(linkOpts); - } - }; + + CcLinkParams linkOptsParams = CcLinkParams.builder().addLinkOpts(linkOpts).build(); + CcLinkingInfo linkOptsProvider = + CcLinkingInfo.Builder.create() + .setStaticModeParamsForDynamicLibrary(linkOptsParams) + .setStaticModeParamsForExecutable(linkOptsParams) + .setDynamicModeParamsForDynamicLibrary(linkOptsParams) + .setDynamicModeParamsForExecutable(linkOptsParams) + .build(); + + ImmutableList<CcLinkingInfo> ccLinkingInfos = + ImmutableList.<CcLinkingInfo>builder() + .add(linkOptsProvider) + .addAll( + Streams.stream(AnalysisUtils.getProviders(deps, JavaCcLinkParamsProvider.class)) + .map(JavaCcLinkParamsProvider::getCcLinkingInfo) + .collect(ImmutableList.toImmutableList())) + .addAll( + Streams.stream( + AnalysisUtils.getProviders(deps, AndroidCcLinkParamsProvider.PROVIDER)) + .map(AndroidCcLinkParamsProvider::getLinkParams) + .collect(ImmutableList.toImmutableList())) + .addAll(AnalysisUtils.getProviders(deps, CcLinkingInfo.PROVIDER)) + .build(); + + return CcLinkingInfo.merge(ccLinkingInfos); } /** Returns {@link AndroidConfiguration} in given context. */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java index 19262c7ce8..50d69dc9ab 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java @@ -30,8 +30,6 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; import com.google.devtools.build.lib.rules.android.AndroidLibraryAarInfo.Aar; -import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore; -import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo; import com.google.devtools.build.lib.rules.java.JavaCommon; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaSourceInfoProvider; @@ -246,12 +244,7 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory { .add( JavaSourceInfoProvider.class, JavaSourceInfoProvider.fromJavaTargetAttributes(javaTargetAttributes, javaSemantics)) - .addNativeDeclaredProvider( - new AndroidCcLinkParamsProvider( - CcLinkingInfo.Builder.create() - .setCcLinkParamsStore( - new CcLinkParamsStore(androidCommon.getCcLinkParamsStore())) - .build())) + .addNativeDeclaredProvider(androidCommon.getCcLinkingInfo()) .addNativeDeclaredProvider(new ProguardSpecProvider(transitiveProguardConfigs)) .addNativeDeclaredProvider( new AndroidProguardInfo(proguardLibrary.collectLocalProguardSpecs())) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java index 6bea350871..0df0d333b2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java @@ -68,10 +68,10 @@ public final class NativeLibs { for (Map.Entry<String, Collection<TransitiveInfoCollection>> entry : getSplitDepsByArchitecture(ruleContext, depsAttributes).asMap().entrySet()) { CcLinkParams linkParams = - AndroidCommon.getCcLinkParamsStore( + AndroidCommon.getCcLinkingInfo( entry.getValue(), ImmutableList.of("-Wl,-soname=lib" + ruleContext.getLabel().getName())) - .get(/* linkingStatically */ true, /* linkShared */ true); + .getStaticModeParamsForDynamicLibrary(); Artifact nativeDepsLibrary = NativeDepsHelper.linkAndroidNativeDepsIfPresent( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingInfo.java index a8a27515af..483575accf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingInfo.java @@ -86,22 +86,19 @@ public final class CcLinkingInfo extends NativeInfo implements CcLinkingInfoApi CcLinkParams dynamicModeParamsForExecutable = (CcLinkParams) nullIfNone(args[i++]); CcRunfiles ccRunfiles = (CcRunfiles) nullIfNone(args[i++]); CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create(); - if (staticModeParamsForDynamicLibrary != null) { - if (staticModeParamsForExecutable == null - || dynamicModeParamsForDynamicLibrary == null - || dynamicModeParamsForExecutable == null) { - throw new EvalException( - loc, - "Every CcLinkParams parameter must be passed to CcLinkingInfo " - + "if one of them is passed."); - } - ccLinkingInfoBuilder.setCcLinkParamsStore( - new CcLinkParamsStore( - staticModeParamsForDynamicLibrary, - staticModeParamsForExecutable, - dynamicModeParamsForDynamicLibrary, - dynamicModeParamsForExecutable)); + if (staticModeParamsForDynamicLibrary == null + || staticModeParamsForExecutable == null + || dynamicModeParamsForDynamicLibrary == null + || dynamicModeParamsForExecutable == null) { + throw new EvalException( + loc, "Every CcLinkParams parameter must be passed to CcLinkingInfo."); } + ccLinkingInfoBuilder.setCcLinkParamsStore( + new CcLinkParamsStore( + staticModeParamsForDynamicLibrary, + staticModeParamsForExecutable, + dynamicModeParamsForDynamicLibrary, + dynamicModeParamsForExecutable)); // TODO(plf): The CcDynamicLibrariesForRuntime provider can be removed perhaps. Do not // add to the API until we know for sure. The CcRunfiles provider is already in the API // at the time of this comment (cl/200184914). Perhaps it can be removed but Skylark rules @@ -187,6 +184,10 @@ public final class CcLinkingInfo extends NativeInfo implements CcLinkingInfoApi /** A Builder for {@link CcLinkingInfo}. */ public static class Builder { CcLinkParamsStore ccLinkParamsStore; + CcLinkParams staticModeParamsForDynamicLibrary; + CcLinkParams staticModeParamsForExecutable; + CcLinkParams dynamicModeParamsForDynamicLibrary; + CcLinkParams dynamicModeParamsForExecutable; CcRunfiles ccRunfiles; CcDynamicLibrariesForRuntime ccDynamicLibrariesForRuntime; @@ -194,8 +195,15 @@ public final class CcLinkingInfo extends NativeInfo implements CcLinkingInfoApi return new CcLinkingInfo.Builder(); } + @Deprecated + // TODO(b/111781390): Use individual setters for each flavor of CcLinkParams. Not all call sites + // are being refactored at once. Work in progress. public Builder setCcLinkParamsStore(CcLinkParamsStore ccLinkParamsStore) { Preconditions.checkState(this.ccLinkParamsStore == null); + Preconditions.checkState(this.staticModeParamsForDynamicLibrary == null); + Preconditions.checkState(this.staticModeParamsForExecutable == null); + Preconditions.checkState(this.dynamicModeParamsForDynamicLibrary == null); + Preconditions.checkState(this.dynamicModeParamsForExecutable == null); this.ccLinkParamsStore = ccLinkParamsStore; return this; } @@ -213,7 +221,47 @@ public final class CcLinkingInfo extends NativeInfo implements CcLinkingInfoApi return this; } + public Builder setStaticModeParamsForDynamicLibrary(CcLinkParams ccLinkParams) { + Preconditions.checkState( + this.staticModeParamsForDynamicLibrary == null && ccLinkParamsStore == null); + this.staticModeParamsForDynamicLibrary = ccLinkParams; + return this; + } + + public Builder setStaticModeParamsForExecutable(CcLinkParams ccLinkParams) { + Preconditions.checkState( + this.staticModeParamsForExecutable == null && ccLinkParamsStore == null); + this.staticModeParamsForExecutable = ccLinkParams; + return this; + } + + public Builder setDynamicModeParamsForDynamicLibrary(CcLinkParams ccLinkParams) { + Preconditions.checkState( + this.dynamicModeParamsForDynamicLibrary == null && ccLinkParamsStore == null); + this.dynamicModeParamsForDynamicLibrary = ccLinkParams; + return this; + } + + public Builder setDynamicModeParamsForExecutable(CcLinkParams ccLinkParams) { + Preconditions.checkState( + this.dynamicModeParamsForExecutable == null && ccLinkParamsStore == null); + this.dynamicModeParamsForExecutable = ccLinkParams; + return this; + } + public CcLinkingInfo build() { + if (ccLinkParamsStore == null) { + Preconditions.checkNotNull(staticModeParamsForDynamicLibrary); + Preconditions.checkNotNull(staticModeParamsForExecutable); + Preconditions.checkNotNull(dynamicModeParamsForDynamicLibrary); + Preconditions.checkNotNull(dynamicModeParamsForExecutable); + ccLinkParamsStore = + new CcLinkParamsStore( + staticModeParamsForDynamicLibrary, + staticModeParamsForExecutable, + dynamicModeParamsForDynamicLibrary, + dynamicModeParamsForExecutable); + } return new CcLinkingInfo(ccLinkParamsStore, ccRunfiles, ccDynamicLibrariesForRuntime); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JplCcLinkParams.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JplCcLinkParams.java index 68f7222d46..09aeecc79c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JplCcLinkParams.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JplCcLinkParams.java @@ -15,12 +15,10 @@ package com.google.devtools.build.lib.rules.java.proto; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget; -import com.google.devtools.build.lib.rules.cpp.AbstractCcLinkParamsStore; -import com.google.devtools.build.lib.rules.cpp.CcLinkParams; -import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore; import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo; import com.google.devtools.build.lib.rules.java.JavaCcLinkParamsProvider; import java.util.ArrayList; @@ -51,19 +49,16 @@ public class JplCcLinkParams { .getTransitiveInfoProviderMap() .getProvider(JavaCcLinkParamsProvider.class)); } - CcLinkingInfo.Builder builder = CcLinkingInfo.Builder.create(); - builder.setCcLinkParamsStore( - new CcLinkParamsStore( - new AbstractCcLinkParamsStore() { - @Override - protected void collect( - CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) { - for (JavaCcLinkParamsProvider provider : providers) { - builder.add(provider.getCcLinkingInfo().getCcLinkParamsStore()); - } - builder.addTransitiveTargets(protoRuntimes); - } - })); - return new JavaCcLinkParamsProvider(builder.build()); + ImmutableList<CcLinkingInfo> ccLinkingInfos = + ImmutableList.<CcLinkingInfo>builder() + .addAll( + providers + .stream() + .map(JavaCcLinkParamsProvider::getCcLinkingInfo) + .collect(ImmutableList.toImmutableList())) + .addAll(AnalysisUtils.getProviders(protoRuntimes, CcLinkingInfo.PROVIDER)) + .build(); + + return new JavaCcLinkParamsProvider(CcLinkingInfo.merge(ccLinkingInfos)); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java index cbb84be9ef..e9d50b91b4 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java @@ -1347,6 +1347,23 @@ public class SkylarkCcCommonTest extends BuildViewTestCase { assertThat(setUpNeverlinkTest("False").getArguments()).contains("-NEVERLINK_OPTION"); } + @Test + public void testEmptyCcLinkingInfoError() throws Exception { + scratch.file("a/BUILD", "load('//tools/build_defs/cc:rule.bzl', 'crule')", "crule(name='r')"); + scratch.file("tools/build_defs/cc/BUILD", ""); + scratch.file( + "tools/build_defs/cc/rule.bzl", + "def _impl(ctx):", + " return [CcLinkingInfo()]", + "crule = rule(", + " _impl,", + ");"); + AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//a:r")); + assertThat(e) + .hasMessageThat() + .contains("Every CcLinkParams parameter must be passed to CcLinkingInfo."); + } + private CppLinkAction setUpNeverlinkTest(String value) throws Exception { SkylarkCcCommonTestHelper.createFilesForTestingLinking( scratch, |