Skip to content

Commit

Permalink
from_str to return buck2_error
Browse files Browse the repository at this point in the history
Summary:
Have `from_str` for `AbsPathBuf` to return `buck2_error` instead of anyhow.

A few inputs to `clap` had to be converted to string as conversion could fail with `buck2_error` which clap doesn't understand. So instead take in a String instead and do the conversion in code.

This allows us to remove a bunch of conversions and dependency on `anyhow` from `buck2_core`

Reviewed By: JakobDegen

Differential Revision: D67764601

fbshipit-source-id: 83a46751eb80bdd677945e9f55b89735bd838ec9
  • Loading branch information
Will-MingLun-Li authored and facebook-github-bot committed Jan 6, 2025
1 parent a0faa18 commit 3cc8f9a
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 33 deletions.
7 changes: 4 additions & 3 deletions app/buck2/src/commands/forkserver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub(crate) struct ForkserverCommand {
fd: RawFd,

#[clap(long)]
state_dir: AbsNormPathBuf,
state_dir: String,

#[clap(long, value_parser = ResourceControlConfig::deserialize)]
resource_control: ResourceControlConfig,
Expand All @@ -42,7 +42,8 @@ impl ForkserverCommand {
_ctx: ClientCommandContext<'_>,
log_reload_handle: Arc<dyn LogConfigurationReloadHandle>,
) -> anyhow::Result<()> {
fs_util::create_dir_all(&self.state_dir).map_err(buck2_error::Error::from)?;
let state_dir = AbsNormPathBuf::from(self.state_dir)?;
fs_util::create_dir_all(&state_dir).map_err(buck2_error::Error::from)?;

#[cfg(unix)]
{
Expand All @@ -57,7 +58,7 @@ impl ForkserverCommand {
Ok(rt.block_on(buck2_forkserver::unix::run_forkserver(
self.fd,
log_reload_handle,
self.state_dir,
state_dir,
self.resource_control,
))?)
}
Expand Down
6 changes: 4 additions & 2 deletions app/buck2_client/src/commands/debug/persist_event_logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub struct PersistEventLogsCommand {
#[clap(long, help = "Name this log will take in Manifold")]
manifold_name: String,
#[clap(long, help = "Where to write this log to on disk")]
local_path: AbsPathBuf,
local_path: String,
#[clap(long, help = "If present, only write to disk and don't upload")]
no_upload: bool,
#[clap(
Expand Down Expand Up @@ -133,7 +133,9 @@ async fn write_task(
Ok(())
}

async fn create_log_file(local_path: AbsPathBuf) -> Result<tokio::fs::File, buck2_error::Error> {
async fn create_log_file(local_path: String) -> Result<tokio::fs::File, buck2_error::Error> {
let local_path = AbsPathBuf::new(local_path)?;

let file = OpenOptions::new()
.create(true)
.append(true)
Expand Down
1 change: 0 additions & 1 deletion app/buck2_core/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ rust_library(
"fbsource//third-party/rust:test-case",
],
deps = [
"fbsource//third-party/rust:anyhow",
"fbsource//third-party/rust:arc-swap",
"fbsource//third-party/rust:blake3",
"fbsource//third-party/rust:compact_str",
Expand Down
1 change: 0 additions & 1 deletion app/buck2_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ repository = { workspace = true }
version = "0.1.0"

[dependencies]
anyhow = { workspace = true }
arc-swap = { workspace = true }
blake3 = { workspace = true }
compact_str = { workspace = true }
Expand Down
36 changes: 16 additions & 20 deletions app/buck2_core/src/fs/paths/abs_norm_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use std::path::PathBuf;
use std::str::FromStr;

use allocative::Allocative;
use buck2_error::conversion::from_any;
use derive_more::Display;
use ref_cast::RefCast;
use relative_path::RelativePath;
Expand Down Expand Up @@ -271,18 +270,15 @@ impl AbsNormPath {

#[cfg(not(windows))]
fn strip_prefix_impl(&self, base: &AbsNormPath) -> buck2_error::Result<&Path> {
self.0
.strip_prefix(&base.0)
.map_err(buck2_error::Error::from)
self.0.strip_prefix(&base.0)
}

#[cfg(windows)]
fn strip_prefix_impl(&self, base: &AbsNormPath) -> buck2_error::Result<&Path> {
if self.windows_prefix()? == base.windows_prefix()? {
self.strip_windows_prefix()
.map_err(from_any)?
.strip_prefix(base.strip_windows_prefix()?)
.map_err(from_any)
Ok(self
.strip_windows_prefix()?
.strip_prefix(base.strip_windows_prefix()?)?)
} else {
Err(buck2_error::buck2_error!([], "Path is not a prefix"))
}
Expand Down Expand Up @@ -428,7 +424,7 @@ impl AbsNormPath {
}
let path_buf = stack.iter().collect::<PathBuf>();

AbsNormPathBuf::try_from(path_buf).map_err(from_any)
AbsNormPathBuf::try_from(path_buf)
}

/// Convert to an owned [`AbsNormPathBuf`].
Expand Down Expand Up @@ -552,7 +548,7 @@ impl AbsNormPath {
if let Some(component) = iter.next() {
prefix.push(component);
}
self.as_path().strip_prefix(&prefix).map_err(from_any)
Ok(self.as_path().strip_prefix(&prefix)?)
}

pub fn ancestors(&self) -> impl Iterator<Item = &'_ AbsNormPath> {
Expand All @@ -571,7 +567,7 @@ impl AbsNormPath {

impl AbsNormPathBuf {
pub fn new(path: PathBuf) -> buck2_error::Result<AbsNormPathBuf> {
let path = AbsPathBuf::try_from(path).map_err(from_any)?;
let path = AbsPathBuf::try_from(path)?;
verify_abs_path(&path)?;
Ok(AbsNormPathBuf(path))
}
Expand All @@ -589,7 +585,7 @@ impl AbsNormPathBuf {
}

pub fn from(s: String) -> buck2_error::Result<Self> {
AbsNormPathBuf::try_from(s).map_err(from_any)
AbsNormPathBuf::try_from(s)
}

/// Creates a new 'AbsPathBuf' with a given capacity used to create the internal
Expand Down Expand Up @@ -730,7 +726,7 @@ impl AbsNormPathBuf {
}

impl TryFrom<String> for AbsNormPathBuf {
type Error = anyhow::Error;
type Error = buck2_error::Error;

/// no allocation conversion
///
Expand All @@ -753,22 +749,22 @@ impl TryFrom<String> for AbsNormPathBuf {
/// assert!(AbsNormPathBuf::try_from("c:/normalize/../bar".to_owned()).is_err());
/// }
/// ```
fn try_from(s: String) -> anyhow::Result<AbsNormPathBuf> {
fn try_from(s: String) -> buck2_error::Result<AbsNormPathBuf> {
AbsNormPathBuf::try_from(OsString::from(s))
}
}

impl TryFrom<OsString> for AbsNormPathBuf {
type Error = anyhow::Error;
type Error = buck2_error::Error;

// no allocation
fn try_from(s: OsString) -> anyhow::Result<AbsNormPathBuf> {
fn try_from(s: OsString) -> buck2_error::Result<AbsNormPathBuf> {
AbsNormPathBuf::try_from(PathBuf::from(s))
}
}

impl TryFrom<PathBuf> for AbsNormPathBuf {
type Error = anyhow::Error;
type Error = buck2_error::Error;

/// no allocation conversion
///
Expand All @@ -792,17 +788,17 @@ impl TryFrom<PathBuf> for AbsNormPathBuf {
/// assert!(AbsNormPathBuf::try_from(PathBuf::from("c:/normalize/../bar")).is_err());
/// }
/// ```
fn try_from(p: PathBuf) -> anyhow::Result<AbsNormPathBuf> {
fn try_from(p: PathBuf) -> buck2_error::Result<AbsNormPathBuf> {
let p = AbsPathBuf::try_from(p)?;
verify_abs_path(&p)?;
Ok(AbsNormPathBuf(p))
}
}

impl FromStr for AbsNormPathBuf {
type Err = anyhow::Error;
type Err = buck2_error::Error;

fn from_str(s: &str) -> anyhow::Result<Self> {
fn from_str(s: &str) -> buck2_error::Result<Self> {
AbsNormPathBuf::try_from(s.to_owned())
}
}
Expand Down
11 changes: 5 additions & 6 deletions app/buck2_core/src/fs/paths/abs_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use std::path::PathBuf;
use std::str::FromStr;

use allocative::Allocative;
use buck2_error::conversion::from_any;
use derive_more::Display;
use ref_cast::RefCast;

Expand Down Expand Up @@ -174,7 +173,7 @@ impl AbsPath {
}

pub fn strip_prefix<P: AsRef<AbsPath>>(&self, prefix: P) -> buck2_error::Result<&Path> {
self.0.strip_prefix(prefix.as_ref()).map_err(from_any)
Ok(self.0.strip_prefix(prefix.as_ref())?)
}

pub fn ancestors(&self) -> impl Iterator<Item = &'_ AbsPath> {
Expand Down Expand Up @@ -246,7 +245,7 @@ impl AbsPathBuf {
}

impl TryFrom<PathBuf> for AbsPathBuf {
type Error = anyhow::Error;
type Error = buck2_error::Error;

fn try_from(path: PathBuf) -> Result<Self, Self::Error> {
AbsPath::new(&path)?;
Expand All @@ -255,17 +254,17 @@ impl TryFrom<PathBuf> for AbsPathBuf {
}

impl TryFrom<String> for AbsPathBuf {
type Error = anyhow::Error;
type Error = buck2_error::Error;

fn try_from(path: String) -> Result<Self, Self::Error> {
AbsPathBuf::try_from(PathBuf::from(path))
}
}

impl FromStr for AbsPathBuf {
type Err = anyhow::Error;
type Err = buck2_error::Error;

fn from_str(s: &str) -> anyhow::Result<AbsPathBuf> {
fn from_str(s: &str) -> buck2_error::Result<AbsPathBuf> {
AbsPathBuf::try_from(s.to_owned())
}
}

0 comments on commit 3cc8f9a

Please sign in to comment.