Skip to content

Commit

Permalink
buck2/dice: ignore overrides for buck2_re_client.override_use_case
Browse files Browse the repository at this point in the history
Summary: Ignore changes to `buck2_re_client.override_use_case` for legacy config state in DICE.

Reviewed By: JakobDegen

Differential Revision: D64477014

fbshipit-source-id: ad909caee5211992150e0b23e0304b85bcf2bfc2
  • Loading branch information
KapJI authored and facebook-github-bot committed Oct 31, 2024
1 parent 3446b86 commit 074c833
Show file tree
Hide file tree
Showing 10 changed files with 212 additions and 2 deletions.
25 changes: 25 additions & 0 deletions app/buck2_common/src/legacy_configs/cells.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use crate::legacy_configs::file_ops::ConfigParserFileOps;
use crate::legacy_configs::file_ops::ConfigPath;
use crate::legacy_configs::file_ops::DefaultConfigParserFileOps;
use crate::legacy_configs::file_ops::DiceConfigFileOps;
use crate::legacy_configs::key::BuckconfigKeyRef;
use crate::legacy_configs::parser::LegacyConfigParser;
use crate::legacy_configs::path::ExternalConfigSource;
use crate::legacy_configs::path::ProjectConfigSource;
Expand All @@ -66,6 +67,30 @@ impl ExternalBuckconfigData {
args: Vec::new(),
}
}

pub fn filter_values<F>(&self, filter: F) -> Self
where
F: Fn(&BuckconfigKeyRef) -> bool,
{
Self {
parse_state: self.parse_state.clone(),
args: self
.args
.iter()
.filter(|arg| match arg {
ResolvedLegacyConfigArg::Flag(flag) => {
flag.cell.is_some()
|| filter(&BuckconfigKeyRef {
section: &flag.section,
property: &flag.key,
})
}
_ => true,
})
.cloned()
.collect(),
}
}
}

/// Used for creating a CellResolver in a buckv1-compatible way based on values
Expand Down
26 changes: 26 additions & 0 deletions app/buck2_common/src/legacy_configs/configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::legacy_configs::args::ResolvedConfigFile;
use crate::legacy_configs::args::ResolvedLegacyConfigArg;
use crate::legacy_configs::file_ops::ConfigParserFileOps;
use crate::legacy_configs::file_ops::ConfigPath;
use crate::legacy_configs::key::BuckconfigKeyRef;
use crate::legacy_configs::parser::LegacyConfigParser;

#[derive(Clone, Dupe, Debug, Allocative)]
Expand Down Expand Up @@ -244,6 +245,31 @@ impl LegacyBuckConfig {
}))
}

pub fn filter_values<F>(&self, filter: F) -> Self
where
F: Fn(&BuckconfigKeyRef) -> bool,
{
let values = self
.0
.values
.iter()
.filter_map(|(section, section_data)| {
let values: SortedMap<_, _> = section_data
.values
.iter()
.filter(|(property, _)| filter(&BuckconfigKeyRef { section, property }))
.map(|(property, value)| (property.clone(), value.clone()))
.collect();
if values.is_empty() {
None
} else {
Some((section.clone(), LegacyBuckConfigSection { values }))
}
})
.collect();
Self(Arc::new(ConfigData { values }))
}

