Skip to content

Commit

Permalink
Remove dependency on buck2_configured from buck2_bxl
Browse files Browse the repository at this point in the history
Summary:
`buck2_bxl` crate is heavy, and having fewer dependencies speeds up incremental compilation.

If this [RFC in Rust](rust-lang/rfcs#3635) will ever be implemented, we will reduce the amount of complexity in code like this.

Initially I wanted to do it to push attribute configuration to downstream crates from `buck2_node` for D63918028, but then decided not to do it for a while. So this diff is not required for D63918028.

Reviewed By: JakobDegen

Differential Revision: D63929039

fbshipit-source-id: 5360c5bb774a4d79471e6dad0ce2c1ba54a6b57b
  • Loading branch information
stepancheg authored and facebook-github-bot committed Oct 9, 2024
1 parent f1be6a6 commit 367ebfb
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 9 deletions.
1 change: 0 additions & 1 deletion app/buck2_bxl/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ rust_library(
"//buck2/app/buck2_build_api:buck2_build_api",
"//buck2/app/buck2_cli_proto:buck2_cli_proto",
"//buck2/app/buck2_common:buck2_common",
"//buck2/app/buck2_configured:buck2_configured",
"//buck2/app/buck2_core:buck2_core",
"//buck2/app/buck2_data:buck2_data",
"//buck2/app/buck2_error:buck2_error",
Expand Down
1 change: 0 additions & 1 deletion app/buck2_bxl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ buck2_artifact = { workspace = true }
buck2_build_api = { workspace = true }
buck2_cli_proto = { workspace = true }
buck2_common = { workspace = true }
buck2_configured = { workspace = true }
buck2_core = { workspace = true }
buck2_data = { workspace = true }
buck2_error = { workspace = true }
Expand Down
14 changes: 8 additions & 6 deletions app/buck2_bxl/src/bxl/starlark_defs/context/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use buck2_build_api::analysis::calculation::RuleAnalysisCalculation;
use buck2_build_api::analysis::registry::AnalysisRegistry;
use buck2_build_api::interpreter::rule_defs::context::AnalysisActions;
use buck2_build_api::interpreter::rule_defs::provider::dependency::Dependency;
use buck2_configured::configuration::calculation::ConfigurationCalculation;
use buck2_core::cells::name::CellName;
use buck2_core::configuration::data::ConfigurationData;
use buck2_core::configuration::pair::ConfigurationNoExec;
Expand All @@ -27,6 +26,7 @@ use buck2_core::target::target_configured_target_label::TargetConfiguredTargetLa
use buck2_interpreter::types::configured_providers_label::StarlarkProvidersLabel;
use buck2_node::attrs::configuration_context::AttrConfigurationContext;
use buck2_node::attrs::configuration_context::AttrConfigurationContextImpl;
use buck2_node::configuration::calculation::CONFIGURATION_CALCULATION;
use buck2_node::configuration::resolved::ConfigurationSettingKey;
use buck2_node::execution::GET_EXECUTION_PLATFORMS;
use derivative::Derivative;
Expand Down Expand Up @@ -78,15 +78,17 @@ pub(crate) async fn resolve_bxl_execution_platform(

let platform_configuration = match target_platform.as_ref() {
Some(global_target_platform) => {
ctx.get_platform_configuration(global_target_platform)
CONFIGURATION_CALCULATION
.get()?
.get_platform_configuration(ctx, global_target_platform)
.await?
}
None => ConfigurationData::unspecified(),
};
let resolved_configuration = {
ctx.get_resolved_configuration(&platform_configuration, cell, &*exec_compatible_with)
.await?
};
let resolved_configuration = CONFIGURATION_CALCULATION
.get()?
.get_resolved_configuration(ctx, &platform_configuration, cell, &exec_compatible_with)
.await?;

// there is not explicit configured deps, so platforms is empty
let platform_cfgs = OrderedMap::new();
Expand Down
31 changes: 30 additions & 1 deletion app/buck2_configured/src/configuration/calculation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ use buck2_build_api::interpreter::rule_defs::provider::builtin::configuration_in
use buck2_build_api::interpreter::rule_defs::provider::builtin::execution_platform_registration_info::FrozenExecutionPlatformRegistrationInfo;
use buck2_common::legacy_configs::key::BuckconfigKeyRef;
use buck2_core::target::target_configured_target_label::TargetConfiguredTargetLabel;
use buck2_node::configuration::calculation::{ConfigurationCalculationDyn, CONFIGURATION_CALCULATION};
use buck2_node::execution::{GetExecutionPlatformsImpl, GET_EXECUTION_PLATFORMS, GetExecutionPlatforms, EXECUTION_PLATFORMS_BUCKCONFIG};
use crate::nodes::calculation::ExecutionPlatformConstraints;

Expand Down Expand Up @@ -363,7 +364,7 @@ struct ResolvedConfigurationKey {
}

#[async_trait]
pub trait ConfigurationCalculation {
pub(crate) trait ConfigurationCalculation {
async fn get_default_platform(
&mut self,
target: &TargetLabel,
Expand Down Expand Up @@ -408,6 +409,34 @@ pub trait ConfigurationCalculation {
) -> buck2_error::Result<ExecutionPlatformResolution>;
}

struct ConfigurationCalculationDynImpl;

#[async_trait]
impl ConfigurationCalculationDyn for ConfigurationCalculationDynImpl {
async fn get_platform_configuration(
&self,
ctx: &mut DiceComputations<'_>,
target: &TargetLabel,
) -> anyhow::Result<ConfigurationData> {
ctx.get_platform_configuration(target).await
}

async fn get_resolved_configuration(
&self,
ctx: &mut DiceComputations<'_>,
target_cfg: &ConfigurationData,
target_node_cell: CellName,
configuration_deps: &[ConfigurationSettingKey],
) -> buck2_error::Result<ResolvedConfiguration> {
ctx.get_resolved_configuration(target_cfg, target_node_cell, configuration_deps)
.await
}
}

pub(crate) fn init_configuration_calculation() {
CONFIGURATION_CALCULATION.init(&ConfigurationCalculationDynImpl);
}

async fn compute_platform_configuration_no_label_check(
ctx: &mut DiceComputations<'_>,
target: &TargetLabel,
Expand Down
1 change: 1 addition & 0 deletions app/buck2_configured/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ pub mod nodes;
pub fn init_late_bindings() {
calculation::init_configured_target_calculation();
configuration::calculation::init_get_execution_platforms();
configuration::calculation::init_configuration_calculation();
nodes::calculation::init_configured_target_node_calculation();
}
1 change: 1 addition & 0 deletions app/buck2_node/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* of this source tree.
*/

pub mod calculation;
pub mod resolved;
pub mod target_platform_detector;
pub mod toolchain_constraints;
38 changes: 38 additions & 0 deletions app/buck2_node/src/configuration/calculation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under both the MIT license found in the
* LICENSE-MIT file in the root directory of this source tree and the Apache
* License, Version 2.0 found in the LICENSE-APACHE file in the root directory
* of this source tree.
*/

use async_trait::async_trait;
use buck2_core::cells::name::CellName;
use buck2_core::configuration::data::ConfigurationData;
use buck2_core::target::label::label::TargetLabel;
use buck2_util::late_binding::LateBinding;
use dice::DiceComputations;

use crate::configuration::resolved::ConfigurationSettingKey;
use crate::configuration::resolved::ResolvedConfiguration;

#[async_trait]
pub trait ConfigurationCalculationDyn: Send + Sync + 'static {
async fn get_platform_configuration(
&self,
dice: &mut DiceComputations<'_>,
target: &TargetLabel,
) -> anyhow::Result<ConfigurationData>;

async fn get_resolved_configuration(
&self,
dice: &mut DiceComputations<'_>,
target_cfg: &ConfigurationData,
target_node_cell: CellName,
configuration_deps: &[ConfigurationSettingKey],
) -> buck2_error::Result<ResolvedConfiguration>;
}

pub static CONFIGURATION_CALCULATION: LateBinding<&'static dyn ConfigurationCalculationDyn> =
LateBinding::new("CONFIGURATION_CALCULATION");

0 comments on commit 367ebfb

Please sign in to comment.