Implement DSYM generation for the CROSSTOOL rules.

PiperOrigin-RevId: 152011915

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<VariablesExtension> variablesExtensions = new ArrayList<>();
   private final NestedSetBuilder<Artifact> linkActionInputs = NestedSetBuilder.stableOrder();
+  private final ImmutableList.Builder<Artifact> 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<Artifact> constructOutputs(
-      Artifact primaryOutput, Collection<Artifact> outputList, Artifact... outputs) {
+      Artifact primaryOutput, Iterable<Artifact> outputList, Artifact... outputs) {
     return new ImmutableList.Builder<Artifact>()
         .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<String> runtimeLibrarySearchDirectories;
     ImmutableSet<String> 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.
    *
    * <p>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<String> 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<Artifact> forceLoadArtifacts;
   private final ImmutableList<String> attributeLinkopts;
   private final ImmutableSet<VariableCategory> activeVariableCategories;
+  private final DsymOutputType dsymOutputType;
+  private final Artifact dsymBundleZip;
 
   private ObjcVariablesExtension(
       RuleContext ruleContext,
@@ -80,7 +87,9 @@ class ObjcVariablesExtension implements VariablesExtension {
       ImmutableList<String> libraryNames,
       ImmutableSet<Artifact> forceLoadArtifacts,
       ImmutableList<String> attributeLinkopts,
-      ImmutableSet<VariableCategory> activeVariableCategories) {
+      ImmutableSet<VariableCategory> 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<Artifact> forceLoadArtifacts;
     private ImmutableList<String> libraryNames;
     private ImmutableList<String> attributeLinkopts;
+    private DsymOutputType dsymOutputType;
+    private Artifact dsymBundleZip;
     
     private final ImmutableSet.Builder<VariableCategory> 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<VariableCategory> 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);
     }
   }
 }