pub(crate) async fn start_parse_for_external_files(
config_paths: &[ConfigPath],
file_ops: &mut dyn ConfigParserFileOps,
Expand Down
12 changes: 11 additions & 1 deletion app/buck2_common/src/legacy_configs/dice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ impl Key for LegacyBuckConfigForCellKey {
let this_cell = cells.get(self.cell_name)?;
let config =
BuckConfigBasedCells::parse_single_cell_with_dice(ctx, this_cell.path()).await?;
let config = config.filter_values(should_ignore_config_change);

ConfigDiffTracker::report_computed_config(ctx, self.cell_name, &config);

Expand Down Expand Up @@ -346,14 +347,23 @@ impl SetLegacyConfigs for DiceTransactionUpdater {
&mut self,
data: Arc<ExternalBuckconfigData>,
) -> anyhow::Result<()> {
Ok(self.changed_to(vec![(LegacyExternalBuckConfigDataKey, Some(data))])?)
// Don't invalidate state if RE use case is overridden.
let data = data.filter_values(should_ignore_config_change);
Ok(self.changed_to(vec![(
LegacyExternalBuckConfigDataKey,
Some(Arc::new(data)),
)])?)
}

fn set_none_legacy_config_external_data(&mut self) -> anyhow::Result<()> {
Ok(self.changed_to(vec![(LegacyExternalBuckConfigDataKey, None)])?)
}
}

fn should_ignore_config_change(config_key: &BuckconfigKeyRef) -> bool {
!(config_key.section == "buck2_re_client" && config_key.property == "override_use_case")
}

#[cfg(test)]
mod tests {
use buck2_cli_proto::ConfigOverride;
Expand Down
3 changes: 2 additions & 1 deletion app/buck2_server/src/ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use buck2_cli_proto::common_build_options::ExecutionStrategy;
use buck2_cli_proto::config_override::ConfigType;
use buck2_cli_proto::ClientContext;
use buck2_cli_proto::CommonBuildOptions;
use buck2_cli_proto::ConfigOverride;
use buck2_common::dice::cells::HasCellResolver;
use buck2_common::dice::cycles::CycleDetectorAdapter;
use buck2_common::dice::cycles::PairDiceCycleDetector;
Expand Down Expand Up @@ -170,7 +171,7 @@ pub struct ServerCommandContext<'a> {
host_xcode_version_override: Option<String>,

reuse_current_config: bool,
config_overrides: Vec<buck2_cli_proto::ConfigOverride>,
config_overrides: Vec<ConfigOverride>,

// This ensures that there's only one RE connection during the lifetime of this context. It's possible
// that we give out other handles, but we don't depend on the lifetimes of those for this guarantee. We
Expand Down
7 changes: 7 additions & 0 deletions tests/core/cells/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,10 @@ buck2_e2e_test(
srcs = ["test_empty_buckconfig.py"],
data_dir = "test_empty_buckconfig_data",
)

buck2_e2e_test(
name = "test_ignore_state_invalidation",
srcs = ["test_ignore_state_invalidation.py"],
data_dir = "test_ignore_state_invalidation_data",
deps = ["//buck2/tests/e2e_util:utils"],
)
103 changes: 103 additions & 0 deletions tests/core/cells/test_ignore_state_invalidation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# 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.

# pyre-strict


import tempfile

from buck2.tests.e2e_util.api.buck import Buck
from buck2.tests.e2e_util.buck_workspace import buck_test
from buck2.tests.e2e_util.helper.utils import filter_events


async def check_dice_equality(buck: Buck) -> None:
dice_equal = await filter_events(
buck,
"Event",
"data",
"Instant",
"data",
"DiceEqualityCheck",
"is_equal",
)
assert len(dice_equal) == 1
assert dice_equal[0] is True


async def check_config_is_the_same(buck: Buck) -> None:
diff_count = await filter_events(
buck,
"Event",
"data",
"Instant",
"data",
"CellConfigDiff",
"config_diff_count",
)
assert len(diff_count) == 1
assert diff_count[0] == 0


@buck_test()
async def test_ignore_state_invalidation_with_re_override_in_arg(buck: Buck) -> None:
# Add arg to switch to buck2-user
await buck.build(
"root//:simple",
"--config",
"buck2_re_client.override_use_case=buck2-user",
)
# No arg, default is buck2-default
await buck.build("root//:simple")
await check_dice_equality(buck)
# Add arg to switch to buck2-user again
await buck.build(
"root//:simple",
"--config",
"buck2_re_client.override_use_case=buck2-user",
)
await check_dice_equality(buck)


@buck_test()
async def test_ignore_state_invalidation_with_re_override_in_config(buck: Buck) -> None:
# Default is buck2-default
await buck.build("root//:simple")
# Add config to switch to buck2-user
with open(buck.cwd / ".buckconfig.local", "w") as f:
f.write("[buck2_re_client]\n")
f.write("override_use_case = buck2-user\n")
await buck.build("root//:simple")
await check_config_is_the_same(buck)
# Add config to return to buck2-default
with open(buck.cwd / ".buckconfig.local", "w") as f:
f.write("[buck2_re_client]\n")
f.write("override_use_case = buck2-default\n")
await buck.build("root//:simple")
await check_config_is_the_same(buck)


@buck_test()
async def test_ignore_state_invalidation_with_re_override_in_external_config(
buck: Buck,
) -> None:
# Default is buck2-default
await buck.build("root//:simple")
# Add config to switch to buck2-user
with tempfile.NamedTemporaryFile("w", delete=False) as f:
f.write("[buck2_re_client]\n")
f.write("override_use_case = buck2-user\n")
f.close()
await buck.build("root//:simple", "--config-file", f.name)
await check_config_is_the_same(buck)
# Add config to return to buck2-default
with tempfile.NamedTemporaryFile("w", delete=False) as f:
f.write("[buck2_re_client]\n")
f.write("override_use_case = buck2-default\n")
f.close()
await buck.build("root//:simple", "--config-file", f.name)
await check_config_is_the_same(buck)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[buildfile]
name=TARGETS.fixture

[cells]
root = .
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
load(":defs.bzl", "simple")

simple(name = "simple")
30 changes: 30 additions & 0 deletions tests/core/cells/test_ignore_state_invalidation_data/defs.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# 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.

def _simple(ctx):
re_use_case = read_config("buck2_re_client", "override_use_case")
if re_use_case != None:
fail("RE use case is set to: {}".format(re_use_case))
output = ctx.actions.declare_output("output")
run = ctx.actions.write(
"run.py",
[
"import os",
"import sys",
"build_id = os.environ[\"BUCK_BUILD_ID\"]",
"with open(sys.argv[1], 'w') as f:",
" f.write(f'{build_id}\\n')",
],
)
ctx.actions.run(
cmd_args(["python3", run, output.as_output()]),
category = "test_category",
)

return [DefaultInfo(default_output = output)]

simple = rule(impl = _simple, attrs = {})

0 comments on commit 074c833

Please sign in to comment.