From 593587fbb154bcf1b78c896b1feef9a6f2b4a29f Mon Sep 17 00:00:00 2001 From: Andrew Klotz Date: Tue, 21 Oct 2025 00:11:36 +0000 Subject: [PATCH] fix suggestion to include full path This change returns a valid suggestion in the case of an invalid path to a BUCK file. This can come up from either making a typo in a target path or accidentially mismatching cases on a case-insensitive OS and then having a build run on a case-sensitive OS Before, we matched on the closest parent directory then returned that as a suggested path, which can be confusing if `root//cat` is not a valid location: ``` dir `root//animals/pets/cats` does not exist. Did you mean `root//cat`? ``` After, we now return the full path to the matched directory allowing copy+paste of the suggestion to fix: ``` dir `root//animals/pets/cats` does not exist. Did you mean `root//animals/pets/cat`? ``` PS: I have no idea if the tests pass, the README says they can only run inside meta so this is just a guess --- app/buck2_common/src/file_ops/error.rs | 5 ++++- tests/core/errors/test_errors.py | 2 ++ .../package_listing/expected.golden.out | 12 ++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/buck2_common/src/file_ops/error.rs b/app/buck2_common/src/file_ops/error.rs index d0d8f6aa12cc4..e2ad6a342892c 100644 --- a/app/buck2_common/src/file_ops/error.rs +++ b/app/buck2_common/src/file_ops/error.rs @@ -132,7 +132,10 @@ pub(super) fn extended_ignore_error<'a>( return Some(ReadDirError::DirectoryDoesNotExist { path: path.to_owned(), suggestion: DirectoryDoesNotExistSuggestion::Typo( - suggestion.to_owned(), + match parent.path().join_normalized(suggestion) { + Ok(p) => p.as_str().to_owned(), + Err(_) => suggestion.to_owned() + } ), }); } diff --git a/tests/core/errors/test_errors.py b/tests/core/errors/test_errors.py index f9d39f80dedbd..031217a88be69 100644 --- a/tests/core/errors/test_errors.py +++ b/tests/core/errors/test_errors.py @@ -61,6 +61,8 @@ async def test_package_listing_errors(buck: Buck) -> None: "//package_listing/data.file/subdir:target", # Missing directory due to typo "//package_listings:", + # Missing directory due to typo shows full path + "//package_listing/data.file:targets", # Missing directory due to being in the wrong cell "//something:", ]: diff --git a/tests/core/errors/test_errors_data/package_listing/expected.golden.out b/tests/core/errors/test_errors_data/package_listing/expected.golden.out index adb6845e7a071..a40246ca26c03 100644 --- a/tests/core/errors/test_errors_data/package_listing/expected.golden.out +++ b/tests/core/errors/test_errors_data/package_listing/expected.golden.out @@ -75,6 +75,18 @@ Caused by: ^-----------------------------^ path `root//package_listing/data.file` is a file, not a directory +Command failed: +Error evaluating expression: + //package_listing/data.file:targets: + ^-----------------^ + + +Caused by: + package `root//package_listing/data.file:targets:` does not exist + ^--------------------^ + dir `root//package_listing/data.file:targets` does not exist. Did you mean `root//package_listing/data.file:target`? + + Command failed: