fix(turborepo): Adopt std::cell::OnceCell (#5763)

We're using Tokio's `OnceCell` for configuration loading, but `OnceCell`
has been stabilized in core. Use that instead.

Closes TURBO-1224

crates/turborepo-lib/src/commands/link.rs

@@ -460,11 +460,10 @@ fn add_space_id_to_turbo_json(base: &CommandBase, space_id: &str) -> Result<()>
 
 #[cfg(test)]
 mod test {
-    use std::fs;
+    use std::{cell::OnceCell, fs};
 
     use anyhow::Result;
     use tempfile::{NamedTempFile, TempDir};
-    use tokio::sync::OnceCell;
     use turbopath::AbsoluteSystemPathBuf;
     use turborepo_ui::UI;
     use vercel_api_mock::start_test_server;

crates/turborepo-lib/src/commands/login.rs

@@ -298,12 +298,11 @@ async fn run_sso_one_shot_server(
 
 #[cfg(test)]
 mod test {
-    use std::fs;
+    use std::{cell::OnceCell, fs};
 
     use reqwest::Url;
     use serde::Deserialize;
     use tempfile::{tempdir, NamedTempFile};
-    use tokio::sync::OnceCell;
     use turbopath::AbsoluteSystemPathBuf;
     use turborepo_ui::UI;
     use vercel_api_mock::start_test_server;

crates/turborepo-lib/src/commands/mod.rs

@@ -1,16 +1,15 @@
-use std::borrow::Borrow;
+use std::{borrow::Borrow, cell::OnceCell};
 
 use anyhow::Result;
 use sha2::{Digest, Sha256};
-use tokio::sync::OnceCell;
 use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf};
 use turborepo_api_client::APIClient;
 use turborepo_ui::UI;
 
 use crate::{
     config::{
         default_user_config_path, get_repo_config_path, ClientConfig, ClientConfigLoader,
-        RepoConfig, RepoConfigLoader, UserConfig, UserConfigLoader,
+        Error as ConfigError, RepoConfig, RepoConfigLoader, UserConfig, UserConfigLoader,
     },
     Args,
 };
@@ -55,18 +54,24 @@ impl CommandBase {
         })
     }
 
-    fn create_repo_config(&self) -> Result<()> {
+    fn repo_config_init(&self) -> Result<RepoConfig, ConfigError> {
         let repo_config_path = get_repo_config_path(self.repo_root.borrow());
 
-        let repo_config = RepoConfigLoader::new(repo_config_path)
+        RepoConfigLoader::new(repo_config_path)
             .with_api(self.args.api.clone())
             .with_login(self.args.login.clone())
             .with_team_slug(self.args.team.clone())
-            .load()?;
+            .load()
+    }
 
-        self.repo_config.set(repo_config)?;
+    pub fn repo_config(&self) -> Result<&RepoConfig, ConfigError> {
+        self.repo_config.get_or_try_init(|| self.repo_config_init())
+    }
 
-        Ok(())
+    pub fn repo_config_mut(&mut self) -> Result<&mut RepoConfig, ConfigError> {
+        // Approximates `get_mut_or_try_init`
+        self.repo_config()?;
+        Ok(self.repo_config.get_mut().unwrap())
     }
 
     // NOTE: This deletes the repo config file. It does *not* remove the
@@ -81,62 +86,37 @@ impl CommandBase {
         Ok(())
     }
 
-    fn create_user_config(&self) -> Result<()> {
-        let user_config = UserConfigLoader::new(default_user_config_path()?)
+    fn user_config_init(&self) -> Result<UserConfig, ConfigError> {
+        UserConfigLoader::new(default_user_config_path()?)
             .with_token(self.args.token.clone())
-            .load()?;
-        self.user_config.set(user_config)?;
-
-        Ok(())
+            .load()
     }
 
-    fn create_client_config(&self) -> Result<()> {
-        let client_config = ClientConfigLoader::new()
-            .with_remote_cache_timeout(self.args.remote_cache_timeout)
-            .load()?;
-        self.client_config.set(client_config)?;
-
-        Ok(())
+    pub fn user_config(&self) -> Result<&UserConfig, ConfigError> {
+        self.user_config.get_or_try_init(|| self.user_config_init())
     }
 
-    pub fn repo_config_mut(&mut self) -> Result<&mut RepoConfig> {
-        if self.repo_config.get().is_none() {
-            self.create_repo_config()?;
-        }
-
-        Ok(self.repo_config.get_mut().unwrap())
-    }
-
-    pub fn repo_config(&self) -> Result<&RepoConfig> {
-        if self.repo_config.get().is_none() {
-            self.create_repo_config()?;
-        }
-
-        Ok(self.repo_config.get().unwrap())
-    }
-
-    pub fn user_config_mut(&mut self) -> Result<&mut UserConfig> {
-        if self.user_config.get().is_none() {
-            self.create_user_config()?;
-        }
-
+    pub fn user_config_mut(&mut self) -> Result<&mut UserConfig, ConfigError> {
+        // Approximates `get_mut_or_try_init`
+        self.user_config()?;
         Ok(self.user_config.get_mut().unwrap())
     }
 
-    pub fn user_config(&self) -> Result<&UserConfig> {
-        if self.user_config.get().is_none() {
-            self.create_user_config()?;
-        }
-
-        Ok(self.user_config.get().unwrap())
+    fn client_config_init(&self) -> Result<ClientConfig, ConfigError> {
+        ClientConfigLoader::new()
+            .with_remote_cache_timeout(self.args.remote_cache_timeout)
+            .load()
     }
 
-    pub fn client_config(&self) -> Result<&ClientConfig> {
-        if self.client_config.get().is_none() {
-            self.create_client_config()?;
-        }
+    pub fn client_config(&self) -> Result<&ClientConfig, ConfigError> {
+        self.client_config
+            .get_or_try_init(|| self.client_config_init())
+    }
 
-        Ok(self.client_config.get().unwrap())
+    pub fn client_config_mut(&mut self) -> Result<&mut ClientConfig, ConfigError> {
+        // Approximates `get_mut_or_try_init`
+        self.client_config()?;
+        Ok(self.client_config.get_mut().unwrap())
     }
 
     pub fn args(&self) -> &Args {

crates/turborepo-lib/src/config/client.rs

@@ -1,9 +1,11 @@
 use std::collections::HashMap;
 
 use anyhow::Result;
-use config::{Config, ConfigError, Environment};
+use config::{Config, Environment};
 use serde::{Deserialize, Serialize};
 
+use crate::config::Error;
+
 const DEFAULT_TIMEOUT: u64 = 20;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -49,28 +51,25 @@ impl ClientConfigLoader {
         self
     }
 
-    pub fn load(self) -> Result<ClientConfig> {
+    pub fn load(self) -> Result<ClientConfig, Error> {
         let Self {
             remote_cache_timeout,
             environment,
         } = self;
 
-        let config_attempt: Result<ClientConfigValue, ConfigError> = Config::builder()
+        let config_attempt = Config::builder()
             .set_default("remote_cache_timeout", DEFAULT_TIMEOUT)?
             .add_source(Environment::with_prefix("turbo").source(environment))
             .set_override_option("remote_cache_timeout", remote_cache_timeout)?
             .build()?
+            // Deserialize is the only user-input-fallible step.
+            // Everything else is programmer error.
+            // This goes wrong when TURBO_REMOTE_CACHE_TIMEOUT can't be deserialized to u64
             .try_deserialize();
 
-        // This goes wrong when TURBO_REMOTE_CACHE_TIMEOUT can't be deserialized to u64
-        match config_attempt {
-            Err(_) => Ok(ClientConfig {
-                config: ClientConfigValue {
-                    remote_cache_timeout: DEFAULT_TIMEOUT,
-                },
-            }),
-            Ok(config) => Ok(ClientConfig { config }),
-        }
+        config_attempt
+            .map_err(Error::Config)
+            .map(|config| ClientConfig { config })
     }
 }
 
@@ -137,68 +136,87 @@ mod test {
             arg: Option<u64>,
             env: String,
             output: u64,
+            want_err: Option<&'static str>,
         }
 
         let tests = [
             TestCase {
                 arg: Some(0),
                 env: String::from("0"),
                 output: 0,
+                want_err: None,
             },
             TestCase {
                 arg: Some(0),
                 env: String::from("2"),
                 output: 0,
+                want_err: None,
             },
             TestCase {
                 arg: Some(0),
                 env: String::from("garbage"),
                 output: 0,
+                want_err: None,
             },
             TestCase {
                 arg: Some(0),
                 env: String::from(""),
                 output: 0,
+                want_err: None,
             },
             TestCase {
                 arg: Some(1),
                 env: String::from("0"),
                 output: 1,
+                want_err: None,
             },
             TestCase {
                 arg: Some(1),
                 env: String::from("2"),
                 output: 1,
+                want_err: None,
             },
             TestCase {
                 arg: Some(1),
                 env: String::from("garbage"),
                 output: 1,
+                want_err: None,
             },
             TestCase {
                 arg: Some(1),
                 env: String::from(""),
                 output: 1,
+                want_err: None,
             },
             TestCase {
                 arg: None,
                 env: String::from("0"),
                 output: 0,
+                want_err: None,
             },
             TestCase {
                 arg: None,
                 env: String::from("2"),
                 output: 2,
+                want_err: None,
             },
             TestCase {
                 arg: None,
                 env: String::from("garbage"),
                 output: DEFAULT_TIMEOUT,
+                want_err: Some(
+                    "invalid type: string \"garbage\", expected an integer for key \
+                     `remote_cache_timeout` in the environment",
+                ),
             },
             TestCase {
                 arg: None,
                 env: String::from(""),
                 output: DEFAULT_TIMEOUT,
+                want_err: Some(
+                    "invalid type: string \"\", expected an integer for key \
+                     `remote_cache_timeout` in the environment",
+                ),
             },
         ];
 
@@ -211,9 +229,13 @@ mod test {
                     env.insert("TURBO_REMOTE_CACHE_TIMEOUT".into(), test.env.clone());
                     Some(env)
                 })
-                .load()?;
+                .load();
 
-            assert_eq!(config.remote_cache_timeout(), test.output);
+            if test.want_err.is_none() {
+                assert_eq!(config.unwrap().remote_cache_timeout(), test.output);
+            } else {
+                assert_eq!(config.err().unwrap().to_string(), test.want_err.unwrap());
+            }
         }
 
         // We can only hit the actual system for env vars in a single test
@@ -222,9 +244,13 @@ mod test {
             set_var("TURBO_REMOTE_CACHE_TIMEOUT", test.env.clone());
             let config = ClientConfigLoader::new()
                 .with_remote_cache_timeout(test.arg)
-                .load()?;
+                .load();
 
-            assert_eq!(config.remote_cache_timeout(), test.output);
+            if test.want_err.is_none() {
+                assert_eq!(config.unwrap().remote_cache_timeout(), test.output);
+            } else {
+                assert_eq!(config.err().unwrap().to_string(), test.want_err.unwrap());
+            }
         }
 
         Ok(())

crates/turborepo-lib/src/lib.rs

@@ -3,6 +3,7 @@
 #![feature(error_generic_member_access)]
 #![feature(provide_any)]
 #![feature(option_get_or_insert_default)]
+#![feature(once_cell_try)]
 #![deny(clippy::all)]
 // Clippy's needless mut lint is buggy: https://github.com/rust-lang/rust-clippy/issues/11299
 #![allow(clippy::needless_pass_by_ref_mut)]