diff --git a/app/buck2_common/src/legacy_configs/cells.rs b/app/buck2_common/src/legacy_configs/cells.rs index 8c20f9f98663b..0b52266236616 100644 --- a/app/buck2_common/src/legacy_configs/cells.rs +++ b/app/buck2_common/src/legacy_configs/cells.rs @@ -350,8 +350,8 @@ impl BuckConfigBasedCells { for (alias, destination) in Self::get_cell_aliases_from_config(&config)? { let alias_path = - cells_aggregator.add_cell_alias(root_path.clone(), alias.clone(), destination)?; - root_aliases.insert(alias, alias_path.clone()); + cells_aggregator.add_cell_alias_for_root_cell(alias.clone(), destination)?; + root_aliases.insert(alias, alias_path); } if let Some(external_cells) = config.get_section("external_cells") { @@ -390,14 +390,6 @@ impl BuckConfigBasedCells { ) .await?; - for (alias, alias_path) in &root_aliases { - cells_aggregator.add_cell_entry(path.clone(), alias.clone(), alias_path.clone())?; - } - - for (alias, destination) in Self::get_cell_aliases_from_config(&config)? { - cells_aggregator.add_cell_alias(path.clone(), alias.clone(), destination)?; - } - buckconfigs.insert(path, config); } diff --git a/app/buck2_core/src/cells.rs b/app/buck2_core/src/cells.rs index c1bbc940d9eb3..c7ed799a25645 100644 --- a/app/buck2_core/src/cells.rs +++ b/app/buck2_core/src/cells.rs @@ -487,6 +487,7 @@ impl CellResolver { #[derive(Debug)] pub struct CellsAggregator { cell_infos: HashMap, + root_aliases: HashMap, } #[derive(Debug, Default)] @@ -495,31 +496,14 @@ struct CellAggregatorInfo { /// So that it is predictable, we always use the first name we encounter, /// so the root file can choose what the alias is called. name: Option, - /// All the aliases known by this cell. - alias_mapping: HashMap, external: Option, } -impl CellAggregatorInfo { - fn add_alias_mapping( - &mut self, - from: NonEmptyCellAlias, - to: CellRootPathBuf, - ) -> anyhow::Result<()> { - let old = self.alias_mapping.insert(from.clone(), to.clone()); - if let Some(old) = old { - if old != to { - return Err(CellError::DuplicateAliases(from, old, to).into()); - } - } - Ok(()) - } -} - impl CellsAggregator { pub fn new() -> Self { Self { cell_infos: HashMap::new(), + root_aliases: HashMap::new(), } } @@ -547,26 +531,41 @@ impl CellsAggregator { if name.is_none() { *name = Some(CellName::unchecked_new(parsed_alias.as_str())?); } - self.cell_info(cell_root) - .add_alias_mapping(parsed_alias, alias_path) + + if cell_root.is_repo_root() { + self.add_cell_alias_for_root_cell_inner(parsed_alias, alias_path)?; + } + Ok(()) } /// Adds a cell alias configuration entry - pub fn add_cell_alias( + pub fn add_cell_alias_for_root_cell( &mut self, - cell_root: CellRootPathBuf, parsed_alias: NonEmptyCellAlias, alias_destination: NonEmptyCellAlias, ) -> anyhow::Result { - let cell_info = self.cell_info(cell_root); - let alias_path = match cell_info.alias_mapping.get(&alias_destination) { + let alias_path = match self.root_aliases.get(&alias_destination) { None => return Err(CellError::AliasOnlyCell(parsed_alias, alias_destination).into()), Some(alias_path) => alias_path.clone(), }; - cell_info.add_alias_mapping(parsed_alias, alias_path.clone())?; + self.add_cell_alias_for_root_cell_inner(parsed_alias, alias_path.clone())?; Ok(alias_path) } + fn add_cell_alias_for_root_cell_inner( + &mut self, + from: NonEmptyCellAlias, + to: CellRootPathBuf, + ) -> anyhow::Result<()> { + let old: Option = self.root_aliases.insert(from.clone(), to.clone()); + if let Some(old) = old { + if old != to { + return Err(CellError::DuplicateAliases(from, old, to).into()); + } + } + Ok(()) + } + fn get_cell_name_from_path(&self, path: &CellRootPath) -> anyhow::Result { self.cell_infos .get(path) @@ -583,7 +582,7 @@ impl CellsAggregator { } /// Creates the 'CellResolver' from all the entries that were aggregated - pub fn make_cell_resolver(self) -> anyhow::Result { + pub fn make_cell_resolver(mut self) -> anyhow::Result { let mut cell_mappings = Vec::new(); let all_cell_roots_for_nested_cells: Vec<_> = self @@ -592,39 +591,35 @@ impl CellsAggregator { .map(|path| Ok((self.get_cell_name_from_path(path)?, path.as_path()))) .collect::>()?; - let mut root_cell_alias_resolver = None; + let mut root_cell_name = None; for (cell_path, cell_info) in &self.cell_infos { let nested_cells = NestedCells::from_cell_roots(&all_cell_roots_for_nested_cells, cell_path); - let mut aliases_for_cell = HashMap::new(); let cell_name = self.get_cell_name_from_path(cell_path)?; - for (alias, path_for_alias) in &cell_info.alias_mapping { - aliases_for_cell - .insert(alias.clone(), self.get_cell_name_from_path(path_for_alias)?); - } - - let alias_resolver = CellAliasResolver::new(cell_name, aliases_for_cell)?; - if cell_path.is_repo_root() { - root_cell_alias_resolver = Some(alias_resolver.clone()); + root_cell_name = Some(cell_name); } cell_mappings.push(CellInstance::new( cell_name, cell_path.clone(), cell_info.external.dupe(), - alias_resolver, nested_cells, )?); } - CellResolver::new( - cell_mappings, - root_cell_alias_resolver.ok_or(CellError::NoRootCell)?, - ) + let root_cell_name = root_cell_name.ok_or(CellError::NoRootCell)?; + let mut root_aliases = HashMap::new(); + for (alias, path) in std::mem::take(&mut self.root_aliases) { + root_aliases.insert(alias, self.get_cell_name_from_path(&path)?); + } + + let root_cell_alias_resolver = CellAliasResolver::new(root_cell_name, root_aliases)?; + + CellResolver::new(cell_mappings, root_cell_alias_resolver) } pub fn mark_external_cell( @@ -779,10 +774,8 @@ mod tests { fn test_alias_only_error() -> anyhow::Result<()> { let mut agg = CellsAggregator::new(); - let cell_root = CellRootPathBuf::new(ProjectRelativePathBuf::try_from("".to_owned())?); assert!( - agg.add_cell_alias( - cell_root, + agg.add_cell_alias_for_root_cell( NonEmptyCellAlias::new("root".to_owned()).unwrap(), NonEmptyCellAlias::new("does_not_exist".to_owned()).unwrap(), ) diff --git a/app/buck2_core/src/cells/instance.rs b/app/buck2_core/src/cells/instance.rs index 56c9c8d77950e..78b4192a140ff 100644 --- a/app/buck2_core/src/cells/instance.rs +++ b/app/buck2_core/src/cells/instance.rs @@ -18,12 +18,9 @@ use crate::cells::cell_root_path::CellRootPathBuf; use crate::cells::external::ExternalCellOrigin; use crate::cells::name::CellName; use crate::cells::nested::NestedCells; -use crate::cells::CellAliasResolver; #[derive(Debug, buck2_error::Error)] enum CellInstanceError { - #[error("Inconsistent cell name: `{0}` in instance, but `{1}` in alias resolver")] - InconsistentCellName(CellName, CellName), #[error( "Attempted to refer to cell `{0}`; however, this is an external cell which cannot be used from `{1}`" )] @@ -45,9 +42,6 @@ struct CellData { /// the project relative path to this 'CellInstance' path: CellRootPathBuf, external: Option, - #[derivative(Debug = "ignore")] - /// the aliases of this specific cell - aliases: CellAliasResolver, nested_cells: NestedCells, } @@ -56,12 +50,8 @@ impl CellInstance { name: CellName, path: CellRootPathBuf, external: Option, - aliases: CellAliasResolver, nested_cells: NestedCells, ) -> anyhow::Result { - if name != aliases.current { - return Err(CellInstanceError::InconsistentCellName(name, aliases.current).into()); - } if external.is_some() && let Some(nested) = nested_cells.check_empty() { @@ -71,7 +61,6 @@ impl CellInstance { name, path, external, - aliases, nested_cells, }))) } @@ -88,11 +77,6 @@ impl CellInstance { &self.0.path } - #[inline] - pub fn testing_cell_alias_resolver(&self) -> &CellAliasResolver { - &self.0.aliases - } - #[inline] pub fn nested_cells(&self) -> &NestedCells { &self.0.nested_cells