From 4348e3ca10f6e2bc1bed925c51524ed78d0059ef Mon Sep 17 00:00:00 2001 From: hlopko Date: Wed, 26 Sep 2018 05:17:39 -0700 Subject: [PATCH] Refactor how AppleCcToolchain passes data to the CcToolchain This is a preparation for a change moving CcToolchainProvider creation from CcToolchain to CcToolchainSuite. #6072. RELNOTES: None. PiperOrigin-RevId: 214589270 --- .../build/lib/analysis/AnalysisUtils.java | 21 --- .../lib/rules/apple/cpp/AppleCcToolchain.java | 20 +-- .../build/lib/rules/cpp/CcToolchain.java | 124 ++++++++++-------- 3 files changed, 73 insertions(+), 92 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java index 099392baabda60..0160718d57c9af 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java @@ -19,16 +19,11 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; import com.google.devtools.build.lib.analysis.config.TransitionResolver; -import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; 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.collect.nestedset.Order; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.BuiltinProvider; @@ -149,22 +144,6 @@ public static PathFragment getManifestPathFromFilesetPath(PathFragment filesetDi return outputManifestFrag; } - /** - * Returns the middleman artifact on the specified attribute of the specified rule for the - * specified mode, or an empty set if it does not exist. - */ - public static NestedSet getMiddlemanFor(RuleContext rule, String attribute, Mode mode) { - TransitiveInfoCollection prereq = rule.getPrerequisite(attribute, mode); - if (prereq == null) { - return NestedSetBuilder.emptySet(Order.STABLE_ORDER); - } - MiddlemanProvider provider = prereq.getProvider(MiddlemanProvider.class); - if (provider == null) { - return NestedSetBuilder.emptySet(Order.STABLE_ORDER); - } - return provider.getMiddlemanArtifact(); - } - /** * Returns a path fragment qualified by the rule name and unique fragment to * disambiguate artifacts produced from the source file appearing in diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java index dbe364e6626014..abbfb9fb2580f7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java @@ -15,19 +15,13 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; -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.apple.AppleConfiguration; import com.google.devtools.build.lib.rules.apple.ApplePlatform; import com.google.devtools.build.lib.rules.apple.AppleToolchain; import com.google.devtools.build.lib.rules.apple.XcodeConfig; import com.google.devtools.build.lib.rules.apple.XcodeConfigProvider; import com.google.devtools.build.lib.rules.cpp.CcToolchain; -import com.google.devtools.build.lib.rules.cpp.CcToolchainRule; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables; import java.util.LinkedHashMap; import java.util.Map; @@ -57,7 +51,7 @@ public class AppleCcToolchain extends CcToolchain { public static final String APPLE_SDK_PLATFORM_VALUE_KEY = "apple_sdk_platform_value"; @Override - protected void addBuildVariables(RuleContext ruleContext, CcToolchainVariables.Builder variables) + protected CcToolchainVariables getAdditionalBuildVariables(RuleContext ruleContext) throws RuleErrorException { AppleConfiguration appleConfiguration = ruleContext.getFragment(AppleConfiguration.class); @@ -70,7 +64,7 @@ protected void addBuildVariables(RuleContext ruleContext, CcToolchainVariables.B ApplePlatform platform = appleConfiguration.getSingleArchPlatform(); Map appleEnv = getEnvironmentBuildVariables(ruleContext); - + CcToolchainVariables.Builder variables = new CcToolchainVariables.Builder(); variables .addStringVariable( XCODE_VERSION_KEY, @@ -109,16 +103,12 @@ protected void addBuildVariables(RuleContext ruleContext, CcToolchainVariables.B .addStringVariable( VERSION_MIN_KEY, XcodeConfig.getMinimumOsForPlatformType(ruleContext, platform.getType()).toString()); + return variables.build(); } @Override - protected NestedSet fullInputsForLink( - RuleContext ruleContext, NestedSet link) { - return NestedSetBuilder.stableOrder() - .addTransitive(link) - .addTransitive( - AnalysisUtils.getMiddlemanFor(ruleContext, CcToolchainRule.LIBC_TOP_ATTR, Mode.TARGET)) - .build(); + protected boolean isAppleToolchain() { + return true; } private ImmutableMap getEnvironmentBuildVariables(RuleContext ruleContext) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index 903d01481ed105..3583b4ec80f08b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; -import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.CompilationHelper; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; @@ -306,7 +305,7 @@ private Artifact convertLLVMRawProfileToIndexed( ruleContext.registerAction( new SpawnAction.Builder() .addInput(rawProfileArtifact) - .addTransitiveInputs(getFiles(ruleContext, "all_files")) + .addTransitiveInputs(getMiddlemanOrFiles(ruleContext, "all_files")) .addOutput(profileArtifact) .useDefaultShellEnvironment() .setExecutable(toolchainInfo.getToolPathFragment(Tool.LLVM_PROFDATA)) @@ -439,17 +438,20 @@ public ConfiguredTarget create(RuleContext ruleContext) final Label label = ruleContext.getLabel(); final NestedSet crosstool = ruleContext.getPrerequisite("all_files", Mode.HOST) .getProvider(FileProvider.class).getFilesToBuild(); - final NestedSet crosstoolMiddleman = getFiles(ruleContext, "all_files"); - final NestedSet compile = getFiles(ruleContext, "compiler_files"); + final NestedSet crosstoolMiddleman = getMiddlemanOrFiles(ruleContext, "all_files"); + final NestedSet compile = getMiddlemanOrFiles(ruleContext, "compiler_files"); final NestedSet compileWithoutIncludes = - getOptionalFiles(ruleContext, "compiler_files_without_includes"); - final NestedSet strip = getFiles(ruleContext, "strip_files"); - final NestedSet objcopy = getFiles(ruleContext, "objcopy_files"); - final NestedSet as = getOptionalFiles(ruleContext, "as_files"); - final NestedSet ar = getOptionalFiles(ruleContext, "ar_files"); - final NestedSet link = getFiles(ruleContext, "linker_files"); - final NestedSet dwp = getFiles(ruleContext, "dwp_files"); - final NestedSet libcLink = inputsForLibc(ruleContext); + getOptionalMiddlemanOrFiles(ruleContext, "compiler_files_without_includes"); + final NestedSet strip = getMiddlemanOrFiles(ruleContext, "strip_files"); + final NestedSet objcopy = getMiddlemanOrFiles(ruleContext, "objcopy_files"); + final NestedSet as = getOptionalMiddlemanOrFiles(ruleContext, "as_files"); + final NestedSet ar = getOptionalMiddlemanOrFiles(ruleContext, "ar_files"); + final NestedSet link = getMiddlemanOrFiles(ruleContext, "linker_files"); + final NestedSet dwp = getMiddlemanOrFiles(ruleContext, "dwp_files"); + final NestedSet libcMiddleman = + getOptionalMiddlemanOrFiles(ruleContext, CcToolchainRule.LIBC_TOP_ATTR, Mode.TARGET); + final NestedSet libc = + getOptionalFiles(ruleContext, CcToolchainRule.LIBC_TOP_ATTR, Mode.TARGET); String purposePrefix = Actions.escapeLabel(label) + "_"; String runtimeSolibDirBase = "_solib_" + "_" + Actions.escapeLabel(label); final PathFragment runtimeSolibDir = @@ -539,7 +541,7 @@ public ConfiguredTarget create(RuleContext ruleContext) NestedSetBuilder> coverageEnvironment = NestedSetBuilder.compileOrder(); - NestedSet coverage = getOptionalFiles(ruleContext, "coverage_files"); + NestedSet coverage = getOptionalMiddlemanOrFiles(ruleContext, "coverage_files"); if (coverage.isEmpty()) { coverage = crosstool; } @@ -599,18 +601,21 @@ public ConfiguredTarget create(RuleContext ruleContext) toolchainInfo, cppConfiguration.getCrosstoolTopPathFragment(), crosstool, - fullInputsForCrosstool(ruleContext, crosstoolMiddleman), + /* crosstoolMiddleman= */ NestedSetBuilder.stableOrder() + .addTransitive(crosstoolMiddleman) + .addTransitive(libcMiddleman) + .build(), compile, compileWithoutIncludes, strip, objcopy, as, ar, - fullInputsForLink(ruleContext, link), + fullInputsForLink(ruleContext, link, libcMiddleman), ruleContext.getPrerequisiteArtifact("$interface_library_builder", Mode.HOST), dwp, coverage, - libcLink, + libc, staticRuntimeLinkInputs, staticRuntimeLinkMiddleman, dynamicRuntimeLinkSymlinks, @@ -620,7 +625,7 @@ public ConfiguredTarget create(RuleContext ruleContext) supportsParamFiles, supportsHeaderParsing, getBuildVariables(ruleContext, toolchainInfo.getDefaultSysroot()), - getBuiltinIncludes(ruleContext), + getBuiltinIncludes(libc), coverageEnvironment.build(), toolchainInfo.supportsInterfaceSharedObjects() ? ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST) @@ -812,9 +817,9 @@ private CToolchain getToolchainFromAttributes( } } - private ImmutableList getBuiltinIncludes(RuleContext ruleContext) { + private ImmutableList getBuiltinIncludes(NestedSet libc) { ImmutableList.Builder result = ImmutableList.builder(); - for (Artifact artifact : inputsForLibc(ruleContext)) { + for (Artifact artifact : libc) { if (artifact.getExecPath().endsWith(BUILTIN_INCLUDE_FILE_SUFFIX)) { result.add(artifact); } @@ -823,36 +828,20 @@ private ImmutableList getBuiltinIncludes(RuleContext ruleContext) { return result.build(); } - private NestedSet inputsForLibc(RuleContext ruleContext) { - TransitiveInfoCollection libc = - ruleContext.getPrerequisite(CcToolchainRule.LIBC_TOP_ATTR, Mode.TARGET); - return libc != null - ? libc.getProvider(FileProvider.class).getFilesToBuild() - : NestedSetBuilder.emptySet(Order.STABLE_ORDER); - } - - private NestedSet fullInputsForCrosstool(RuleContext ruleContext, - NestedSet crosstoolMiddleman) { - return NestedSetBuilder.stableOrder() - .addTransitive(crosstoolMiddleman) - .addTransitive( - AnalysisUtils.getMiddlemanFor(ruleContext, CcToolchainRule.LIBC_TOP_ATTR, Mode.TARGET)) - .build(); - } - /** * Returns the crosstool-derived link action inputs for a given rule. Adds the given set of * artifacts as extra inputs. */ - protected NestedSet fullInputsForLink( - RuleContext ruleContext, NestedSet link) { - return NestedSetBuilder.stableOrder() - .addTransitive(link) - .addTransitive( - AnalysisUtils.getMiddlemanFor(ruleContext, CcToolchainRule.LIBC_TOP_ATTR, Mode.TARGET)) - .add(ruleContext.getPrerequisiteArtifact("$interface_library_builder", Mode.HOST)) - .add(ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST)) - .build(); + private NestedSet fullInputsForLink( + RuleContext ruleContext, NestedSet link, NestedSet libcLink) { + NestedSetBuilder builder = + NestedSetBuilder.stableOrder().addTransitive(link).addTransitive(libcLink); + if (!isAppleToolchain()) { + builder + .add(ruleContext.getPrerequisiteArtifact("$interface_library_builder", Mode.HOST)) + .add(ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST)); + } + return builder.build(); } private CppModuleMap createCrosstoolModuleMap(RuleContext ruleContext) { @@ -877,8 +866,13 @@ private TransitiveInfoCollection selectDep( return ruleContext.getPrerequisites(attribute, Mode.TARGET).get(0); } - private NestedSet getFiles(RuleContext context, String attribute) { - TransitiveInfoCollection dep = context.getPrerequisite(attribute, Mode.HOST); + private NestedSet getMiddlemanOrFiles(RuleContext context, String attribute) { + return getMiddlemanOrFiles(context, attribute, Mode.HOST); + } + + private NestedSet getMiddlemanOrFiles( + RuleContext context, String attribute, Mode mode) { + TransitiveInfoCollection dep = context.getPrerequisite(attribute, mode); MiddlemanProvider middlemanProvider = dep.getProvider(MiddlemanProvider.class); // We use the middleman if we can (if the dep is a filegroup), otherwise, just the regular // filesToBuild (e.g. if it is a simple input file) @@ -887,10 +881,23 @@ private NestedSet getFiles(RuleContext context, String attribute) { : dep.getProvider(FileProvider.class).getFilesToBuild(); } - private NestedSet getOptionalFiles(RuleContext context, String attribute) { - TransitiveInfoCollection dep = context.getPrerequisite(attribute, Mode.HOST); + private NestedSet getOptionalMiddlemanOrFiles(RuleContext context, String attribute) { + return getOptionalMiddlemanOrFiles(context, attribute, Mode.HOST); + } + + private NestedSet getOptionalMiddlemanOrFiles( + RuleContext context, String attribute, Mode mode) { + TransitiveInfoCollection dep = context.getPrerequisite(attribute, mode); return dep != null - ? getFiles(context, attribute) + ? getMiddlemanOrFiles(context, attribute, mode) + : NestedSetBuilder.emptySet(Order.STABLE_ORDER); + } + + private NestedSet getOptionalFiles( + RuleContext ruleContext, String attribute, Mode mode) { + TransitiveInfoCollection dep = ruleContext.getPrerequisite(attribute, mode); + return dep != null + ? dep.getProvider(FileProvider.class).getFilesToBuild() : NestedSetBuilder.emptySet(Order.STABLE_ORDER); } @@ -943,21 +950,26 @@ private final CcToolchainVariables getBuildVariables( variables.addStringVariable(CcCommon.SYSROOT_VARIABLE_NAME, sysroot.getPathString()); } - addBuildVariables(ruleContext, variables); + variables.addAllNonTransitive(getAdditionalBuildVariables(ruleContext)); return variables.build(); } /** - * Add local build variables from subclasses into {@link - * com.google.devtools.build.lib.rules.cpp.CcToolchainVariables} returned from {@link - * #getBuildVariables(RuleContext, PathFragment)}. + * Add local build variables from subclasses into {@link CcToolchainVariables} returned from + * {@link CcToolchain#getBuildVariables(RuleContext, PathFragment)}. * *

This method is meant to be overridden by subclasses of CcToolchain. */ - protected void addBuildVariables(RuleContext ruleContext, CcToolchainVariables.Builder variables) + protected boolean isAppleToolchain() { + // To be overridden in subclass. + return false; + } + + protected CcToolchainVariables getAdditionalBuildVariables(RuleContext ruleContext) throws RuleErrorException { - // To be overridden in subclasses. + // To be overridden in subclass. + return CcToolchainVariables.EMPTY; } private PathFragment calculateSysroot(RuleContext ruleContext, PathFragment defaultSysroot) {