Wrap inputs to ObjcLink (via clang) invocations in an objlist, and pass the args as a -filelist arg.

This prevents certain many-arg actions from making clang unhappy.

Also ensures that no -force_load artifacts are present in the filelist. Clang seems to not dedupe artifacts present in both a filelist and -force_load, causing duplicate symbol errors.

--
MOS_MIGRATED_REVID=125170226

2 files changed, 41 insertions, 22 deletions

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 5446cb1fed..8036821b7f 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
@@ -779,7 +779,7 @@ public final class CompilationSupport {
 
   private void registerArchiveActions(ImmutableList.Builder<Artifact> objFiles, Artifact archive) {
     for (Action action :
-        archiveActions(objFiles.build(), archive, intermediateArtifacts.objList())) {
+        archiveActions(objFiles.build(), archive, intermediateArtifacts.archiveObjList())) {
       ruleContext.registerAction(action);
     }
   }
@@ -791,11 +791,7 @@ public final class CompilationSupport {
 
     ImmutableList.Builder<Action> actions = new ImmutableList.Builder<>();
 
-    actions.add(new FileWriteAction(
-        ruleContext.getActionOwner(),
-        objList,
-        Artifact.joinExecPaths("\n", objFiles),
-        /*makeExecutable=*/ false));
+    actions.add(objFilelistAction(objFiles, objList));
 
     actions.add(ObjcRuleClasses.spawnAppleEnvActionBuilder(
             appleConfiguration, appleConfiguration.getSingleArchPlatform())
@@ -816,6 +812,14 @@ public final class CompilationSupport {
     return actions.build();
   }
 
+  private Action objFilelistAction(Iterable<Artifact> objFiles, Artifact objList) {
+    return new FileWriteAction(
+        ruleContext.getActionOwner(),
+        objList,
+        Artifact.joinExecPaths("\n", objFiles),
+        /*makeExecutable=*/ false);
+  }
+
   /**
    * Registers an action to create an archive artifact by fully (statically) linking all
    * transitive dependencies of this rule.
@@ -1045,6 +1049,7 @@ public final class CompilationSupport {
             .addInputs(ccLibraries)
             .addInputs(extraLinkInputs)
             .addInputs(prunedJ2ObjcArchives)
+            .addInput(intermediateArtifacts.linkerObjList())
             .addInput(xcrunwrapper(ruleContext).getExecutable())
             .build(ruleContext));
 
@@ -1158,11 +1163,24 @@ public final class CompilationSupport {
       commandLine.add("-dead_strip").add("-no_dead_strip_inits_and_terms");
     }
 
+    Iterable<Artifact> ccLibrariesToForceLoad =
+        Iterables.filter(ccLibraries, ALWAYS_LINKED_CC_LIBRARY);
+
+    ImmutableSet<Artifact> forceLinkArtifacts = ImmutableSet.<Artifact>builder()
+        .addAll(objcProvider.get(FORCE_LOAD_LIBRARY))
+        .addAll(ccLibrariesToForceLoad).build();
+
+    Artifact inputFileList = intermediateArtifacts.linkerObjList();
+    Iterable<Artifact> objFiles =
+        Iterables.concat(bazelBuiltLibraries, objcProvider.get(IMPORTED_LIBRARY), ccLibraries);
+    // Clang loads archives specified in filelists and also specified as -force_load twice,
+    // resulting in duplicate symbol errors unless they are deduped.
+    objFiles = Iterables.filter(objFiles, Predicates.not(Predicates.in(forceLinkArtifacts)));
+
+    ruleContext.registerAction(objFilelistAction(objFiles, inputFileList));
+    
     if (objcConfiguration.shouldPrioritizeStaticLibs()) {
-      commandLine
-          .addExecPaths(bazelBuiltLibraries)
-          .addExecPaths(objcProvider.get(IMPORTED_LIBRARY))
-          .addExecPaths(ccLibraries);
+      commandLine.add("-filelist").add(inputFileList.getExecPathString());
     }
 
     commandLine
@@ -1184,19 +1202,12 @@ public final class CompilationSupport {
         .addFormatEach("-l%s", libraryNames);
 
     if (!objcConfiguration.shouldPrioritizeStaticLibs()) {
-      commandLine
-          .addExecPaths(bazelBuiltLibraries)
-          .addExecPaths(objcProvider.get(IMPORTED_LIBRARY))
-          .addExecPaths(ccLibraries);
+      commandLine.add("-filelist").add(inputFileList.getExecPathString());
     }
 
-    Iterable<Artifact> ccLibrariesToForceLoad =
-        Iterables.filter(ccLibraries, ALWAYS_LINKED_CC_LIBRARY);
-
     commandLine
         .addExecPath("-o", linkedBinary)
-        .addBeforeEach("-force_load", Artifact.toExecPaths(objcProvider.get(FORCE_LOAD_LIBRARY)))
-        .addBeforeEach("-force_load", Artifact.toExecPaths(ccLibrariesToForceLoad))
+        .addBeforeEachExecPath("-force_load", forceLinkArtifacts)
         .add(extraLinkArgs)
         .add(objcProvider.get(ObjcProvider.LINKOPT));
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
index 099db303ce..ec5c05564f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
@@ -149,11 +149,19 @@ public final class IntermediateArtifacts {
   }
 
   /**
+   * The .objlist file, which contains a list of paths of object files to archive and is read by
+   * clang (via -filelist flag) in the link action (for binary creation).
+   */
+  public Artifact linkerObjList() {
+    return appendExtension("-linker.objlist");
+  }
+
+  /**
    * The .objlist file, which contains a list of paths of object files to archive  and is read by
-   * libtool in the archive action.
+   * libtool (via -filelist flag) in the archive action.
    */
-  public Artifact objList() {
-    return appendExtension(".objlist");
+  public Artifact archiveObjList() {
+    return appendExtension("-archive.objlist");
   }
 
   /**