From 17adca4d05b3bf9baf479d30cddee18c6651d0df Mon Sep 17 00:00:00 2001 From: cpeyser Date: Mon, 3 Apr 2017 15:22:09 +0000 Subject: Implement DSYM generation for the CROSSTOOL rules. PiperOrigin-RevId: 152011915 --- .../build/lib/rules/cpp/CppLinkActionBuilder.java | 11 ++++- .../build/lib/rules/objc/CompilationSupport.java | 51 +++++++++++++++++++++ .../rules/objc/CrosstoolCompilationSupport.java | 35 +++++++++----- .../lib/rules/objc/LegacyCompilationSupport.java | 53 ---------------------- .../lib/rules/objc/ObjcVariablesExtension.java | 52 +++++++++++++++++++-- 5 files changed, 132 insertions(+), 70 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index 532ad5ed58..f65e09d902 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -190,6 +190,7 @@ public class CppLinkActionBuilder { private final List variablesExtensions = new ArrayList<>(); private final NestedSetBuilder linkActionInputs = NestedSetBuilder.stableOrder(); + private final ImmutableList.Builder linkActionOutputs = ImmutableList.builder(); /** * Creates a builder that builds {@link CppLinkAction} instances. @@ -593,7 +594,7 @@ public class CppLinkActionBuilder { actionOutputs = constructOutputs( output, - linkstampMap.values(), + Iterables.concat(linkstampMap.values(), linkActionOutputs.build()), interfaceOutputLibrary == null ? null : interfaceOutputLibrary.getArtifact(), symbolCounts); } @@ -853,7 +854,7 @@ public class CppLinkActionBuilder { } private static ImmutableList constructOutputs( - Artifact primaryOutput, Collection outputList, Artifact... outputs) { + Artifact primaryOutput, Iterable outputList, Artifact... outputs) { return new ImmutableList.Builder() .add(primaryOutput) .addAll(outputList) @@ -1250,6 +1251,12 @@ public class CppLinkActionBuilder { return this; } + /** Adds an extra output artifact to the link action. */ + public CppLinkActionBuilder addActionOutput(Artifact output) { + this.linkActionOutputs.add(output); + return this; + } + private static class LinkArgCollector { ImmutableSet runtimeLibrarySearchDirectories; ImmutableSet librarySearchDirectories; diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index 18dc1d423e..c0859047de 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -877,6 +877,57 @@ public abstract class CompilationSupport { @Nullable CcToolchainProvider ccToolchain, @Nullable FdoSupportProvider fdoSupport) throws InterruptedException; + private PathFragment removeSuffix(PathFragment path, String suffix) { + String name = path.getBaseName(); + Preconditions.checkArgument( + name.endsWith(suffix), "expected %s to end with %s, but it does not", name, suffix); + return path.replaceName(name.substring(0, name.length() - suffix.length())); + } + + protected CompilationSupport registerDsymActions(DsymOutputType dsymOutputType) { + Artifact tempDsymBundleZip = intermediateArtifacts.tempDsymBundleZip(dsymOutputType); + Artifact linkedBinary = + objcConfiguration.shouldStripBinary() + ? intermediateArtifacts.unstrippedSingleArchitectureBinary() + : intermediateArtifacts.strippedSingleArchitectureBinary(); + Artifact debugSymbolFile = intermediateArtifacts.dsymSymbol(dsymOutputType); + Artifact dsymPlist = intermediateArtifacts.dsymPlist(dsymOutputType); + + PathFragment dsymOutputDir = removeSuffix(tempDsymBundleZip.getExecPath(), ".temp.zip"); + PathFragment dsymPlistZipEntry = dsymPlist.getExecPath().relativeTo(dsymOutputDir); + PathFragment debugSymbolFileZipEntry = + debugSymbolFile + .getExecPath() + .replaceName(linkedBinary.getFilename()) + .relativeTo(dsymOutputDir); + + StringBuilder unzipDsymCommand = + new StringBuilder() + .append( + String.format( + "unzip -p %s %s > %s", + tempDsymBundleZip.getExecPathString(), + dsymPlistZipEntry, + dsymPlist.getExecPathString())) + .append( + String.format( + " && unzip -p %s %s > %s", + tempDsymBundleZip.getExecPathString(), + debugSymbolFileZipEntry, + debugSymbolFile.getExecPathString())); + + ruleContext.registerAction( + new SpawnAction.Builder() + .setMnemonic("UnzipDsym") + .setShellCommand(unzipDsymCommand.toString()) + .addInput(tempDsymBundleZip) + .addOutput(dsymPlist) + .addOutput(debugSymbolFile) + .build(ruleContext)); + + return this; + } + /** * Returns all framework names to pass to the linker using {@code -framework} flags. For a * framework in the directory foo/bar.framework, the name is "bar". Each framework is found diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java index 30b8d9f55f..26c8aaac83 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java @@ -89,12 +89,14 @@ public class CrosstoolCompilationSupport extends CompilationSupport { * "is_not_test_target" instead of the more intuitive "is_test_target". */ private static final String IS_NOT_TEST_TARGET_FEATURE_NAME = "is_not_test_target"; + /** Enabled if this target generates debug symbols in a dSYM file. */ + private static final String GENERATE_DSYM_FILE_FEATURE_NAME = "generate_dsym_file"; /** * Enabled if this target does not generate debug symbols. * *

Note that the crosstool does not support feature negation in FlagSet.with_feature, which is * the mechanism used to condition linker arguments here. Therefore, we expose - * "no_generate_debug_symbols" instead of the more intuitive "generate_debug_symbols". + * "no_generate_debug_symbols" in addition to "generate_dsym_file" */ private static final String NO_GENERATE_DEBUG_SYMBOLS_FEATURE_NAME = "no_generate_debug_symbols"; @@ -260,7 +262,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { ? LinkTargetType.OBJCPP_EXECUTABLE : LinkTargetType.OBJC_EXECUTABLE; - ObjcVariablesExtension extension = + ObjcVariablesExtension.Builder extensionBuilder = new ObjcVariablesExtension.Builder() .setRuleContext(ruleContext) .setObjcProvider(objcProvider) @@ -270,12 +272,11 @@ public class CrosstoolCompilationSupport extends CompilationSupport { .setLibraryNames(libraryNames(objcProvider)) .setForceLoadArtifacts(getForceLoadArtifacts(objcProvider)) .setAttributeLinkopts(attributes.linkopts()) - .addVariableCategory(VariableCategory.EXECUTABLE_LINKING_VARIABLES) - .build(); - + .addVariableCategory(VariableCategory.EXECUTABLE_LINKING_VARIABLES); + Artifact binaryToLink = getBinaryToLink(); FdoSupportProvider fdoSupport = CppHelper.getFdoSupport(ruleContext, ":cc_toolchain"); - CppLinkAction executableLinkAction = + CppLinkActionBuilder executableLinkAction = new CppLinkActionBuilder(ruleContext, binaryToLink, toolchain, fdoSupport) .setMnemonic("ObjcLink") .addActionInputs(bazelBuiltLibraries) @@ -289,10 +290,20 @@ public class CrosstoolCompilationSupport extends CompilationSupport { .setLinkType(linkType) .setLinkStaticness(LinkStaticness.FULLY_STATIC) .addLinkopts(ImmutableList.copyOf(extraLinkArgs)) - .addVariablesExtension(extension) - .setFeatureConfiguration(getFeatureConfiguration(ruleContext, buildConfiguration)) - .build(); - ruleContext.registerAction(executableLinkAction); + .setFeatureConfiguration(getFeatureConfiguration(ruleContext, buildConfiguration)); + + if (objcConfiguration.generateDsym()) { + Artifact dsymBundleZip = intermediateArtifacts.tempDsymBundleZip(dsymOutputType); + extensionBuilder + .setDsymBundleZip(dsymBundleZip) + .addVariableCategory(VariableCategory.DSYM_VARIABLES) + .setDsymOutputType(dsymOutputType); + registerDsymActions(dsymOutputType); + executableLinkAction.addActionOutput(dsymBundleZip); + } + + executableLinkAction.addVariablesExtension(extensionBuilder.build()); + ruleContext.registerAction(executableLinkAction.build()); if (objcConfiguration.shouldStripBinary()) { registerBinaryStripAction(binaryToLink, getStrippingType(extraLinkArgs)); @@ -424,7 +435,9 @@ public class CrosstoolCompilationSupport extends CompilationSupport { if (!TargetUtils.isTestRule(ruleContext.getRule())) { activatedCrosstoolSelectables.add(IS_NOT_TEST_TARGET_FEATURE_NAME); } - if (!configuration.getFragment(ObjcConfiguration.class).generateDsym()) { + if (configuration.getFragment(ObjcConfiguration.class).generateDsym()) { + activatedCrosstoolSelectables.add(GENERATE_DSYM_FILE_FEATURE_NAME); + } else { activatedCrosstoolSelectables.add(NO_GENERATE_DEBUG_SYMBOLS_FEATURE_NAME); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java index cd8db4c994..17bc6215ff 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java @@ -49,7 +49,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; -import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate; import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate.OutputPathMapper; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -67,7 +66,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; import com.google.devtools.build.lib.rules.cpp.CppModuleMap; import com.google.devtools.build.lib.rules.cpp.FdoSupportProvider; -import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.LinkedHashSet; @@ -802,57 +800,6 @@ public class LegacyCompilationSupport extends CompilationSupport { } } - private CompilationSupport registerDsymActions(DsymOutputType dsymOutputType) { - Artifact tempDsymBundleZip = intermediateArtifacts.tempDsymBundleZip(dsymOutputType); - Artifact linkedBinary = - objcConfiguration.shouldStripBinary() - ? intermediateArtifacts.unstrippedSingleArchitectureBinary() - : intermediateArtifacts.strippedSingleArchitectureBinary(); - Artifact debugSymbolFile = intermediateArtifacts.dsymSymbol(dsymOutputType); - Artifact dsymPlist = intermediateArtifacts.dsymPlist(dsymOutputType); - - PathFragment dsymOutputDir = removeSuffix(tempDsymBundleZip.getExecPath(), ".temp.zip"); - PathFragment dsymPlistZipEntry = dsymPlist.getExecPath().relativeTo(dsymOutputDir); - PathFragment debugSymbolFileZipEntry = - debugSymbolFile - .getExecPath() - .replaceName(linkedBinary.getFilename()) - .relativeTo(dsymOutputDir); - - StringBuilder unzipDsymCommand = - new StringBuilder() - .append( - String.format( - "unzip -p %s %s > %s", - tempDsymBundleZip.getExecPathString(), - dsymPlistZipEntry, - dsymPlist.getExecPathString())) - .append( - String.format( - " && unzip -p %s %s > %s", - tempDsymBundleZip.getExecPathString(), - debugSymbolFileZipEntry, - debugSymbolFile.getExecPathString())); - - ruleContext.registerAction( - new SpawnAction.Builder() - .setMnemonic("UnzipDsym") - .setShellCommand(unzipDsymCommand.toString()) - .addInput(tempDsymBundleZip) - .addOutput(dsymPlist) - .addOutput(debugSymbolFile) - .build(ruleContext)); - - return this; - } - - private PathFragment removeSuffix(PathFragment path, String suffix) { - String name = path.getBaseName(); - Preconditions.checkArgument( - name.endsWith(suffix), "expect %s to end with %s, but it does not", name, suffix); - return path.replaceName(name.substring(0, name.length() - suffix.length())); - } - /** Returns a list of clang flags used for all link and compile actions executed through clang. */ private List commonLinkAndCompileFlagsForClang( ObjcProvider provider, ObjcConfiguration objcConfiguration, diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java index bc2a0b8bcb..ac15390cff 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.rules.apple.AppleConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.StringSequenceBuilder; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import java.util.Set; /** Build variable extensions for templating a toolchain for objc builds. */ @@ -55,6 +56,10 @@ class ObjcVariablesExtension implements VariablesExtension { static final String FORCE_LOAD_EXEC_PATHS_VARIABLE_NAME = "force_load_exec_paths"; static final String DEP_LINKOPTS_VARIABLE_NAME = "dep_linkopts"; static final String ATTR_LINKOPTS_VARIABLE_NAME = "attr_linkopts"; + + // dsym variables + static final String DSYM_PATH_VARIABLE_NAME = "dsym_path"; + static final String DSYM_BUNDLE_ZIP_VARIABLE_NAME = "dsym_bundle_zip"; private final RuleContext ruleContext; private final ObjcProvider objcProvider; @@ -68,6 +73,8 @@ class ObjcVariablesExtension implements VariablesExtension { private final ImmutableSet forceLoadArtifacts; private final ImmutableList attributeLinkopts; private final ImmutableSet activeVariableCategories; + private final DsymOutputType dsymOutputType; + private final Artifact dsymBundleZip; private ObjcVariablesExtension( RuleContext ruleContext, @@ -80,7 +87,9 @@ class ObjcVariablesExtension implements VariablesExtension { ImmutableList libraryNames, ImmutableSet forceLoadArtifacts, ImmutableList attributeLinkopts, - ImmutableSet activeVariableCategories) { + ImmutableSet activeVariableCategories, + DsymOutputType dsymOutputType, + Artifact dsymBundleZip) { this.ruleContext = ruleContext; this.objcProvider = objcProvider; this.compilationArtifacts = compilationArtifacts; @@ -93,11 +102,16 @@ class ObjcVariablesExtension implements VariablesExtension { this.forceLoadArtifacts = forceLoadArtifacts; this.attributeLinkopts = attributeLinkopts; this.activeVariableCategories = activeVariableCategories; + this.dsymOutputType = dsymOutputType; + this.dsymBundleZip = dsymBundleZip; } /** Type of build variable that can optionally exported by this extension. */ public enum VariableCategory { - ARCHIVE_VARIABLES, FULLY_LINK_VARIABLES, EXECUTABLE_LINKING_VARIABLES; + ARCHIVE_VARIABLES, + FULLY_LINK_VARIABLES, + EXECUTABLE_LINKING_VARIABLES, + DSYM_VARIABLES; } @Override @@ -114,6 +128,9 @@ class ObjcVariablesExtension implements VariablesExtension { if (activeVariableCategories.contains(VariableCategory.EXECUTABLE_LINKING_VARIABLES)) { addExecutableLinkVariables(builder); } + if (activeVariableCategories.contains(VariableCategory.DSYM_VARIABLES)) { + addDsymVariables(builder); + } } private void addPchVariables(CcToolchainFeatures.Variables.Builder builder) { @@ -192,6 +209,14 @@ class ObjcVariablesExtension implements VariablesExtension { builder.addStringSequenceVariable(ATTR_LINKOPTS_VARIABLE_NAME, attributeLinkopts); } + private void addDsymVariables(CcToolchainFeatures.Variables.Builder builder) { + builder.addStringVariable( + DSYM_BUNDLE_ZIP_VARIABLE_NAME, dsymBundleZip.getShellEscapedExecPathString()); + builder.addStringVariable( + DSYM_PATH_VARIABLE_NAME, + FileSystemUtils.removeExtension(dsymBundleZip.getExecPath()).getPathString()); + } + /** A Builder for {@link ObjcVariablesExtension}. */ static class Builder { private RuleContext ruleContext; @@ -204,6 +229,8 @@ class ObjcVariablesExtension implements VariablesExtension { private ImmutableSet forceLoadArtifacts; private ImmutableList libraryNames; private ImmutableList attributeLinkopts; + private DsymOutputType dsymOutputType; + private Artifact dsymBundleZip; private final ImmutableSet.Builder activeVariableCategoriesBuilder = ImmutableSet.builder(); @@ -274,6 +301,18 @@ class ObjcVariablesExtension implements VariablesExtension { return this; } + /** Sets the dsym type for the dsym bundle generated by this target. */ + public Builder setDsymOutputType(DsymOutputType dsymOutputType) { + this.dsymOutputType = dsymOutputType; + return this; + } + + /** Sets the Artifact for the zipped dsym bundle. */ + public Builder setDsymBundleZip(Artifact dsymBundleZip) { + this.dsymBundleZip = dsymBundleZip; + return this; + } + public ObjcVariablesExtension build() { ImmutableSet activeVariableCategories = @@ -295,7 +334,10 @@ class ObjcVariablesExtension implements VariablesExtension { Preconditions.checkNotNull(forceLoadArtifacts, "missing force-load artifacts"); Preconditions.checkNotNull(attributeLinkopts, "missing attribute linkopts"); } - + if (activeVariableCategories.contains(VariableCategory.DSYM_VARIABLES)) { + Preconditions.checkNotNull(dsymOutputType, "missing dsym output type"); + } + return new ObjcVariablesExtension( ruleContext, objcProvider, @@ -307,7 +349,9 @@ class ObjcVariablesExtension implements VariablesExtension { libraryNames, forceLoadArtifacts, attributeLinkopts, - activeVariableCategories); + activeVariableCategories, + dsymOutputType, + dsymBundleZip); } } } -- cgit v1.2.3