ide-compile output group now mirrors the compile-only output group for cc rules

--
MOS_MIGRATED_REVID=135816762

4 files changed, 25 insertions, 45 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java
index a6c84893e5..deab1cff3d 100644
--- a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect.Builder;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.OutputGroupProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
@@ -67,11 +68,9 @@ import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.android.AndroidIdeInfoProvider;
 import com.google.devtools.build.lib.rules.android.AndroidIdeInfoProvider.SourceDirectory;
 import com.google.devtools.build.lib.rules.android.AndroidSdkProvider;
-import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
 import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
-import com.google.devtools.build.lib.rules.cpp.LinkerInput;
 import com.google.devtools.build.lib.rules.java.JavaExportsProvider;
 import com.google.devtools.build.lib.rules.java.JavaGenJarsProvider;
 import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
@@ -197,11 +196,19 @@ public class AndroidStudioInfoAspect extends NativeAspectClass implements Config
         dependenciesResult,
         providerBuilder);
 
+    NestedSetBuilder<Artifact> ideCompileArtifacts = NestedSetBuilder.stableOrder();
+    // Add artifacts required for compilation
+    OutputGroupProvider outputGroupProvider = base.getProvider(OutputGroupProvider.class);
+    if (outputGroupProvider != null) {
+      ideCompileArtifacts.addTransitive(
+          outputGroupProvider.getOutputGroup(OutputGroupProvider.FILES_TO_COMPILE));
+    }
+
     builder
         .addOutputGroup(IDE_INFO, provider.getIdeInfoFiles())
         .addOutputGroup(IDE_INFO_TEXT, provider.getIdeInfoTextFiles())
         .addOutputGroup(IDE_RESOLVE, provider.getIdeResolveFiles())
-        .addOutputGroup(IDE_COMPILE, provider.getIdeCompileFiles())
+        .addOutputGroup(IDE_COMPILE, ideCompileArtifacts.build())
         .addProvider(provider);
 
     return builder.build();
@@ -296,10 +303,8 @@ public class AndroidStudioInfoAspect extends NativeAspectClass implements Config
       providerBuilder.ideInfoFilesBuilder().addTransitive(depProvider.getIdeInfoFiles());
       providerBuilder.ideInfoTextFilesBuilder().addTransitive(depProvider.getIdeInfoTextFiles());
       providerBuilder.ideResolveFilesBuilder().addTransitive(depProvider.getIdeResolveFiles());
-      providerBuilder.ideCompileFilesBuilder().addTransitive(depProvider.getIdeCompileFiles());
     }
 
-
     return new DependenciesResult(
         dependencies,
         runtimeDepsBuilder.build(),
@@ -318,7 +323,6 @@ public class AndroidStudioInfoAspect extends NativeAspectClass implements Config
     providerBuilder.ideInfoFilesBuilder().add(ideInfoFile);
     providerBuilder.ideInfoTextFilesBuilder().add(ideInfoTextFile);
     NestedSetBuilder<Artifact> ideResolveArtifacts = providerBuilder.ideResolveFilesBuilder();
-    NestedSetBuilder<Artifact> ideCompileArtifacts = providerBuilder.ideCompileFilesBuilder();
 
     RuleIdeInfo.Builder outputBuilder = RuleIdeInfo.newBuilder();
 
@@ -345,8 +349,8 @@ public class AndroidStudioInfoAspect extends NativeAspectClass implements Config
     if (isCppRule(base)) {
       CppCompilationContext cppCompilationContext = base.getProvider(CppCompilationContext.class);
       if (cppCompilationContext != null) {
-        CRuleIdeInfo cRuleIdeInfo = makeCRuleIdeInfo(base, ruleContext,
-            cppCompilationContext, ideResolveArtifacts, ideCompileArtifacts);
+        CRuleIdeInfo cRuleIdeInfo = makeCRuleIdeInfo(base, ruleContext, cppCompilationContext,
+            ideResolveArtifacts);
         outputBuilder.setCRuleIdeInfo(cRuleIdeInfo);
       }
     }
@@ -679,8 +683,7 @@ public class AndroidStudioInfoAspect extends NativeAspectClass implements Config
       ConfiguredTarget base,
       RuleContext ruleContext,
       CppCompilationContext cppCompilationContext,
-      NestedSetBuilder<Artifact> ideResolveArtifacts,
-      NestedSetBuilder<Artifact> ideCompileArtifacts) {
+      NestedSetBuilder<Artifact> ideResolveArtifacts) {
     CRuleIdeInfo.Builder builder = CRuleIdeInfo.newBuilder();
 
     Collection<Artifact> sourceFiles = getSources(ruleContext);
@@ -715,14 +718,6 @@ public class AndroidStudioInfoAspect extends NativeAspectClass implements Config
       builder.addTransitiveSystemIncludeDirectory(pathFragment.getSafePathString());
     }
 
