Remove 'unstable_include_other_outputs' option from build report

Summary:
I couldn't find any usage of this outside of buck2, so thought it'd be safe to remove as it defaults to 'false'

codehub: https://www.internalfb.com/code/search?q=repo%3Aall%20unstable_include_other_outputs

scuba: https://fburl.com/scuba/buck2_builds/6dujawo6

Also checked skewer-case for both and didn't find anything either. Lmk if I should check anywhere else or if this should be kept for reasons I'm not aware of

Reviewed By: JakobDegen

Differential Revision: D68235831

fbshipit-source-id: c1206d30c964880c739e6ffa4f146ab3e45bac00

app/buck2_build_api/src/build.rs

@@ -59,7 +59,7 @@ pub mod build_report;
 mod graph_size;
 
 /// The types of provider to build on the configured providers label
-#[derive(Debug, Clone, Dupe, Allocative)]
+#[derive(Debug, Clone, Dupe, Allocative, PartialEq)]
 pub enum BuildProviderType {
     Default,
     DefaultOther,

app/buck2_build_api/src/build/build_report.rs

@@ -161,7 +161,6 @@ enum EntryLabel {
 
 pub struct BuildReportOpts {
     pub print_unconfigured_section: bool,
-    pub unstable_include_other_outputs: bool,
     pub unstable_include_failures_build_report: bool,
     pub unstable_include_package_project_relative_paths: bool,
     pub unstable_build_report_filename: String,
@@ -172,7 +171,6 @@ pub struct BuildReportCollector<'a> {
     cell_resolver: &'a CellResolver,
     overall_success: bool,
     include_unconfigured_section: bool,
-    include_other_outputs: bool,
     error_cause_cache: HashMap<buck2_error::UniqueRootId, usize>,
     next_cause_index: usize,
     strings: BTreeMap<String, String>,
@@ -188,7 +186,6 @@ impl<'a> BuildReportCollector<'a> {
         cell_resolver: &'a CellResolver,
         project_root: &ProjectRoot,
         include_unconfigured_section: bool,
-        include_other_outputs: bool,
         include_failures: bool,
         include_package_project_relative_paths: bool,
         configured: &BTreeMap<ConfiguredProvidersLabel, Option<ConfiguredBuildTargetResult>>,
@@ -199,7 +196,6 @@ impl<'a> BuildReportCollector<'a> {
             cell_resolver,
             overall_success: true,
             include_unconfigured_section,
-            include_other_outputs,
             error_cause_cache: HashMap::default(),
             next_cause_index: 0,
             strings: BTreeMap::default(),
@@ -357,52 +353,20 @@ impl<'a> BuildReportCollector<'a> {
         for (label, result) in results {
             let provider_name: Arc<str> = report_providers_name(label).into();
 
-            result.outputs.iter().for_each(|res| {
-                match res {
-                    Ok(artifacts) => {
-                        let mut is_default = false;
-                        let mut is_other = false;
-
-                        match artifacts.provider_type {
-                            BuildProviderType::Default => {
-                                // as long as we have requested it as a default info, it should  be
-                                // considered a default output whether or not it also appears as an other
-                                // non-main output
-                                is_default = true;
-                            }
-                            BuildProviderType::DefaultOther
-                            | BuildProviderType::Run
-                            | BuildProviderType::Test => {
-                                // as long as the output isn't the default, we add it to other outputs.
-                                // This means that the same artifact may appear twice if its part of the
-                                // default AND the other outputs, but this is intended as it accurately
-                                // describes the type of the artifact
-                                is_other = true;
-                            }
-                        }
-
+            result.outputs.iter().for_each(|res| match res {
+                Ok(artifacts) => {
+                    if artifacts.provider_type == BuildProviderType::Default {
                         for (artifact, _value) in artifacts.values.iter() {
-                            if is_default {
-                                configured_report
-                                    .inner
-                                    .outputs
-                                    .entry(provider_name.clone())
-                                    .or_default()
-                                    .insert(artifact.resolve_path(self.artifact_fs).unwrap());
-                            }
-
-                            if is_other && self.include_other_outputs {
-                                configured_report
-                                    .inner
-                                    .other_outputs
-                                    .entry(provider_name.clone())
-                                    .or_default()
-                                    .insert(artifact.resolve_path(self.artifact_fs).unwrap());
-                            }
+                            configured_report
+                                .inner
+                                .outputs
+                                .entry(provider_name.clone())
+                                .or_default()
+                                .insert(artifact.resolve_path(self.artifact_fs).unwrap());
                         }
                     }
-                    Err(e) => errors.push(e.dupe()),
                 }
+                Err(e) => errors.push(e.dupe()),
             });
 
             errors.extend(result.errors.iter().cloned());
@@ -565,16 +529,6 @@ pub async fn build_report_opts<'a>(
             )
             .await?
             .unwrap_or(true),
-        unstable_include_other_outputs: ctx
-            .parse_legacy_config_property(
-                cell_resolver.root_cell(),
-                BuckconfigKeyRef {
-                    section: "build_report",
-                    property: "unstable_include_other_outputs",
-                },
-            )
-            .await?
-            .unwrap_or(false),
         unstable_include_failures_build_report: build_opts.unstable_include_failures_build_report,
         unstable_include_package_project_relative_paths: build_opts
             .unstable_include_package_project_relative_paths,
@@ -600,7 +554,6 @@ pub fn generate_build_report(
         cell_resolver,
         project_root,
         opts.print_unconfigured_section,
-        opts.unstable_include_other_outputs,
         opts.unstable_include_failures_build_report,
         opts.unstable_include_package_project_relative_paths,
         configured,

app/buck2_bxl/src/command.rs

@@ -213,7 +213,6 @@ async fn bxl(
         let build_report_opts = BuildReportOpts {
             // These are all deprecated for `buck2 build`, so don't need to support them
             print_unconfigured_section: false,
-            unstable_include_other_outputs: false,
             unstable_include_failures_build_report: false,
             unstable_include_package_project_relative_paths: false,
             unstable_build_report_filename: bxl_opts.unstable_build_report_filename.clone(),

tests/core/build/test_uncategorized.py

@@ -75,8 +75,6 @@ def read_all_outputs(buck: Buck, report: str) -> typing.List[str]:
 async def test_build_providers(buck: Buck) -> None:
     await buck.build(
         "//:target",
-        "-c",
-        "build_report.unstable_include_other_outputs=true",
         "--build-default-info",
         "--skip-run-info",
         "--skip-test-info",
@@ -91,8 +89,6 @@ async def test_build_providers(buck: Buck) -> None:
 
     await buck.build(
         "//:target",
-        "-c",
-        "build_report.unstable_include_other_outputs=true",
         "--skip-default-info",
         "--build-run-info",
         "--skip-test-info",
@@ -102,13 +98,10 @@ async def test_build_providers(buck: Buck) -> None:
 
     outputs = read_all_outputs(buck, "report")
     assert all("/build" not in o for o in outputs)
-    assert any("/run" in o for o in outputs)
     assert all("/test" not in o for o in outputs)
 
     await buck.build(
         "//:target",
-        "-c",
-        "build_report.unstable_include_other_outputs=true",
         "--skip-default-info",
         "--skip-run-info",
         "--build-test-info",
@@ -119,7 +112,6 @@ async def test_build_providers(buck: Buck) -> None:
     outputs = read_all_outputs(buck, "report")
     assert all("/build" not in o for o in outputs)
     assert all("/run" not in o for o in outputs)
-    assert any("/test" in o for o in outputs)
 
 
 @buck_test(data_dir="projected_artifacts")

tests/e2e/build/test_build_inplace.py

@@ -439,15 +439,13 @@ async def test_build_test_dependencies(buck: Buck) -> None:
     target = "fbcode//buck2/tests/targets/rules/sh_test:test_with_env"
     build = await buck.build(
         target,
-        "-c",
-        "build_report.unstable_include_other_outputs=true",
         "--build-test-info",
         "--build-report",
         "-",
     )
     report = build.get_build_report().build_report
 
-    path = ["results", target, "other_outputs", "DEFAULT"]
+    path = ["results", target, "other_outputs"]
     for p in path:
         report = report[p]
 
@@ -456,7 +454,7 @@ async def test_build_test_dependencies(buck: Buck) -> None:
         if "__file__" in artifact:
             has_file = True
 
-    assert has_file
+    assert not has_file
 
 
 # TODO(marwhal): Fix and enable on Windows