Remove TempsProvider in favor of an output group.

--
MOS_MIGRATED_REVID=87136119

8 files changed, 33 insertions, 68 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TempsProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/TempsProvider.java
deleted file mode 100644
index 109992ee92..0000000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/TempsProvider.java
+++ /dev/null
@@ -1,42 +0,0 @@
-// Copyright 2014 Google Inc. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.analysis;
-
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-
-import java.util.Collection;
-
-/**
- * A {@link TransitiveInfoProvider} for rule classes that save extra files when
- * {@code --save_temps} is in effect.
- */
-@Immutable
-public final class TempsProvider implements TransitiveInfoProvider {
-
-  private final ImmutableList<Artifact> temps;
-
-  public TempsProvider(ImmutableList<Artifact> temps) {
-    this.temps = temps;
-  }
-
-  /**
-   * Return the extra artifacts to save when {@code --save_temps} is in effect.
-   */
-  public Collection<Artifact> getTemps() {
-    return temps;
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
index c6d0dfad24..7810240885 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
@@ -108,11 +108,6 @@ public final class TopLevelArtifactHelper {
       TopLevelArtifactContext context) {
     NestedSetBuilder<Artifact> importantBuilder = NestedSetBuilder.stableOrder();
     NestedSetBuilder<Artifact> allBuilder = NestedSetBuilder.stableOrder();
-    TempsProvider tempsProvider = target.getProvider(TempsProvider.class);
-    if (tempsProvider != null) {
-      importantBuilder.addAll(tempsProvider.getTemps());
-    }
-
     TopLevelArtifactProvider topLevelArtifactProvider =
         target.getProvider(TopLevelArtifactProvider.class);
     if (topLevelArtifactProvider != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java
index 3b9631f990..aceec1efb4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java
@@ -68,6 +68,17 @@ public final class TopLevelArtifactProvider implements TransitiveInfoProvider {
    */
   public static final String HIDDEN_TOP_LEVEL = HIDDEN_OUTPUT_GROUP_PREFIX + "hidden_top_level";
 
+  /**
+   * Temporary files created during building a rule, for example, .i, .d and .s files for C++
+   * compilation.
+   *
+   * <p>This output group is somewhat special: it is always built, but it only contains files when
+   * the {@code --save_temps} command line option present. I'm not sure if this is to save RAM by
+   * not creating the associated actions and artifacts if we don't need them or just historical
+   * baggage.
+   */
+  public static final String TEMP_FILES = "temp_files";
+
   private final ImmutableMap<String, NestedSet<Artifact>> outputGroups;
 
   TopLevelArtifactProvider(ImmutableMap<String, NestedSet<Artifact>> outputGroups) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index ac8c693736..29b5600cba 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -521,6 +521,7 @@ public class BuildRequest implements OptionsClassProvider {
   private ImmutableSet<String> determineOutputGroups() {
     Set<String> current = new TreeSet<>();
     current.addAll(getBuildOptions().outputGroups);
+    current.add(TopLevelArtifactProvider.TEMP_FILES);
     if (getOptions(Options.class).collectCodeCoverage
         && !getBuildOptions().compileOnly
         && !getBuildOptions().compilationPrerequisitesOnly
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 7882751115..0c8d404935 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -51,9 +51,9 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.InputFileConfiguredTarget;
 import com.google.devtools.build.lib.analysis.OutputFileConfiguredTarget;
-import com.google.devtools.build.lib.analysis.TempsProvider;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
+import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
 import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
@@ -700,9 +700,10 @@ public class ExecutionTool {
 
       // For failed compilation, it is still useful to examine temp artifacts,
       // (ie, preprocessed and assembler files).
-      TempsProvider tempsProvider = target.getProvider(TempsProvider.class);
-      if (tempsProvider != null) {
-        for (Artifact temp : tempsProvider.getTemps()) {
+      TopLevelArtifactProvider topLevelProvider =
+          target.getProvider(TopLevelArtifactProvider.class);
+      if (topLevelProvider != null) {
+        for (Artifact temp : topLevelProvider.getOutputGroup(TopLevelArtifactProvider.TEMP_FILES)) {
           if (temp.getPath().exists()) {
             outErr.printErrLn("  See temp at " +
                 OutputDirectoryLinksUtils.getPrettyPath(temp.getPath(),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 7cbe43e1d7..0c172ef285 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -26,7 +26,6 @@ import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.TempsProvider;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -129,9 +128,9 @@ public final class CcCommon {
     linkopts = initLinkopts();
   }
 
-  ImmutableList<Artifact> getTemps(CcCompilationOutputs compilationOutputs) {
+  NestedSet<Artifact> getTemps(CcCompilationOutputs compilationOutputs) {
     return cppConfiguration.isLipoContextCollector()
-        ? ImmutableList.<Artifact>of()
+        ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER)
         : compilationOutputs.getTemps();
   }
 
@@ -710,9 +709,9 @@ public final class CcCommon {
             collectTransitiveCcNativeLibraries(ruleContext, linkingOutputs.getDynamicLibraries())))
         .add(InstrumentedFilesProvider.class, getInstrumentedFilesProvider(
             instrumentedObjectFiles))
-        .add(TempsProvider.class, new TempsProvider(getTemps(ccCompilationOutputs)))
         .add(CppDebugFileProvider.class, new CppDebugFileProvider(
             dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts()))
+        .addOutputGroup(TopLevelArtifactProvider.TEMP_FILES, getTemps(ccCompilationOutputs))
         .addOutputGroup(TopLevelArtifactProvider.FILES_TO_COMPILE,
             NestedSetBuilder.wrap(Order.STABLE_ORDER, getFilesToCompile(ccCompilationOutputs)))
         .addOutputGroup(TopLevelArtifactProvider.COMPILATION_PREREQUISITES,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java
index b9fa4e8f49..fcbd836416 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java
@@ -17,6 +17,8 @@ package com.google.devtools.build.lib.rules.cpp;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 
 import java.util.ArrayList;
 import java.util.LinkedHashSet;
@@ -50,7 +52,7 @@ public class CcCompilationOutputs {
   /**
    * All artifacts that are created if "--save_temps" is true.
    */
-  private final ImmutableList<Artifact> temps;
+  private final NestedSet<Artifact> temps;
 
   /**
    * All token .h.processed files created when preprocessing or parsing headers.
@@ -61,7 +63,7 @@ public class CcCompilationOutputs {
 
   private CcCompilationOutputs(ImmutableList<Artifact> objectFiles,
       ImmutableList<Artifact> picObjectFiles, ImmutableList<Artifact> dwoFiles,
-      ImmutableList<Artifact> picDwoFiles, ImmutableList<Artifact> temps,
+      ImmutableList<Artifact> picDwoFiles, NestedSet<Artifact> temps,
       ImmutableList<Artifact> headerTokenFiles,
       ImmutableList<IncludeScannable> lipoScannables) {
     this.objectFiles = objectFiles;
@@ -99,7 +101,7 @@ public class CcCompilationOutputs {
   /**
    * Returns an unmodifiable view of the temp files set.
    */
-  public ImmutableList<Artifact> getTemps() {
+  public NestedSet<Artifact> getTemps() {
     return temps;
   }
 
@@ -123,14 +125,14 @@ public class CcCompilationOutputs {
     private final Set<Artifact> picObjectFiles = new LinkedHashSet<>();
     private final Set<Artifact> dwoFiles = new LinkedHashSet<>();
     private final Set<Artifact> picDwoFiles = new LinkedHashSet<>();
-    private final Set<Artifact> temps = new LinkedHashSet<>();
+    private final NestedSetBuilder<Artifact> temps = NestedSetBuilder.stableOrder();
     private final Set<Artifact> headerTokenFiles = new LinkedHashSet<>();
     private final List<IncludeScannable> lipoScannables = new ArrayList<>();
 
     public CcCompilationOutputs build() {
       return new CcCompilationOutputs(ImmutableList.copyOf(objectFiles),
           ImmutableList.copyOf(picObjectFiles), ImmutableList.copyOf(dwoFiles),
-          ImmutableList.copyOf(picDwoFiles), ImmutableList.copyOf(temps),
+          ImmutableList.copyOf(picDwoFiles), temps.build(),
           ImmutableList.copyOf(headerTokenFiles),
           ImmutableList.copyOf(lipoScannables));
     }
@@ -140,7 +142,7 @@ public class CcCompilationOutputs {
       this.picObjectFiles.addAll(outputs.picObjectFiles);
       this.dwoFiles.addAll(outputs.dwoFiles);
       this.picDwoFiles.addAll(outputs.picDwoFiles);
-      this.temps.addAll(outputs.temps);
+      this.temps.addTransitive(outputs.temps);
       this.headerTokenFiles.addAll(outputs.headerTokenFiles);
       this.lipoScannables.addAll(outputs.lipoScannables);
       return this;
@@ -186,7 +188,7 @@ public class CcCompilationOutputs {
      * Adds temp files.
      */
     public Builder addTemps(Iterable<Artifact> artifacts) {
-      Iterables.addAll(temps, artifacts);
+      temps.addAll(artifacts);
       return this;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 4d7179816c..717033509d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.rules.cpp;
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -28,7 +27,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.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
-import com.google.devtools.build.lib.analysis.TempsProvider;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
@@ -649,8 +647,8 @@ public final class CcLibraryHelper {
     providers.put(CppDebugFileProvider.class, new CppDebugFileProvider(
         dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts()));
     providers.put(TransitiveLipoInfoProvider.class, collectTransitiveLipoInfo(ccOutputs));
-    providers.put(TempsProvider.class, getTemps(ccOutputs));
     Map<String, NestedSet<Artifact>> outputGroups = new TreeMap<>();
+    outputGroups.put(TopLevelArtifactProvider.TEMP_FILES, getTemps(ccOutputs));
     if (emitCompileProviders) {
       outputGroups.put(TopLevelArtifactProvider.FILES_TO_COMPILE, getFilesToCompile(ccOutputs));
       outputGroups.put(TopLevelArtifactProvider.COMPILATION_PREREQUISITES,
@@ -870,10 +868,10 @@ public final class CcLibraryHelper {
         : new CcExecutionDynamicLibrariesProvider(builder.build());
   }
 
-  private TempsProvider getTemps(CcCompilationOutputs compilationOutputs) {
+  private NestedSet<Artifact> getTemps(CcCompilationOutputs compilationOutputs) {
     return ruleContext.getFragment(CppConfiguration.class).isLipoContextCollector()
-        ? new TempsProvider(ImmutableList.<Artifact>of())
-        : new TempsProvider(compilationOutputs.getTemps());
+        ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER)
+        : compilationOutputs.getTemps();
   }
 
   private NestedSet<Artifact> getFilesToCompile(CcCompilationOutputs compilationOutputs) {