-    // Add libs to ide-compile
-    CcLinkParamsProvider ccLinkParams = base.getProvider(CcLinkParamsProvider.class);
-    if (ccLinkParams != null) {
-      for (LinkerInput lib : ccLinkParams.getCcLinkParams(true, false).getLibraries()) {
-        ideCompileArtifacts.add(lib.getArtifact());
-      }
-    }
-
     androidStudioInfoSemantics.augmentCppRuleInfo(
         builder, base, ruleContext, cppCompilationContext, ideResolveArtifacts);
 
diff --git a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoFilesProvider.java b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoFilesProvider.java
index 04af6749a6..8f57d965d9 100644
--- a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoFilesProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoFilesProvider.java
@@ -29,7 +29,6 @@ public final class AndroidStudioInfoFilesProvider implements TransitiveInfoProvi
   private final NestedSet<Artifact> ideInfoFiles;
   private final NestedSet<Artifact> ideInfoTextFiles;
   private final NestedSet<Artifact> ideResolveFiles;
-  private final NestedSet<Artifact> ideCompileFiles;
   private final NestedSet<Label> exportedDeps;
 
   /**
@@ -39,14 +38,12 @@ public final class AndroidStudioInfoFilesProvider implements TransitiveInfoProvi
     private final NestedSetBuilder<Artifact> ideInfoFilesBuilder;
     private final NestedSetBuilder<Artifact> ideInfoTextFilesBuilder;
     private final NestedSetBuilder<Artifact> ideResolveFilesBuilder;
-    private final NestedSetBuilder<Artifact> ideCompileFilesBuilder;
     private NestedSetBuilder<Label> exportedDepsBuilder;
 
     public Builder() {
       ideInfoFilesBuilder = NestedSetBuilder.stableOrder();
       ideInfoTextFilesBuilder = NestedSetBuilder.stableOrder();
       ideResolveFilesBuilder = NestedSetBuilder.stableOrder();
-      ideCompileFilesBuilder = NestedSetBuilder.stableOrder();
       exportedDepsBuilder = NestedSetBuilder.stableOrder();
     }
 
@@ -62,10 +59,6 @@ public final class AndroidStudioInfoFilesProvider implements TransitiveInfoProvi
       return ideResolveFilesBuilder;
     }
 
-    public NestedSetBuilder<Artifact> ideCompileFilesBuilder() {
-      return ideCompileFilesBuilder;
-    }
-
     public NestedSetBuilder<Label> exportedDepsBuilder() {
       return exportedDepsBuilder;
     }
@@ -75,7 +68,6 @@ public final class AndroidStudioInfoFilesProvider implements TransitiveInfoProvi
           ideInfoFilesBuilder.build(),
           ideInfoTextFilesBuilder.build(),
           ideResolveFilesBuilder.build(),
-          ideCompileFilesBuilder.build(),
           exportedDepsBuilder.build()
       );
     }
@@ -85,12 +77,10 @@ public final class AndroidStudioInfoFilesProvider implements TransitiveInfoProvi
       NestedSet<Artifact> ideInfoFiles,
       NestedSet<Artifact> ideInfoTextFiles,
       NestedSet<Artifact> ideResolveFiles,
-      NestedSet<Artifact> ideCompileFiles,
       NestedSet<Label> exportedDeps) {
     this.ideInfoFiles = ideInfoFiles;
     this.ideInfoTextFiles = ideInfoTextFiles;
     this.ideResolveFiles = ideResolveFiles;
-    this.ideCompileFiles = ideCompileFiles;
     this.exportedDeps = exportedDeps;
   }
 
@@ -106,10 +96,6 @@ public final class AndroidStudioInfoFilesProvider implements TransitiveInfoProvi
     return ideResolveFiles;
   }
 
-  public NestedSet<Artifact> getIdeCompileFiles() {
-    return ideCompileFiles;
-  }
-
   public NestedSet<Label> getExportedDeps() {
     return exportedDeps;
   }
diff --git a/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java b/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java
index 5d5f7db50c..bd5f2414e0 100644
--- a/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java
@@ -1390,18 +1390,20 @@ public class AndroidStudioInfoAspectTest extends AndroidStudioInfoAspectTestBase
 
   @Test
   public void testSimpleCCLibraryWithDeps() throws Exception {
+    // Specify '-fPIC' so that compilation output filenames are consistent for mac and linux.
     scratch.file(
         "com/google/example/BUILD",
         "cc_library(",
-        "   name = 'lib',",
-        "   srcs = ['lib/lib.cc'],",
-        "   hdrs = ['lib/lib.h'],",
+        "    name = 'lib',",
+        "    srcs = ['lib/lib.cc'],",
+        "    hdrs = ['lib/lib.h'],",
         ")",
         "cc_library(",
         "    name = 'simple',",
         "    srcs = ['simple/simple.cc'],",
         "    hdrs = ['simple/simple.h'],",
         "    deps = [':lib'],",
+        "    nocopts = '-fPIC',",
         ")");
     Map<String, RuleIdeInfo> ruleIdeInfos = buildRuleIdeInfo("//com/google/example:simple");
     assertThat(ruleIdeInfos).hasSize(3);
@@ -1412,8 +1414,7 @@ public class AndroidStudioInfoAspectTest extends AndroidStudioInfoAspectTestBase
     assertThat(ruleIdeInfo.getDependenciesList()).hasSize(2);
 
     assertThat(getIdeCompileFiles()).containsExactly(
-        "com/google/example/liblib.a",
-        "com/google/example/libsimple.a");
+        "com/google/example/_objs/simple/com/google/example/simple/simple.o");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/ideinfo/intellij_info.bzl b/src/test/java/com/google/devtools/build/lib/ideinfo/intellij_info.bzl
index 4d2cfa8ff1..f179b68727 100644
--- a/src/test/java/com/google/devtools/build/lib/ideinfo/intellij_info.bzl
+++ b/src/test/java/com/google/devtools/build/lib/ideinfo/intellij_info.bzl
@@ -198,11 +198,11 @@ def tool_label(label_str):
 def build_c_rule_ide_info(target, ctx):
   """Build CRuleIdeInfo.
 
-  Returns a tuple of (CRuleIdeInfo proto, a set of ide-resolve-files, a set of ide-compile-files).
-  (or (None, empty set, empty set) if the rule is not a C rule).
+  Returns a tuple of (CRuleIdeInfo proto, a set of ide-resolve-files).
+  (or (None, empty set) if the rule is not a C rule).
   """
   if not hasattr(target, "cc"):
-    return (None, set(), set())
+    return (None, set())
 
   sources = sources_from_rule(ctx)
 
@@ -229,8 +229,7 @@ def build_c_rule_ide_info(target, ctx):
       transitive_system_include_directory = cc_provider.system_include_directories,
   )
   ide_resolve_files = cc_provider.transitive_headers
-  ide_compile_files = cc_provider.libs
-  return (c_rule_ide_info, ide_resolve_files, ide_compile_files)
+  return (c_rule_ide_info, ide_resolve_files)
 
 def build_c_toolchain_ide_info(target, ctx):
   """Build CToolchainIdeInfo.
@@ -476,7 +475,7 @@ def _aspect_impl_helper(target, ctx, for_test):
   prerequisites = direct_dep_targets + runtime_dep_targets + list_omit_none(legacy_resource_target)
   ide_info_text = set()
   ide_resolve_files = set()
-  ide_compile_files = set()
+  ide_compile_files = target.output_group("files_to_compile_INTERNAL_")
   intellij_infos = dict()
   for dep in prerequisites:
     ide_info_text = ide_info_text | dep.intellij_info_files.ide_info_text
@@ -485,9 +484,8 @@ def _aspect_impl_helper(target, ctx, for_test):
       intellij_infos.update(dep.intellij_infos)
 
   # Collect C-specific information
-  (c_rule_ide_info, c_ide_resolve_files, c_ide_compile_files) = build_c_rule_ide_info(target, ctx)
+  (c_rule_ide_info, c_ide_resolve_files) = build_c_rule_ide_info(target, ctx)
   ide_resolve_files = ide_resolve_files | c_ide_resolve_files
-  ide_compile_files = ide_compile_files | c_ide_compile_files
 
   (c_toolchain_ide_info, c_toolchain_ide_resolve_files) = build_c_toolchain_ide_info(target, ctx)
   ide_resolve_files = ide_resolve_files | c_toolchain_ide_resolve_files