From 6983fc7012b71b86662c74bceb81fdede42cfb9a Mon Sep 17 00:00:00 2001
From: Jakob Degen <jakobdegen@meta.com>
Date: Sun, 14 Jul 2024 19:17:14 -0700
Subject: [PATCH] configs: Fallible file existence check

Summary: This operation is not actually infallible

Reviewed By: stepancheg

Differential Revision: D59664959

fbshipit-source-id: 11916f71b50919910bd9ca5491372804d1c41b10
---
 app/buck2_common/src/legacy_configs/cells.rs  |  2 +-
 .../src/legacy_configs/configs.rs             |  6 ++---
 .../src/legacy_configs/file_ops.rs            | 22 +++++++++----------
 app/buck2_common/src/legacy_configs/parser.rs |  4 ++--
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/app/buck2_common/src/legacy_configs/cells.rs b/app/buck2_common/src/legacy_configs/cells.rs
index f7948aba51b1..c86d2436e1f9 100644
--- a/app/buck2_common/src/legacy_configs/cells.rs
+++ b/app/buck2_common/src/legacy_configs/cells.rs
@@ -192,7 +192,7 @@ impl BuckConfigBasedCells {
                 self.inner.read_file_lines(path).await
             }
 
-            async fn file_exists(&mut self, path: &ConfigPath) -> bool {
+            async fn file_exists(&mut self, path: &ConfigPath) -> anyhow::Result<bool> {
                 self.inner.file_exists(path).await
             }
 
diff --git a/app/buck2_common/src/legacy_configs/configs.rs b/app/buck2_common/src/legacy_configs/configs.rs
index b253c091ac38..7ca7c711dc74 100644
--- a/app/buck2_common/src/legacy_configs/configs.rs
+++ b/app/buck2_common/src/legacy_configs/configs.rs
@@ -385,11 +385,11 @@ pub mod testing {
     #[async_trait::async_trait]
     #[allow(private_interfaces)]
     impl ConfigParserFileOps for TestConfigParserFileOps {
-        async fn file_exists(&mut self, path: &ConfigPath) -> bool {
+        async fn file_exists(&mut self, path: &ConfigPath) -> anyhow::Result<bool> {
             let ConfigPath::Project(path) = path else {
-                return false;
+                return Ok(false);
             };
-            self.data.contains_key(path)
+            Ok(self.data.contains_key(path))
         }
 
         async fn read_file_lines(
diff --git a/app/buck2_common/src/legacy_configs/file_ops.rs b/app/buck2_common/src/legacy_configs/file_ops.rs
index 48e59c509886..6628ba7da048 100644
--- a/app/buck2_common/src/legacy_configs/file_ops.rs
+++ b/app/buck2_common/src/legacy_configs/file_ops.rs
@@ -13,6 +13,7 @@ use std::io::BufRead;
 use allocative::Allocative;
 use anyhow::Context;
 use buck2_core::cells::CellResolver;
+use buck2_core::fs::fs_util;
 use buck2_core::fs::paths::abs_norm_path::AbsNormPathBuf;
 use buck2_core::fs::paths::file_name::FileNameBuf;
 use buck2_core::fs::paths::forward_rel_path::ForwardRelativePath;
@@ -80,7 +81,7 @@ pub trait ConfigParserFileOps: Send + Sync {
         path: &ConfigPath,
     ) -> anyhow::Result<Box<dyn Iterator<Item = Result<String, std::io::Error>> + Send>>;
 
-    async fn file_exists(&mut self, path: &ConfigPath) -> bool;
+    async fn file_exists(&mut self, path: &ConfigPath) -> anyhow::Result<bool>;
 
     async fn read_dir(&mut self, path: &ConfigPath) -> anyhow::Result<Vec<ConfigDirEntry>>;
 }
@@ -107,8 +108,8 @@ impl ConfigParserFileOps for DefaultConfigParserFileOps {
         Ok(Box::new(file.lines()))
     }
 
-    async fn file_exists(&mut self, path: &ConfigPath) -> bool {
-        path.resolve_absolute(&self.project_fs).exists()
+    async fn file_exists(&mut self, path: &ConfigPath) -> anyhow::Result<bool> {
+        fs_util::try_exists(path.resolve_absolute(&self.project_fs)).map_err(Into::into)
     }
 
     async fn read_dir(&mut self, path: &ConfigPath) -> anyhow::Result<Vec<ConfigDirEntry>> {
@@ -177,19 +178,18 @@ impl<'a, 'b> DiceConfigFileOps<'a, 'b> {
 
 #[async_trait::async_trait]
 impl ConfigParserFileOps for DiceConfigFileOps<'_, '_> {
-    async fn file_exists(&mut self, path: &ConfigPath) -> bool {
+    async fn file_exists(&mut self, path: &ConfigPath) -> anyhow::Result<bool> {
         let ConfigPath::Project(path) = path else {
             // File is outside of project root, for example, /etc/buckconfigs.d/experiments
             return self.io_ops.file_exists(path).await;
         };
-        let Ok(path) = self.cell_resolver.get_cell_path(path) else {
-            // Can't actually happen
-            return false;
-        };
+        let path = self.cell_resolver.get_cell_path(path)?;
 
-        DiceFileComputations::read_path_metadata_if_exists(self.ctx, path.as_ref())
-            .await
-            .is_ok_and(|meta| meta.is_some())
+        Ok(
+            DiceFileComputations::read_path_metadata_if_exists(self.ctx, path.as_ref())
+                .await?
+                .is_some(),
+        )
     }
 
     async fn read_file_lines(
diff --git a/app/buck2_common/src/legacy_configs/parser.rs b/app/buck2_common/src/legacy_configs/parser.rs
index 53a64481af1f..695845b71392 100644
--- a/app/buck2_common/src/legacy_configs/parser.rs
+++ b/app/buck2_common/src/legacy_configs/parser.rs
@@ -112,7 +112,7 @@ impl LegacyConfigParser {
         follow_includes: bool,
         file_ops: &mut dyn ConfigParserFileOps,
     ) -> anyhow::Result<()> {
-        if file_ops.file_exists(path).await {
+        if file_ops.file_exists(path).await? {
             self.start_file(path, source)?;
             self.parse_file_on_stack(path, follow_includes, file_ops)
                 .await
@@ -311,7 +311,7 @@ impl LegacyConfigParser {
                         }
                     };
 
-                    match (optional, file_ops.file_exists(&include_file).await) {
+                    match (optional, file_ops.file_exists(&include_file).await?) {
                         (_, true) => {
                             self.push_file(i, &include_file)?;
                             self.parse_file_on_stack(&include_file, parse_includes, file_ops)