Skip to content

Commit

Permalink
configs: Fallible file existence check
Browse files Browse the repository at this point in the history
Summary: This operation is not actually infallible

Reviewed By: stepancheg

Differential Revision: D59664959

fbshipit-source-id: 11916f71b50919910bd9ca5491372804d1c41b10
  • Loading branch information
JakobDegen authored and facebook-github-bot committed Jul 15, 2024
1 parent 1b6aed6 commit 6983fc7
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 17 deletions.
2 changes: 1 addition & 1 deletion app/buck2_common/src/legacy_configs/cells.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions app/buck2_common/src/legacy_configs/configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
22 changes: 11 additions & 11 deletions app/buck2_common/src/legacy_configs/file_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>>;
}
Expand All @@ -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>> {
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions app/buck2_common/src/legacy_configs/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 6983fc7

Please sign in to comment.