Automated rollback of commit 71479d3ef1627a30b9ec75ca50e6a90cf7db6377. *** Reason for rollback *** http://https://github.com/bazelbuild/bazel/commit/71479d3ef1627a30b9ec75ca50e6a90cf7db6377 broke thousands of targets in the nightly, notably: //abuse/ares/rulesystem/generator/python/rule_set:main (x1754) //net/encapdecap/common/templates:gen_dr_scopes (x903) //testing/cloud/vex/public:vex_main_internal (x782) //wireless/android/play/playlog/server:playlog_server (x401) *** Original change description *** C++: Removes calls to setCcLinkparamsStore of CcLinkingInfo.Builder This is in preparation for deleting CcLinkParamsStore. All remaining calls to the setCcLinkparamsStore method of the CcLinkingInfo builder have been removed. RELNOTES:none PiperOrigin-RevId: 206361808
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index 167a44a..871def8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -887,14 +887,15 @@ CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create(); // TODO(b/111289526): Remove CcLinkingInfo provider from cc_binary as soon as the flag - // --noexperimental_enable_cc_dynlibs_for_runtime is flipped. An empty CcLinkParamsStore is not + // --experimental_enable_cc_dynlibs_for_runtime is flipped. An empty CcLinkParamsStore is not // needed, but here we set it to avoid a null pointer exception in places where we're expecting // it. In the future CcLinkParamsStore will be obligatory. - ccLinkingInfoBuilder - .setStaticModeParamsForDynamicLibrary(CcLinkParams.EMPTY) - .setStaticModeParamsForExecutable(CcLinkParams.EMPTY) - .setDynamicModeParamsForDynamicLibrary(CcLinkParams.EMPTY) - .setDynamicModeParamsForExecutable(CcLinkParams.EMPTY); + ccLinkingInfoBuilder.setCcLinkParamsStore( + new CcLinkParamsStore( + /* staticModeParamsForDynamicLibrary= */ CcLinkParams.EMPTY, + /* staticModeParamsForExecutable= */ CcLinkParams.EMPTY, + /* dynamicModeParamsForDynamicLibrary= */ CcLinkParams.EMPTY, + /* dynamicModeParamsForExecutable= */ CcLinkParams.EMPTY)); if (cppConfiguration.enableCcDynamicLibrariesForRuntime()) { ccLinkingInfoBuilder.setCcDynamicLibrariesForRuntime( new CcDynamicLibrariesForRuntime(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java index e5eb997..2c0e64e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
@@ -55,7 +55,6 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; -import java.util.function.BiFunction; import javax.annotation.Nullable; /** @@ -537,45 +536,11 @@ collectDynamicLibrariesForRuntimeArtifacts( ccLinkingOutputs.getDynamicLibrariesForRuntime())); } - - final CcLinkingOutputs ccLinkingOutputsFinalized = ccLinkingOutputs; - BiFunction<Boolean, Boolean, CcLinkParams> createParams = - (staticMode, forDynamicLibrary) -> { - CcLinkParams.Builder builder = CcLinkParams.builder(); - builder.addLinkstamps(linkstamps.build(), ccCompilationContext); - for (CcLinkingInfo ccLinkingInfo : ccLinkingInfos) { - builder.addTransitiveArgs( - ccLinkingInfo.getCcLinkParams( - /* staticMode= */ staticMode, /* forDynamicLibrary */ forDynamicLibrary)); - } - if (!neverlink) { - builder.addLibraries( - ccLinkingOutputsFinalized.getPreferredLibraries( - staticMode, - /*preferPic=*/ forDynamicLibrary - || ruleContext.getFragment(CppConfiguration.class).forcePic())); - if (!staticMode - || (ccLinkingOutputsFinalized.getStaticLibraries().isEmpty() - && ccLinkingOutputsFinalized.getPicStaticLibraries().isEmpty())) { - builder.addDynamicLibrariesForRuntime( - LinkerInputs.toLibraryArtifacts( - ccLinkingOutputsFinalized.getDynamicLibrariesForRuntime())); - } - builder.addLinkOpts(linkopts); - builder.addNonCodeInputs(nonCodeLinkerInputs); - } - return builder.build(); - }; - - ccLinkingInfoBuilder - .setStaticModeParamsForDynamicLibrary( - createParams.apply(/* staticMode= */ true, /* forDynamicLibrary= */ true)) - .setStaticModeParamsForExecutable( - createParams.apply(/* staticMode= */ true, /* forDynamicLibrary= */ false)) - .setDynamicModeParamsForDynamicLibrary( - createParams.apply(/* staticMode= */ false, /* forDynamicLibrary= */ true)) - .setDynamicModeParamsForExecutable( - createParams.apply(/* staticMode= */ false, /* forDynamicLibrary= */ false)); + CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); + boolean forcePic = cppConfiguration.forcePic(); + ccLinkingInfoBuilder.setCcLinkParamsStore( + new CcLinkParamsStore( + createCcLinkParamsStore(ccLinkingOutputs, ccCompilationContext, forcePic))); providers.put(ccLinkingInfoBuilder.build()); return new LinkingInfo( providers.build(), outputGroups, ccLinkingOutputs, originalLinkingOutputs); @@ -646,6 +611,35 @@ outputGroups.put(DYNAMIC_LIBRARY_OUTPUT_GROUP_NAME, dynamicLibrary.build()); } + private AbstractCcLinkParamsStore createCcLinkParamsStore( + final CcLinkingOutputs ccLinkingOutputs, + final CcCompilationContext ccCompilationContext, + final boolean forcePic) { + return new AbstractCcLinkParamsStore() { + @Override + protected void collect( + CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) { + builder.addLinkstamps(linkstamps.build(), ccCompilationContext); + for (CcLinkingInfo ccLinkingInfo : ccLinkingInfos) { + builder.add(ccLinkingInfo.getCcLinkParamsStore()); + } + if (!neverlink) { + builder.addLibraries( + ccLinkingOutputs.getPreferredLibraries( + linkingStatically, /*preferPic=*/ linkShared || forcePic)); + if (!linkingStatically + || (ccLinkingOutputs.getStaticLibraries().isEmpty() + && ccLinkingOutputs.getPicStaticLibraries().isEmpty())) { + builder.addDynamicLibrariesForRuntime( + LinkerInputs.toLibraryArtifacts(ccLinkingOutputs.getDynamicLibrariesForRuntime())); + } + builder.addLinkOpts(linkopts); + builder.addNonCodeInputs(nonCodeLinkerInputs); + } + } + }; + } + private NestedSet<LinkerInput> collectNativeCcLibraries(CcLinkingOutputs ccLinkingOutputs) { NestedSetBuilder<LinkerInput> result = NestedSetBuilder.linkOrder(); result.addAll(ccLinkingOutputs.getDynamicLibrariesForLinking());
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 b2048a0..483575a 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
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.SkylarkType; import java.util.Collection; +import java.util.stream.Stream; import javax.annotation.Nullable; /** Wrapper for every C++ linking provider. */ @@ -92,11 +93,12 @@ throw new EvalException( loc, "Every CcLinkParams parameter must be passed to CcLinkingInfo."); } - ccLinkingInfoBuilder - .setStaticModeParamsForDynamicLibrary(staticModeParamsForDynamicLibrary) - .setStaticModeParamsForExecutable(staticModeParamsForExecutable) - .setDynamicModeParamsForDynamicLibrary(dynamicModeParamsForDynamicLibrary) - .setDynamicModeParamsForExecutable(dynamicModeParamsForExecutable); + 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 @@ -108,10 +110,12 @@ public static final CcLinkingInfo EMPTY = CcLinkingInfo.Builder.create() - .setStaticModeParamsForDynamicLibrary(CcLinkParams.EMPTY) - .setStaticModeParamsForExecutable(CcLinkParams.EMPTY) - .setDynamicModeParamsForDynamicLibrary(CcLinkParams.EMPTY) - .setDynamicModeParamsForExecutable(CcLinkParams.EMPTY) + .setCcLinkParamsStore( + new CcLinkParamsStore( + /* staticModeParamsForDynamicLibrary= */ CcLinkParams.EMPTY, + /* staticModeParamsForExecutable= */ CcLinkParams.EMPTY, + /* dynamicModeParamsForDynamicLibrary= */ CcLinkParams.EMPTY, + /* dynamicModeParamsForExecutable= */ CcLinkParams.EMPTY)) .build(); private final CcLinkParamsStore ccLinkParamsStore; @@ -159,26 +163,13 @@ } public static CcLinkingInfo merge(Collection<CcLinkingInfo> ccLinkingInfos) { - CcLinkParams.Builder staticModeParamsForDynamicLibraryBuilder = CcLinkParams.builder(); - CcLinkParams.Builder staticModeParamsForExecutableBuilder = CcLinkParams.builder(); - CcLinkParams.Builder dynamicModeParamsForDynamicLibraryBuilder = CcLinkParams.builder(); - CcLinkParams.Builder dynamicModeParamsForExecutableBuilder = CcLinkParams.builder(); - for (CcLinkingInfo ccLinkingInfo : ccLinkingInfos) { - staticModeParamsForDynamicLibraryBuilder.addTransitiveArgs( - ccLinkingInfo.getStaticModeParamsForDynamicLibrary()); - staticModeParamsForExecutableBuilder.addTransitiveArgs( - ccLinkingInfo.getStaticModeParamsForExecutable()); - dynamicModeParamsForDynamicLibraryBuilder.addTransitiveArgs( - ccLinkingInfo.getDynamicModeParamsForDynamicLibrary()); - dynamicModeParamsForExecutableBuilder.addTransitiveArgs( - ccLinkingInfo.getDynamicModeParamsForExecutable()); - } - return new CcLinkingInfo.Builder() - .setStaticModeParamsForDynamicLibrary(staticModeParamsForDynamicLibraryBuilder.build()) - .setStaticModeParamsForExecutable(staticModeParamsForExecutableBuilder.build()) - .setDynamicModeParamsForDynamicLibrary(dynamicModeParamsForDynamicLibraryBuilder.build()) - .setDynamicModeParamsForExecutable(dynamicModeParamsForExecutableBuilder.build()) - .build(); + CcLinkingInfo.Builder builder = new CcLinkingInfo.Builder(); + builder.setCcLinkParamsStore( + CcLinkParamsStore.merge( + Stream.concat(Stream.of(CcLinkingInfo.EMPTY), ccLinkingInfos.stream()) + .map(CcLinkingInfo::getCcLinkParamsStore) + .collect(ImmutableList.toImmutableList()))); + return builder.build(); } @Override @@ -190,24 +181,9 @@ return ccDynamicLibrariesForRuntime; } - public CcLinkParams getCcLinkParams(boolean staticMode, boolean forDynamicLibrary) { - if (staticMode) { - if (forDynamicLibrary) { - return getStaticModeParamsForDynamicLibrary(); - } else { - return getStaticModeParamsForExecutable(); - } - } else { - if (forDynamicLibrary) { - return getDynamicModeParamsForDynamicLibrary(); - } else { - return getDynamicModeParamsForExecutable(); - } - } - } - /** A Builder for {@link CcLinkingInfo}. */ public static class Builder { + CcLinkParamsStore ccLinkParamsStore; CcLinkParams staticModeParamsForDynamicLibrary; CcLinkParams staticModeParamsForExecutable; CcLinkParams dynamicModeParamsForDynamicLibrary; @@ -219,6 +195,19 @@ 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; + } + public Builder setCcRunfiles(CcRunfiles ccRunfiles) { Preconditions.checkState(this.ccRunfiles == null); this.ccRunfiles = ccRunfiles; @@ -233,40 +222,46 @@ } public Builder setStaticModeParamsForDynamicLibrary(CcLinkParams ccLinkParams) { - Preconditions.checkState(this.staticModeParamsForDynamicLibrary == null); + Preconditions.checkState( + this.staticModeParamsForDynamicLibrary == null && ccLinkParamsStore == null); this.staticModeParamsForDynamicLibrary = ccLinkParams; return this; } public Builder setStaticModeParamsForExecutable(CcLinkParams ccLinkParams) { - Preconditions.checkState(this.staticModeParamsForExecutable == null); + Preconditions.checkState( + this.staticModeParamsForExecutable == null && ccLinkParamsStore == null); this.staticModeParamsForExecutable = ccLinkParams; return this; } public Builder setDynamicModeParamsForDynamicLibrary(CcLinkParams ccLinkParams) { - Preconditions.checkState(this.dynamicModeParamsForDynamicLibrary == null); + Preconditions.checkState( + this.dynamicModeParamsForDynamicLibrary == null && ccLinkParamsStore == null); this.dynamicModeParamsForDynamicLibrary = ccLinkParams; return this; } public Builder setDynamicModeParamsForExecutable(CcLinkParams ccLinkParams) { - Preconditions.checkState(this.dynamicModeParamsForExecutable == null); + Preconditions.checkState( + this.dynamicModeParamsForExecutable == null && ccLinkParamsStore == null); this.dynamicModeParamsForExecutable = ccLinkParams; return this; } public CcLinkingInfo build() { - Preconditions.checkNotNull(staticModeParamsForDynamicLibrary); - Preconditions.checkNotNull(staticModeParamsForExecutable); - Preconditions.checkNotNull(dynamicModeParamsForDynamicLibrary); - Preconditions.checkNotNull(dynamicModeParamsForExecutable); - CcLinkParamsStore ccLinkParamsStore = - new CcLinkParamsStore( - staticModeParamsForDynamicLibrary, - staticModeParamsForExecutable, - dynamicModeParamsForDynamicLibrary, - dynamicModeParamsForExecutable); + 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/objc/ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java index ff8e381..42741b5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.objc; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -25,16 +24,12 @@ import com.google.devtools.build.lib.analysis.test.InstrumentedFilesProvider; 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.ArtifactCategory; import com.google.devtools.build.lib.rules.cpp.CcCompilationContext; import com.google.devtools.build.lib.rules.cpp.CcCompilationInfo; -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.cpp.LinkerInputs; -import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes; import com.google.devtools.build.lib.syntax.Type; -import com.google.devtools.build.lib.vfs.FileSystemUtils; import java.util.Map; import java.util.TreeMap; @@ -110,14 +105,9 @@ CcCompilationInfo.Builder ccCompilationInfoBuilder = CcCompilationInfo.Builder.create(); ccCompilationInfoBuilder.setCcCompilationContext(ccCompilationContext); - CcLinkParams ccLinkParams = buildCcLinkParams(common); - CcLinkingInfo ccLinkingInfo = - CcLinkingInfo.Builder.create() - .setStaticModeParamsForDynamicLibrary(ccLinkParams) - .setStaticModeParamsForExecutable(ccLinkParams) - .setDynamicModeParamsForDynamicLibrary(ccLinkParams) - .setDynamicModeParamsForExecutable(ccLinkParams) - .build(); + CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create(); + ccLinkingInfoBuilder.setCcLinkParamsStore( + new CcLinkParamsStore(new ObjcLibraryCcLinkParamsStore(common))); return ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild.build()) .addNativeDeclaredProvider(common.getObjcProvider()) @@ -127,33 +117,11 @@ .addProvider( InstrumentedFilesProvider.class, compilationSupport.getInstrumentedFilesProvider(objectFilesCollector.build())) - .addNativeDeclaredProvider(ccLinkingInfo) + .addNativeDeclaredProvider(ccLinkingInfoBuilder.build()) .addOutputGroups(outputGroupCollector) .build(); } - public CcLinkParams buildCcLinkParams(ObjcCommon common) { - ImmutableSet.Builder<LibraryToLink> libraries = new ImmutableSet.Builder<>(); - ObjcProvider objcProvider = common.getObjcProvider(); - for (Artifact library : objcProvider.get(ObjcProvider.LIBRARY)) { - libraries.add( - LinkerInputs.opaqueLibraryToLink( - library, - ArtifactCategory.STATIC_LIBRARY, - FileSystemUtils.removeExtension(library.getRootRelativePathString()))); - } - libraries.addAll(objcProvider.get(ObjcProvider.CC_LIBRARY)); - - CcLinkParams.Builder builder = CcLinkParams.builder(); - builder.addLibraries(libraries.build()); - - for (SdkFramework sdkFramework : objcProvider.get(ObjcProvider.SDK_FRAMEWORK)) { - builder.addLinkOpts(ImmutableList.of("-framework", sdkFramework.getName())); - } - - return builder.build(); - } - /** Throws errors or warnings for bad attribute state. */ private static void validateAttributes(RuleContext ruleContext) { for (String copt : ObjcCommon.getNonCrosstoolCopts(ruleContext)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryCcLinkParamsStore.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryCcLinkParamsStore.java new file mode 100644 index 0000000..a97ba84 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryCcLinkParamsStore.java
@@ -0,0 +1,61 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.objc; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.rules.cpp.AbstractCcLinkParamsStore; +import com.google.devtools.build.lib.rules.cpp.ArtifactCategory; +import com.google.devtools.build.lib.rules.cpp.CcLinkParams; +import com.google.devtools.build.lib.rules.cpp.LinkerInputs; +import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; +import com.google.devtools.build.lib.vfs.FileSystemUtils; + +/** + * A {@link AbstractCcLinkParamsStore} to be propagated to dependent cc_{library, binary} targets. + */ +class ObjcLibraryCcLinkParamsStore extends AbstractCcLinkParamsStore { + + private final ObjcCommon common; + + /** + * Create a params store. + * + * @param common the {@link ObjcCommon} instance for this target. + */ + public ObjcLibraryCcLinkParamsStore(ObjcCommon common) { + this.common = common; + } + + @Override + protected void collect( + CcLinkParams.Builder builder, boolean linkingStatically, boolean linkShared) { + ObjcProvider objcProvider = common.getObjcProvider(); + + ImmutableSet.Builder<LibraryToLink> libraries = new ImmutableSet.Builder<>(); + for (Artifact library : objcProvider.get(ObjcProvider.LIBRARY)) { + libraries.add(LinkerInputs.opaqueLibraryToLink( + library, ArtifactCategory.STATIC_LIBRARY, + FileSystemUtils.removeExtension(library.getRootRelativePathString()))); + } + libraries.addAll(objcProvider.get(ObjcProvider.CC_LIBRARY)); + builder.addLibraries(libraries.build()); + + for (SdkFramework sdkFramework : objcProvider.get(ObjcProvider.SDK_FRAMEWORK)) { + builder.addLinkOpts(ImmutableList.of("-framework", sdkFramework.getName())); + } + } +} \ No newline at end of file