Expose type of library_to_link in CROSSTOOL

This cl relieves us from hard-coding -l and -l: flags in Bazel. To be able to
express the behavior in CROSSTOOL, we need to know what type of library are we
dealing with.

--
PiperOrigin-RevId: 145271426
MOS_MIGRATED_REVID=145271426

5 files changed, 256 insertions, 50 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
index d3e83505ef..47a9fa667e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -987,25 +987,64 @@ public class CcToolchainFeatures implements Serializable {
      * significantly reduces memory overhead.
      */
     @Immutable
-    public static final class LibraryToLinkValue implements VariableValue {
+    public static class LibraryToLinkValue implements VariableValue {
 
-      public static final String NAMES_FIELD_NAME = "names";
+      public static final String OBJECT_FILES_FIELD_NAME = "object_files";
+      public static final String NAME_FIELD_NAME = "name";
+      public static final String TYPE_FIELD_NAME = "type";
       public static final String IS_WHOLE_ARCHIVE_FIELD_NAME = "is_whole_archive";
-      public static final String IS_LIB_GROUP_FIELD_NAME = "is_lib_group";
+      private enum Type {
+        OBJECT_FILE("object_file"),
+        OBJECT_FILE_GROUP("object_file_group"),
+        INTERFACE_LIBRARY("interface_library"),
+        STATIC_LIBRARY("static_library"),
+        DYNAMIC_LIBRARY("dynamic_library"),
+        VERSIONED_DYNAMIC_LIBRARY("versioned_dynamic_library");
+
+        private final String name;
+
+        Type(String name) {
+          this.name = name;
+        }
+      }
 
-      private final ImmutableList<String> names;
+      private final String name;
+      private final ImmutableList<String> objectFiles;
       private final boolean isWholeArchive;
-      private final boolean isLibGroup;
+      private final Type type;
+
+      public static LibraryToLinkValue forDynamicLibrary(String name, boolean isWholeArchive) {
+        return new LibraryToLinkValue(name, null, isWholeArchive, Type.DYNAMIC_LIBRARY);
+      }
+
+      public static LibraryToLinkValue forVersionedDynamicLibrary(
+          String name, boolean isWholeArchive) {
+        return new LibraryToLinkValue(name, null, isWholeArchive, Type.VERSIONED_DYNAMIC_LIBRARY);
+      }
+
+      public static LibraryToLinkValue forInterfaceLibrary(String name, boolean isWholeArchive) {
+        return new LibraryToLinkValue(name, null, isWholeArchive, Type.INTERFACE_LIBRARY);
+      }
 
-      public LibraryToLinkValue(String name, boolean isWholeArchive) {
-        this(ImmutableList.of(name), isWholeArchive, false);
+      public static LibraryToLinkValue forStaticLibrary(String name, boolean isWholeArchive) {
+        return new LibraryToLinkValue(name, null, isWholeArchive, Type.STATIC_LIBRARY);
       }
 
-      public LibraryToLinkValue(
-          ImmutableList<String> names, boolean isWholeArchive, boolean isLibGroup) {
-        this.names = names;
+      public static LibraryToLinkValue forObjectFile(String name, boolean isWholeArchive) {
+        return new LibraryToLinkValue(name, null, isWholeArchive, Type.OBJECT_FILE);
+      }
+
+      public static LibraryToLinkValue forObjectFileGroup(
+          ImmutableList<String> objects, boolean isWholeArchive) {
+        return new LibraryToLinkValue(null, objects, isWholeArchive, Type.OBJECT_FILE_GROUP);
+      }
+
+      private LibraryToLinkValue(
+          String name, ImmutableList<String> objectFiles, boolean isWholeArchive, Type type) {
+        this.name = name;
+        this.objectFiles = objectFiles;
         this.isWholeArchive = isWholeArchive;
-        this.isLibGroup = isLibGroup;
+        this.type = type;
       }
 
       @Override
@@ -1025,22 +1064,27 @@ public class CcToolchainFeatures implements Serializable {
       @Override
       public VariableValue getFieldValue(String variableName, String field) {
         Preconditions.checkNotNull(field);
-        if (NAMES_FIELD_NAME.equals(field)) {
-          return new StringSequence(names);
+        if (NAME_FIELD_NAME.equals(field) && !type.equals(Type.OBJECT_FILE_GROUP)) {
+          return new StringValue(name);
+        } else if (OBJECT_FILES_FIELD_NAME.equals(field) && type.equals(Type.OBJECT_FILE_GROUP)) {
+          return new StringSequence(objectFiles);
+        } else if (TYPE_FIELD_NAME.equals(field)) {
+          return new StringValue(type.name);
         } else if (IS_WHOLE_ARCHIVE_FIELD_NAME.equals(field)) {
           return new IntegerValue(isWholeArchive ? 1 : 0);
-        } else if (IS_LIB_GROUP_FIELD_NAME.equals(field)) {
-          return new IntegerValue(isLibGroup ? 1 : 0);
         } else if ("whole_archive_presence".equals(field)) {
           // TODO(b/33403458): Cleanup this workaround once bazel >=0.4.3 is released.
           return isWholeArchive ? new IntegerValue(0) : null;
         } else if ("no_whole_archive_presence".equals(field)) {
           // TODO(b/33403458): Cleanup this workaround once bazel >=0.4.3 is released.
           return !isWholeArchive ? new IntegerValue(0) : null;
-        } else if ("lib_group_presence".equals(field)) {
-          // TODO(b/33403458): Cleanup this workaround once bazel >=0.4.3 is released.
-          return isLibGroup ? new IntegerValue(0) : null;
         } else {
+          // TODO(b/33403458): Cleanup this workaround once bazel >=0.4.3 is released.
+          for (Type t : Type.values()) {
+            if ((t.name + "_presence").equals(field)) {
+              return type.equals(t) ? new IntegerValue(0) : null;
+            }
+          }
           return null;
         }
       }
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 6bd3ca64c3..d0c8e49abb 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
@@ -1613,19 +1613,22 @@ public class CppLinkActionBuilder {
       if (CppFileTypes.SHARED_LIBRARY.matches(name)) {
         // Use normal shared library resolution rules for shared libraries.
         String libName = name.replaceAll("(^lib|\\.(so|dylib)$)", "");
-        librariesToLink.addValue(new LibraryToLinkValue("-l" + libName, inputIsWholeArchive));
+        librariesToLink.addValue(
+            LibraryToLinkValue.forDynamicLibrary(libName, inputIsWholeArchive));
       } else if (CppFileTypes.VERSIONED_SHARED_LIBRARY.matches(name)) {
         // Versioned shared libraries require the exact library filename, e.g.:
         // -lfoo -> libfoo.so
         // -l:libfoo.so.1 -> libfoo.so.1
-        librariesToLink.addValue(new LibraryToLinkValue("-l:" + name, inputIsWholeArchive));
+        librariesToLink.addValue(
+            LibraryToLinkValue.forVersionedDynamicLibrary(name, inputIsWholeArchive));
       } else {
         // Interface shared objects have a non-standard extension
         // that the linker won't be able to find.  So use the
         // filename directly rather than a -l option.  Since the
         // library has an SONAME attribute, this will work fine.
         librariesToLink.addValue(
-            new LibraryToLinkValue(inputArtifact.getExecPathString(), inputIsWholeArchive));
+            LibraryToLinkValue.forInterfaceLibrary(
+                inputArtifact.getExecPathString(), inputIsWholeArchive));
       }
     }
 
@@ -1640,7 +1643,8 @@ public class CppLinkActionBuilder {
         SequenceBuilder librariesToLink,
         boolean isRuntimeLinkerInput,
         @Nullable Map<Artifact, Artifact> ltoMap) {
-      Preconditions.checkState(!(input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY));
+      ArtifactCategory artifactCategory = input.getArtifactCategory();
+      Preconditions.checkState(artifactCategory != ArtifactCategory.DYNAMIC_LIBRARY);
       // If we had any LTO artifacts, ltoMap whould be non-null. In that case,
       // we should have created a thinltoParamFile which the LTO indexing
       // step will populate with the exec paths that correspond to the LTO
@@ -1669,33 +1673,38 @@ public class CppLinkActionBuilder {
           if (!nonLTOArchiveMembers.isEmpty()) {
             boolean inputIsWholeArchive = !isRuntimeLinkerInput && needWholeArchive;
             librariesToLink.addValue(
-                new LibraryToLinkValue(nonLTOArchiveMembers, inputIsWholeArchive, true));
+                LibraryToLinkValue.forObjectFileGroup(nonLTOArchiveMembers, inputIsWholeArchive));
           }
         }
       } else {
+        Preconditions.checkArgument(
+            artifactCategory.equals(ArtifactCategory.OBJECT_FILE)
+                || artifactCategory.equals(ArtifactCategory.STATIC_LIBRARY)
+                || artifactCategory.equals(ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY));
         boolean isAlwaysLinkStaticLibrary =
-            input.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY;
-        // For anything else, add the input directly.
-        Artifact inputArtifact = input.getArtifact();
+            artifactCategory == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY;
         boolean inputIsWholeArchive =
             (!isRuntimeLinkerInput && (isAlwaysLinkStaticLibrary || needWholeArchive))
                 || (isRuntimeLinkerInput && isAlwaysLinkStaticLibrary && !needWholeArchive);
 
+        Artifact inputArtifact = input.getArtifact();
         if (ltoMap != null && ltoMap.remove(inputArtifact) != null) {
           // The LTO artifacts that should be included in the final link
           // are listed in the thinltoParamFile.
           return;
         }
 
+        String name;
         if (input.isFake()) {
-          librariesToLink.addValue(
-              new LibraryToLinkValue(
-                  Link.FAKE_OBJECT_PREFIX + inputArtifact.getExecPathString(),
-                  inputIsWholeArchive));
+          name = Link.FAKE_OBJECT_PREFIX + inputArtifact.getExecPathString();
         } else {
-          librariesToLink.addValue(
-              new LibraryToLinkValue(inputArtifact.getExecPathString(), inputIsWholeArchive));
+          name = inputArtifact.getExecPathString();
         }
+
+        librariesToLink.addValue(
+            artifactCategory.equals(ArtifactCategory.OBJECT_FILE)
+                ? LibraryToLinkValue.forObjectFile(name, inputIsWholeArchive)
+                : LibraryToLinkValue.forStaticLibrary(name, inputIsWholeArchive));
       }
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
index f0bd59a746..d614c3d063 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
@@ -294,7 +294,10 @@ public class CppLinkActionConfigs {
                 "       flag_group {",
                 "           iterate_over: 'libraries_to_link'",
                 "           flag_group {",
-                "               expand_if_true: 'libraries_to_link.is_lib_group'",
+                "               expand_if_equal: {",
+                "                   variable: 'libraries_to_link.type'",
+                "                   value: 'object_file_group'",
+                "               }",
                 "               flag: '-Wl,--start-lib'",
                 "           }",
                 ifLinux(
@@ -304,8 +307,47 @@ public class CppLinkActionConfigs {
                     "           flag: '-Wl,-whole-archive'",
                     "       }",
                     "       flag_group {",
-                    "           iterate_over: 'libraries_to_link.names'",
-                    "           flag: '%{libraries_to_link.names}'",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'object_file_group'",
+                    "           }",
+                    "           iterate_over: 'libraries_to_link.object_files'",
+                    "           flag: '%{libraries_to_link.object_files}'",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'object_file'",
+                    "           }",
+                    "           flag: '%{libraries_to_link.name}'",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'interface_library'",
+                    "           }",
+                    "           flag: '%{libraries_to_link.name}'",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'static_library'",
+                    "           }",
+                    "           flag: '%{libraries_to_link.name}'",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'dynamic_library'",
+                    "           }",
+                    "           flag: '-l%{libraries_to_link.name}'",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'versioned_dynamic_library'",
+                    "           }",
+                    "           flag: '-l:%{libraries_to_link.name}'",
                     "       }",
                     "       flag_group {",
                     "           expand_if_true: 'libraries_to_link.is_whole_archive'",
@@ -314,17 +356,95 @@ public class CppLinkActionConfigs {
                 ifMac(
                     platform,
                     "       flag_group {",
-                    "           expand_if_true: 'libraries_to_link.is_whole_archive'",
-                    "           iterate_over: 'libraries_to_link.names'",
-                    "           flag: '-Wl,-force_load,%{libraries_to_link.names}'",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'object_file_group'",
+                    "           }",
+                    "           iterate_over: 'libraries_to_link.object_files'",
+                    "           flag_group {",
+                    "               expand_if_false: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '%{libraries_to_link.object_files}'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-Wl,-force_load,%{libraries_to_link.object_files}'",
+                    "           }",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'object_file'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_false: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '%{libraries_to_link.name}'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-Wl,-force_load,%{libraries_to_link.name}'",
+                    "           }",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'interface_library'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_false: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '%{libraries_to_link.name}'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-Wl,-force_load,%{libraries_to_link.name}'",
+                    "           }",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'static_library'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_false: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '%{libraries_to_link.name}'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-Wl,-force_load,%{libraries_to_link.name}'",
+                    "           }",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'dynamic_library'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_false: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-l%{libraries_to_link.name}'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-Wl,-force_load,-l%{libraries_to_link.name}'",
+                    "           }",
                     "       }",
                     "       flag_group {",
-                    "           expand_if_false: 'libraries_to_link.is_whole_archive'",
-                    "           iterate_over: 'libraries_to_link.names'",
-                    "           flag: '%{libraries_to_link.names}'",
+                    "           expand_if_equal: {",
+                    "               variable: 'libraries_to_link.type'",
+                    "               value: 'versioned_dynamic_library'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_false: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-l:%{libraries_to_link.name}'",
+                    "           }",
+                    "           flag_group {",
+                    "               expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "               flag: '-Wl,-force_load,-l:%{libraries_to_link.name}'",
+                    "           }",
                     "       }"),
                 "           flag_group {",
-                "               expand_if_true: 'libraries_to_link.is_lib_group'",
+                "               expand_if_equal: {",
+                "                   variable: 'libraries_to_link.type'",
+                "                   value: 'object_file_group'",
+                "               }",
                 "               flag: '-Wl,--end-lib'",
                 "           }",
                 "       }",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
index 35ae31818b..52ebddbae2 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java
@@ -28,9 +28,11 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionExce
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.IntegerValue;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.LibraryToLinkValue;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.SequenceBuilder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.StringSequenceBuilder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.StructureBuilder;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariableValue;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariableValueBuilder;
 import com.google.devtools.build.lib.testutil.Suite;
 import com.google.devtools.build.lib.testutil.TestSpec;
@@ -1335,4 +1337,31 @@ public class CcToolchainFeaturesTest {
           .contains(String.format(ActionConfig.FLAG_SET_WITH_ACTION_ERROR, "c++-compile"));
     }
   }
+
+  @Test
+  public void testLibraryToLinkValue() {
+    assertThat(
+            LibraryToLinkValue.forDynamicLibrary("foo", false)
+                .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.NAME_FIELD_NAME)
+                .getStringValue(LibraryToLinkValue.NAME_FIELD_NAME))
+        .isEqualTo("foo");
+    assertThat(
+            LibraryToLinkValue.forDynamicLibrary("foo", false)
+                .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.OBJECT_FILES_FIELD_NAME))
+        .isNull();
+
+    assertThat(
+            LibraryToLinkValue.forObjectFileGroup(ImmutableList.of("foo", "bar"), false)
+                .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.NAME_FIELD_NAME))
+        .isNull();
+    Iterable<? extends VariableValue> objects =
+        LibraryToLinkValue.forObjectFileGroup(ImmutableList.of("foo", "bar"), false)
+            .getFieldValue("LibraryToLinkValue", LibraryToLinkValue.OBJECT_FILES_FIELD_NAME)
+            .getSequenceValue(LibraryToLinkValue.OBJECT_FILES_FIELD_NAME);
+    ImmutableList.Builder<String> objectNames = ImmutableList.builder();
+    for (VariableValue object : objects) {
+      objectNames.add(object.getStringValue("name"));
+    }
+    assertThat(objectNames.build()).containsExactly("foo", "bar");
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
index e8c86bf443..6ea0a1342d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
@@ -130,18 +130,22 @@ public class LinkBuildVariablesTest extends BuildViewTestCase {
 
     ConfiguredTarget target = getConfiguredTarget("//x:foo");
     Variables variables = getLinkBuildVariables(target, LinkTargetType.DYNAMIC_LIBRARY);
-    VariableValue librariesToLinkSequence = variables.getVariable("libraries_to_link");
+    VariableValue librariesToLinkSequence =
+        variables.getVariable(CppLinkActionBuilder.LIBRARIES_TO_LINK_VARIABLE);
     assertThat(librariesToLinkSequence).isNotNull();
     Iterable<? extends VariableValue> librariesToLink =
-        librariesToLinkSequence.getSequenceValue("libraries_to_link");
+        librariesToLinkSequence.getSequenceValue(CppLinkActionBuilder.LIBRARIES_TO_LINK_VARIABLE);
     assertThat(librariesToLink).hasSize(1);
-    VariableValue nameValue = librariesToLink.iterator().next().getFieldValue(
-        "librariesToLink", LibraryToLinkValue.NAMES_FIELD_NAME);
+    VariableValue nameValue =
+        librariesToLink
+            .iterator()
+            .next()
+            .getFieldValue(
+                CppLinkActionBuilder.LIBRARIES_TO_LINK_VARIABLE,
+                LibraryToLinkValue.NAME_FIELD_NAME);
     assertThat(nameValue).isNotNull();
-    Iterable<? extends VariableValue> names = nameValue.getSequenceValue("names");
-    assertThat(names).isNotNull();
-    assertThat(names).hasSize(1);
-    assertThat(names.iterator().next().getStringValue("names")).matches(".*a\\..*o");
+    String name = nameValue.getStringValue(LibraryToLinkValue.NAME_FIELD_NAME);
+    assertThat(name).matches(".*a\\..*o");
   }
 
   @Test