Remove 'buck2_env_anyhow'

Summary: Originally created to help during migration away from anyhow. Not really necessary anymore except for one usage in `buck2_tpx_cli`. Decided that it's probably worth adding a `buck2_error` dependency so we can get rid of a chunk of code.

Reviewed By: JakobDegen

Differential Revision: D67072206

fbshipit-source-id: 4fbe8483c7ac7274052744b131a3ed988b0bf22c

Author: Will-MingLun-Li

app/buck2/BUCK

@@ -99,6 +99,7 @@ buck_rust_binary(
         "//buck2/app/buck2_build_info:buck2_build_info",
         "//buck2/app/buck2_client_ctx:buck2_client_ctx",
         "//buck2/app/buck2_core:buck2_core",
+        "//buck2/app/buck2_error:buck2_error",
         "//buck2/app/buck2_event_log:buck2_event_log",
         "//buck2/app/buck2_events:buck2_events",
         "//buck2/app/buck2_wrapper_common:buck2_wrapper_common",

app/buck2/bin/buck2.rs

@@ -23,7 +23,7 @@ use buck2_client_ctx::exit_result::ExitResult;
 use buck2_client_ctx::restarter::Restarter;
 use buck2_client_ctx::stdin::Stdin;
 use buck2_client_ctx::stdio;
-use buck2_core::buck2_env_anyhow;
+use buck2_core::buck2_env;
 use buck2_core::fs::working_dir::AbsWorkingDir;
 use buck2_core::logging::init_tracing_for_writer;
 use buck2_core::logging::log_file::TracingLogFile;
@@ -127,7 +127,7 @@ fn main() -> ! {
         panic::initialize()?;
         check_cargo();
 
-        let force_want_restart = buck2_env_anyhow!("FORCE_WANT_RESTART", bool)?;
+        let force_want_restart = buck2_env!("FORCE_WANT_RESTART", bool)?;
 
         let log_reload_handle = init_logging()?;
 

app/buck2/src/lib.rs

@@ -52,7 +52,7 @@ use buck2_cmd_starlark_client::StarlarkCommand;
 use buck2_common::argv::Argv;
 use buck2_common::invocation_paths_result::InvocationPathsResult;
 use buck2_common::invocation_roots::get_invocation_paths_result;
-use buck2_core::buck2_env_anyhow;
+use buck2_core::buck2_env;
 use buck2_core::fs::paths::file_name::FileNameBuf;
 use buck2_error::buck2_error;
 use buck2_error::BuckErrorContext;
@@ -190,7 +190,7 @@ impl Opt {
 
 pub fn exec(process: ProcessContext<'_>) -> ExitResult {
     let mut immediate_config = ImmediateConfigContext::new(process.working_dir);
-    let arg0_override = buck2_env_anyhow!("BUCK2_ARG0")?;
+    let arg0_override = buck2_env!("BUCK2_ARG0")?;
     let expanded_args = expand_argv(
         arg0_override,
         process.args.to_vec(),

app/buck2_core/src/env/__macro_refs.rs

@@ -13,10 +13,10 @@ use std::str::FromStr;
 
 pub use linkme;
 
-pub fn convert_from_str<T>(v: &str) -> anyhow::Result<T>
+pub fn convert_from_str<T>(v: &str) -> buck2_error::Result<T>
 where
     T: FromStr,
-    anyhow::Error: From<<T as FromStr>::Err>,
+    buck2_error::Error: From<<T as FromStr>::Err>,
 {
     Ok(T::from_str(v)?)
 }
@@ -189,151 +189,3 @@ macro register($var:expr, ty=$ty:ty, default=$default:expr, applicability=$appli
         applicability: $applicability,
     };
 }}
-
-// Code below returns anyhow::Error, it is used while we transition from anyhow to buck2_error in buck2/app
-// TODO(minglunli): Delete the code below once we have fully transitioned to buck2_error
-
-/// This macro is used to register environment variables that are used by Buck2.
-///
-/// The first argument to the macro must always be a string literal with the name of the environment
-/// variable.
-///
-/// Additionally, you can specify the following, comma separated:
-///
-///  - `type=<type>` - the Rust type that the environment variable should be converted to, using
-///    `FromStr::from_str`. Defaults to `&'static str` if not specified.
-///  - `default=<value>` - an expression for the default value to use if the environment variable is
-///    not set.
-///  - `converter=<expr>` - a function to use as an alternative to the `FromStr::from_str`
-///    conversion. Must have signature `fn(&str) -> Result<Ty, E>`
-///  - `applicability=<internal|testing>` - to indicate that the variable is not used in OSS or only
-///    for self-testing of buck2
-///
-/// The macro expands to an expression of type `anyhow::Result<Type>` if a default is set, and
-/// `anyhow::Result<Option<Type>` otherwise.
-pub macro buck2_env_anyhow {
-    ($var:expr, bool $(, $($rest:tt)*)?) => {{
-        let v: anyhow::Result<bool> = $crate::env::__macro_refs::buck2_env_anyhow!($var, type=bool, default=false, converter = |s| {
-            match s.to_lowercase().as_str() {
-                "1" | "true" => Ok(true),
-                "0" | "false" => Ok(false),
-                _ => Err(anyhow::anyhow!("Invalid bool value: {}", s)),
-            }
-        }, $($($rest)*)?);
-        v
-    }},
-    ($var:expr, type=$ty:ty, default=$default:expr, converter=$converter:expr $(, $($rest:tt)*)?) => {{
-        $crate::env::__macro_refs::parse_anyhow!(
-            (
-            var=$var,
-            parser=$converter,
-            stored_type=$ty,
-            processor=|x| x.copied().unwrap_or_else(|| $default),
-            output_type=$ty,
-            default_repr=std::option::Option::Some(stringify!($default)),
-            ),
-            $($($rest)*)?
-        )
-    }},
-    ($var:expr, type=$ty:ty, default=$default:expr $(, $($rest:tt)*)?) => {{
-        $crate::env::__macro_refs::parse_anyhow!(
-            (
-            var=$var,
-            parser=$crate::env::__macro_refs::convert_from_str,
-            stored_type=$ty,
-            processor=|x| x.copied().unwrap_or_else(|| $default),
-            output_type=$ty,
-            default_repr=std::option::Option::Some(stringify!($default)),
-            ),
-            $($($rest)*)?
-        )
-    }},
-    ($var:expr, type=$ty:ty, converter=$converter:expr $(, $($rest:tt)*)?) => {{
-        $crate::env::__macro_refs::parse_anyhow!(
-            (
-            var=$var,
-            parser=$converter,
-            stored_type=$ty,
-            processor=|x| x,
-            output_type=std::option::Option<&$ty>,
-            default_repr=std::option::Option::None,
-            ),
-            $($($rest)*)?
-        )
-    }},
-    ($var:expr, type=$ty:ty $(, $($rest:tt)*)?) => {{
-        $crate::env::__macro_refs::parse_anyhow!(
-            (
-            var=$var,
-            parser=$crate::env::__macro_refs::convert_from_str,
-            stored_type=$ty,
-            processor=|x| x.copied(),
-            output_type=std::option::Option<$ty>,
-            default_repr=std::option::Option::None,
-            ),
-            $($($rest)*)?
-        )
-    }},
-    ($var:expr $(, $($rest:tt)*)?) => {{
-        $crate::env::__macro_refs::parse_anyhow!(
-            (
-            var=$var,
-            parser=$crate::env::__macro_refs::convert_from_str,
-            stored_type=std::string::String,
-            processor=|x| x.map(|x| x.as_str()),
-            output_type=std::option::Option<&'static str>,
-            default_repr=std::option::Option::None,
-            ),
-            $($($rest)*)?
-        )
-    }},
-}
-
-#[allow(unused_macros)]
-macro parse_anyhow {
-    (
-        $already_parsed:tt,
-        applicability=internal$(,)?
-    ) => {
-        $crate::env::__macro_refs::expand_anyhow!($already_parsed, applicability=$crate::env::registry::Applicability::Internal,)
-    },
-    (
-        $already_parsed:tt,
-        applicability=testing$(,)?
-    ) => {
-        $crate::env::__macro_refs::expand_anyhow!($already_parsed, applicability=$crate::env::registry::Applicability::Testing,)
-    },
-    (
-        $already_parsed:tt,
-        $(,)?
-    ) => {
-        $crate::env::__macro_refs::expand_anyhow!($already_parsed, applicability=$crate::env::registry::Applicability::All,)
-    },
-}
-
-/// `parser` is `&str -> anyhow::Result<$stored_type>`, `processor` is `Option<& $stored_type> -> $output_type`
-///
-/// The extra set of parentheses is a trick to let us pass things through `parse2` transparently
-#[allow(unused_macros)]
-macro expand_anyhow(
-    (
-    var=$var:expr,
-    parser=$parser:expr,
-    stored_type=$stored_ty:ty,
-    processor=$processor:expr,
-    output_type=$output_ty:ty,
-    default_repr=$default_repr:expr,
-    ),
-    applicability=$applicability:expr,
-) {{
-    $crate::env::__macro_refs::register!(
-        $var,
-        ty = $stored_ty,
-        default = $default_repr,
-        applicability = $applicability
-    );
-    static ENV_HELPER: $crate::env::helper::EnvHelper<$stored_ty> =
-        $crate::env::helper::EnvHelper::with_converter_from_macro($var, $parser);
-    let v: anyhow::Result<$output_ty> = ENV_HELPER.get_anyhow().map($processor);
-    v
-}}

app/buck2_core/src/env/helper.rs

@@ -14,15 +14,15 @@ use std::sync::OnceLock;
 use buck2_error::BuckErrorContext;
 
 pub struct EnvHelper<T> {
-    convert: fn(&str) -> anyhow::Result<T>,
+    convert: fn(&str) -> buck2_error::Result<T>,
     var: &'static str,
     cell: OnceLock<Option<T>>,
 }
 
 impl<T> EnvHelper<T> {
     pub const fn with_converter_from_macro(
         var: &'static str,
-        convert: fn(&str) -> anyhow::Result<T>,
+        convert: fn(&str) -> buck2_error::Result<T>,
     ) -> Self {
         Self {
             convert,
@@ -43,7 +43,7 @@ impl<T> EnvHelper<T> {
             .get_or_try_init(move || match env::var(var) {
                 Ok(v) => {
                     tracing::info!("Env override found: ${} = {}", var, v);
-                    Ok(Some((convert)(&v).map_err(anyhow::Error::from)?))
+                    Ok(Some((convert)(&v)?))
                 }
                 Err(VarError::NotPresent) => Ok(None),
                 Err(VarError::NotUnicode(..)) => {
@@ -53,8 +53,4 @@ impl<T> EnvHelper<T> {
             .map(Option::as_ref)
             .with_buck_error_context(|| format!("Invalid value for ${}", var))
     }
-
-    pub fn get_anyhow(&'static self) -> anyhow::Result<Option<&'static T>> {
-        self.get().map_err(anyhow::Error::from)
-    }
 }

app/buck2_core/src/lib.rs

@@ -51,7 +51,6 @@ pub mod target_aliases;
 pub mod unsafe_send_future;
 
 pub use env::__macro_refs::buck2_env;
-pub use env::__macro_refs::buck2_env_anyhow;
 pub use env::__macro_refs::buck2_env_name;
 
 /// Marker for things that are only sensible to use inside Facebook,

app/buck2_execute/src/re/uploader.rs

@@ -18,7 +18,7 @@ use buck2_common::cas_digest::TrackedCasDigest;
 use buck2_common::file_ops::FileDigest;
 use buck2_common::file_ops::FileDigestKind;
 use buck2_common::file_ops::TrackedFileDigest;
-use buck2_core::buck2_env_anyhow;
+use buck2_core::buck2_env;
 use buck2_core::execution_types::executor_config::RemoteExecutorUseCase;
 use buck2_core::fs::project::ProjectRoot;
 use buck2_core::fs::project_rel_path::ProjectRelativePath;
@@ -445,19 +445,19 @@ fn add_injected_missing_digests<'a>(
     input_digests: &HashSet<&'a TrackedFileDigest>,
     missing_digests: &mut HashSet<&'a TrackedFileDigest>,
 ) -> anyhow::Result<()> {
-    fn convert_digests(val: &str) -> anyhow::Result<Vec<FileDigest>> {
+    fn convert_digests(val: &str) -> buck2_error::Result<Vec<FileDigest>> {
         val.split(' ')
             .map(|digest| {
                 let digest = TDigest::from_str(digest)
                     .with_context(|| format!("Invalid digest: `{}`", digest))?;
                 // This code does not run in a test but it is only used for testing.
                 let digest = FileDigest::from_re(&digest, DigestConfig::testing_default())?;
-                anyhow::Ok(digest)
+                buck2_error::Ok(digest)
             })
             .collect()
     }
 
-    let ingested_digests = buck2_env_anyhow!(
+    let ingested_digests = buck2_env!(
         "BUCK2_TEST_INJECTED_MISSING_DIGESTS",
         type=Vec<FileDigest>,
         converter=convert_digests,

app/buck2_execute_impl/BUCK

@@ -27,7 +27,6 @@ rust_library(
         "fbsource//third-party/rust:assert_matches",
     ],
     deps = [
-        "fbsource//third-party/rust:anyhow",
         "fbsource//third-party/rust:async-condvar-fair",
         "fbsource//third-party/rust:async-trait",
         "fbsource//third-party/rust:chrono",

app/buck2_execute_impl/src/materializers/deferred/io_handler.rs

@@ -479,15 +479,15 @@ fn maybe_tombstone_digest(digest: &FileDigest) -> buck2_error::Result<&FileDiges
     // instead of a not-found error.
     static TOMBSTONE_DIGEST: Lazy<FileDigest> = Lazy::new(|| FileDigest::new_sha1([0; 20], 1));
 
-    fn convert_digests(val: &str) -> anyhow::Result<HashSet<FileDigest>> {
+    fn convert_digests(val: &str) -> buck2_error::Result<HashSet<FileDigest>> {
         val.split(' ')
             .map(|digest| {
                 let digest = TDigest::from_str(digest)
                     .with_buck_error_context(|| format!("Invalid digest: `{}`", digest))?;
                 // This code is only used by E2E tests, so while it's not *a test*, testing_default
                 // is an OK choice here.
                 let digest = FileDigest::from_re(&digest, DigestConfig::testing_default())?;
-                anyhow::Ok(digest)
+                buck2_error::Ok(digest)
             })
             .collect()
     }

app/buck2_test/src/executor_launcher.rs

@@ -17,7 +17,7 @@ use std::task::Poll;
 
 use anyhow::Context as _;
 use async_trait::async_trait;
-use buck2_core::buck2_env_anyhow;
+use buck2_core::buck2_env;
 use buck2_events::dispatch::EventDispatcher;
 use buck2_grpc::DuplexChannel;
 use buck2_grpc::ServerHandle;
@@ -138,7 +138,7 @@ impl ExecutorLauncher for OutOfProcessTestExecutor {
     async fn launch(&self, tpx_args: Vec<String>) -> anyhow::Result<ExecutorLaunch> {
         // Declare outside of `cfg(unix)` so `buck2 help-env` would include it on Windows
         // even if it is no-op on Windows.
-        let use_tcp = buck2_env_anyhow!("BUCK2_TEST_TPX_USE_TCP", bool)?;
+        let use_tcp = buck2_env!("BUCK2_TEST_TPX_USE_TCP", bool)?;
 
         if !use_tcp {
             #[cfg(unix)]