Skip to content

Commit 00f11de

Browse files
Scott Caometa-codesync[bot]
authored andcommitted
Fix uquery owner behavior around package boudary violations outside of allowlist
Summary: This diff fixes the `uquery` owner behavior for package boundary violations outside of the allowlist in Buck2. Previously, uquery owner would only look up all parent packages if source file lives in package boundary exception allowlist buckconfig. However, with buck2, we no longer enforce this allowlist, so in order to return all owners of a source file, we should always check all parent packages for owning targets. Posted here: https://fb.workplace.com/groups/buck2dev/permalink/4249680261986680/ Differential Revision: D92361150 fbshipit-source-id: 1e46885a1044acdc2a919ffef18b38c736592303
1 parent 81b1e03 commit 00f11de

File tree

2 files changed

+7
-30
lines changed

2 files changed

+7
-30
lines changed

app/buck2_query_impls/src/dice.rs

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use buck2_build_api::configure_targets::load_compatible_patterns_with_modifiers;
1616
use buck2_common::dice::cells::HasCellResolver;
1717
use buck2_common::dice::data::HasIoProvider;
1818
use buck2_common::file_ops::dice::DiceFileComputations;
19-
use buck2_common::package_boundary::HasPackageBoundaryExceptions;
2019
use buck2_common::package_listing::dice::DicePackageListingResolver;
2120
use buck2_common::package_listing::resolver::PackageListingResolver;
2221
use buck2_common::pattern::resolve::ResolveTargetPatterns;
@@ -28,6 +27,7 @@ use buck2_core::cells::CellResolver;
2827
use buck2_core::cells::cell_path::CellPath;
2928
use buck2_core::cells::cell_path_with_allowed_relative_dir::CellPathWithAllowedRelativeDir;
3029
use buck2_core::cells::name::CellName;
30+
use buck2_core::cells::paths::CellRelativePath;
3131
use buck2_core::configuration::compatibility::MaybeCompatible;
3232
use buck2_core::fs::project::ProjectRoot;
3333
use buck2_core::fs::project_rel_path::ProjectRelativePath;
@@ -277,30 +277,17 @@ impl UqueryDelegate for DiceQueryDelegate<'_, '_> {
277277
Ok(ResolveTargetPatterns::resolve(&mut self.ctx.get(), &parsed_patterns).await?)
278278
}
279279

280-
// This returns 1 package normally but can return multiple packages if the path is covered under `self.package_boundary_exceptions`.
280+
// Returns all packages from immediate enclosing up to cell root that could potentially own the path.
281281
async fn get_enclosing_packages(
282282
&self,
283283
path: &CellPath,
284284
) -> buck2_error::Result<Vec<PackageLabel>> {
285-
// Without package boundary violations, there is only 1 owning package for a path.
286-
// However, with package boundary violations, all parent packages of the enclosing package can also be owners.
287-
if let Some(enclosing_violation_path) = self
288-
.ctx
289-
.get()
290-
.get_package_boundary_exception(path.as_ref())
285+
let cell_root = CellPath::new(path.cell(), CellRelativePath::empty().to_buf());
286+
Ok(DicePackageListingResolver(&mut self.ctx.get())
287+
.get_enclosing_packages(path.as_ref(), cell_root.as_ref())
291288
.await?
292-
{
293-
return Ok(DicePackageListingResolver(&mut self.ctx.get())
294-
.get_enclosing_packages(path.as_ref(), (*enclosing_violation_path).as_ref())
295-
.await?
296-
.into_iter()
297-
.collect());
298-
}
299-
300-
let package = DicePackageListingResolver(&mut self.ctx.get())
301-
.get_enclosing_package(path.as_ref())
302-
.await?;
303-
Ok(vec![package])
289+
.into_iter()
290+
.collect())
304291
}
305292

306293
async fn eval_file_literal(&self, literal: &str) -> buck2_error::Result<FileSet> {

tests/core/query/uquery/test_uquery.py

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,6 @@ async def test_uquery_owner(buck: Buck) -> None:
8484

8585
@buck_test(data_dir="bxl_simple")
8686
async def test_query_owner_with_explicit_package_boundary_violation(buck: Buck) -> None:
87-
# This needs to be changed to `expect_failure` once Buck2 is checking path validity
88-
# outside of `package_boundary_exceptions`
89-
result = await buck.uquery(
90-
"""owner(package_boundary_violation/bin)""",
91-
"-c",
92-
"project.package_boundary_exceptions=",
93-
)
94-
assert "root//package_boundary_violation:bin" in result.stdout
95-
assert "root//:package_boundary_violation" not in result.stdout
96-
9787
result = await buck.uquery("""owner(package_boundary_violation/bin)""")
9888
assert "root//package_boundary_violation:bin" in result.stdout
9989
assert "root//:package_boundary_violation" in result.stdout

0 commit comments

Comments
 (0)