Repository: bazelbuild/bazel
Issue: Duplicate symbols from apple_static_library with Swift
Labels: type: bug
P3
team-Rules-CPP

When using the `ios_static_framework` rule from rules_apple, which has an underlying `apple_static_library` target. If you have a dependency tree like this:

```
ios_static_framework
swift_library
objc_library
cc_library
```

The symbols from the `cc_library` target end up duplicated in the final binary. Our assumption is that it has something to do with this comment https://github.com/bazelbuild/bazel/blob/1645eb8ac6a1df5f7f41599039113bba9f081c89/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java#L760-L765 since the `cc_library` targets get added to the ObjcProvider's `library` field by rules_swift https://github.com/bazelbuild/rules_swift/blob/2ebfc403ffdb428befd38e25e236ae181aa1a49b/swift/internal/compiling.bzl#L1486-L1501 which is required for normal cases, but appears to cause this duplication issue when fully linking the binary.

Here's a small sample project that repros this: [duplicatesymbols.zip](https://github.com/bazelbuild/bazel/files/4531969/duplicatesymbols.zip)

With this project run `bazel build ios_framework`, you will then see this warning:

```
/Applications/Xcode-11.3.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning same member name (ccfoo_8ef19ba5cbaeb2c2d18053f5cec4195e.o) in output file used for input files: bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild-ST-96e8508176c304a494afa69d0efb2bff98cc46f1bcec8e953485b6ad02335d21/bin/libcclib.a(ccfoo_8ef19ba5cbaeb2c2d18053f5cec4195e.o) and: bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild-ST-96e8508176c304a494afa69d0efb2bff98cc46f1bcec8e953485b6ad02335d21/bin/libcclib.a(ccfoo_8ef19ba5cbaeb2c2d18053f5cec4195e.o) due to use of basename, truncation and blank paddingf
```

and if you unzip the produced framework and run `nm` on it you can see the duplication:

```
% nm swiftlib.framework/swiftlib

swiftlib.framework/swiftlib(swiftfoo.swift_942f052b5affefc31fa39383ae5d7835.o):
0000000000000000 T _$s8swiftlib3fooyyF
0000000000000018 S ___swift_reflection_version
                 U __swift_FORCE_LOAD_$_swiftCompatibility50
0000000000000008 S __swift_FORCE_LOAD_$_swiftCompatibility50_$_swiftlib
                 U __swift_FORCE_LOAD_$_swiftCompatibilityDynamicReplacements
0000000000000010 S __swift_FORCE_LOAD_$_swiftCompatibilityDynamicReplacements_$_swiftlib
000000000000001a s l_llvm.swift_module_hash

swiftlib.framework/swiftlib(objcfoo_a34b0088e5e8bcbb11ab6edade8e425b.o):
                 U _OBJC_CLASS_$_NSObject
00000000000000c0 S _OBJC_CLASS_$_objcfoo
                 U _OBJC_METACLASS_$_NSObject
0000000000000098 S _OBJC_METACLASS_$_objcfoo
                 U __objc_empty_cache
0000000000000050 s l_OBJC_CLASS_RO_$_objcfoo
0000000000000008 s l_OBJC_METACLASS_RO_$_objcfoo

swiftlib.framework/swiftlib(ccfoo_8ef19ba5cbaeb2c2d18053f5cec4195e.o):
0000000000000000 T __Z6cc_foov

swiftlib.framework/swiftlib(ccfoo_8ef19ba5cbaeb2c2d18053f5cec4195e.o):
0000000000000000 T __Z6cc_foov
```

where `__Z6cc_foov` should only be there once.

You can also see the cc_library archive is passed twice in the libtool invocation:

```
external/local_config_cc/libtool \
 -no_warning_for_no_symbols \
 -static \
 -arch_only x86_64 \
 -syslibroot __BAZEL_XCODE_SDKROOT__ \
  -o \
  bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild-ST-96e8508176c304a494afa69d0efb2bff98cc46f1bcec8e953485b6ad02335d21/bin/ios_framework.apple_static_library-fl.a \
  bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild-ST-96e8508176c304a494afa69d0efb2bff98cc46f1bcec8e953485b6ad02335d21/bin/libswiftlib.a \
  bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild-ST-96e8508176c304a494afa69d0efb2bff98cc46f1bcec8e953485b6ad02335d21/bin/libobjclib.a \
  bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild-ST-96e8508176c304a494afa69d0efb2bff98cc46f1bcec8e953485b6ad02335d21/bin/libcclib.a \
  bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild-ST-96e8508176c304a494afa69d0efb2bff98cc46f1bcec8e953485b6ad02335d21/bin/libcclib.a
```

### What operating system are you running Bazel on?

macOS

### What's the output of `bazel info release`?

release 3.1.0

###  Have you found anything relevant by searching the web?

No, it sounds like Google probably isn't using `ios_static_framework` with Swift much yet, so this hasn't been hit internally.

### Any other information, logs, or outputs that you want to share?

I spent some time poking around this code:

https://github.com/bazelbuild/bazel/blob/f7d726f88f20c871d274b75a01f6a64d186403dc/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java#L189-L201

since I expect adding both sets of libraries here may be related, but I haven't been able to come up with a fix.

Issue comments:
I've submitted a potential fix for this that appears to solve my repro case locally https://github.com/bazelbuild/bazel/pull/11225

Pull request review comments:
```suggestion
    ImmutableSet<Artifact> ccLibs = ImmutableSet.copyOf(objcProvider.getCcLibraries());
```
Thanks! I was trying to find the nicer invocation here
`copyOf` is most likely more efficient too. Depending on the type of `getCcLibraries` it can allocate a set of right size and copy elements from it without resizing. A builder allocates a buffer of default size which is fine for small collections, but with large collections it would thrown away right after `addAll` call.

Patch that resolved the issue:
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 969e419e418ed1..160ca6939a6aac 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
@@ -18,8 +18,10 @@
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LINKOPT;
 
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -189,9 +191,18 @@ private void addArchiveVariables(CcToolchainVariables.Builder builder) {
   private void addFullyLinkArchiveVariables(CcToolchainVariables.Builder builder) {
     builder.addStringVariable(
         FULLY_LINKED_ARCHIVE_PATH_VARIABLE_NAME, fullyLinkArchive.getExecPathString());
+
+    // ObjcProvider.getObjcLibraries contains both libraries from objc providers
+    // as well as those from CcInfo. ObjcProvider.getCcLibraries only contains
+    // those from CcInfo. We have to split these lists to make sure duplicate
+    // libraries are not included in the fully linked archive.
+    ImmutableSet<Artifact> ccLibs = ImmutableSet.copyOf(objcProvider.getCcLibraries());
+    Predicate<Artifact> isNotCcLib = library -> !ccLibs.contains(library);
+    Iterable<Artifact> objcLibraries = Iterables.filter(objcProvider.getObjcLibraries(), isNotCcLib);
+
     builder.addStringSequenceVariable(
         OBJC_LIBRARY_EXEC_PATHS_VARIABLE_NAME,
-        Artifact.toExecPaths(objcProvider.getObjcLibraries()));
+        Artifact.toExecPaths(objcLibraries));
     builder.addStringSequenceVariable(
         CC_LIBRARY_EXEC_PATHS_VARIABLE_NAME,
         Artifact.toExecPaths(objcProvider.getCcLibraries()));


Test patch: