diff --git a/app/buck2_build_api/src/build.rs b/app/buck2_build_api/src/build.rs index 25880f68f7c3..978db50fa4be 100644 --- a/app/buck2_build_api/src/build.rs +++ b/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, diff --git a/app/buck2_build_api/src/build/build_report.rs b/app/buck2_build_api/src/build/build_report.rs index e5930c37389e..88f37ef2582c 100644 --- a/app/buck2_build_api/src/build/build_report.rs +++ b/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, next_cause_index: usize, strings: BTreeMap, @@ -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>, @@ -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 = 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, diff --git a/app/buck2_bxl/src/command.rs b/app/buck2_bxl/src/command.rs index e38b4bbe93f9..3e656911db15 100644 --- a/app/buck2_bxl/src/command.rs +++ b/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(), diff --git a/tests/core/build/test_uncategorized.py b/tests/core/build/test_uncategorized.py index db1570e4281d..302f8fbc4658 100644 --- a/tests/core/build/test_uncategorized.py +++ b/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") diff --git a/tests/e2e/build/test_build_inplace.py b/tests/e2e/build/test_build_inplace.py index 72725f2be46b..8b2446de2546 100644 --- a/tests/e2e/build/test_build_inplace.py +++ b/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