Skip to content

Commit

Permalink
refactor(opts): remove config from opts (#9562)
Browse files Browse the repository at this point in the history
### Description

Remove the config field from `Opts` entirely. I'm doing this because the
config data already gets partially moved into `Opts`, so by keeping the
config around, we have multiple sources for the same data. Now `Opts` is
the fully transformed, normalized config for a run.

You can review this commit by commit if you like

### Testing Instructions
  • Loading branch information
NicholasLYang authored Dec 4, 2024
1 parent 985b8f7 commit 2f0a75b
Show file tree
Hide file tree
Showing 13 changed files with 155 additions and 72 deletions.
7 changes: 6 additions & 1 deletion crates/turborepo-cache/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,12 @@ mod test {
"2.0.0",
true,
)?;
let opts = CacheOpts::default();
let opts = CacheOpts {
cache_dir: ".turbo/cache".into(),
cache: Default::default(),
workers: 0,
remote_cache_opts: None,
};
let api_auth = APIAuth {
team_id: Some("my-team".to_string()),
token: "my-token".to_string(),
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl Default for CacheActions {
}
}

#[derive(Clone, Debug, Default, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct CacheOpts {
pub cache_dir: Utf8PathBuf,
pub cache: CacheConfig,
Expand Down
3 changes: 1 addition & 2 deletions crates/turborepo-lib/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1283,8 +1283,7 @@ pub async fn run(
}
}
Command::Config => {
let base = CommandBase::new(cli_args.clone(), repo_root, version, color_config)?;
config::run(base).await?;
config::run(repo_root, cli_args).await?;
Ok(0)
}
Command::Ls {
Expand Down
11 changes: 6 additions & 5 deletions crates/turborepo-lib/src/commands/config.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use camino::Utf8Path;
use serde::Serialize;
use turbopath::AbsoluteSystemPathBuf;
use turborepo_repository::{package_graph::PackageGraph, package_json::PackageJson};

use crate::{cli, cli::EnvMode, commands::CommandBase, turbo_json::UIMode};
use crate::{cli, cli::EnvMode, commands::CommandBase, turbo_json::UIMode, Args};

#[derive(Debug, Serialize)]
#[serde(rename_all = "camelCase")]
Expand All @@ -26,11 +27,11 @@ struct ConfigOutput<'a> {
cache_dir: &'a Utf8Path,
}

pub async fn run(base: CommandBase) -> Result<(), cli::Error> {
let config = base.config();
let root_package_json = PackageJson::load(&base.repo_root.join_component("package.json"))?;
pub async fn run(repo_root: AbsoluteSystemPathBuf, args: Args) -> Result<(), cli::Error> {
let config = CommandBase::load_config(&repo_root, &args)?;
let root_package_json = PackageJson::load(&repo_root.join_component("package.json"))?;

let package_graph = PackageGraph::builder(&base.repo_root, root_package_json)
let package_graph = PackageGraph::builder(&repo_root, root_package_json)
.build()
.await?;

Expand Down
9 changes: 5 additions & 4 deletions crates/turborepo-lib/src/commands/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,9 @@ pub async fn link(
let api_client = base.api_client()?;
let token = base
.opts()
.config
.token()
.api_client_opts
.token
.as_deref()
.ok_or_else(|| Error::TokenNotFound {
command: base.color_config.apply(BOLD.apply_to("`npx turbo login`")),
})?;
Expand Down Expand Up @@ -639,7 +640,7 @@ mod test {
.build()?;

let mut base = CommandBase::from_opts(
Opts::new(&Args::default(), config)?,
Opts::new(&repo_root, &Args::default(), config)?,
repo_root.clone(),
"1.0.0",
ColorConfig::new(false),
Expand Down Expand Up @@ -701,7 +702,7 @@ mod test {
.build()?;

let mut base = CommandBase::from_opts(
Opts::new(&Args::default(), config)?,
Opts::new(&repo_root, &Args::default(), config)?,
repo_root.clone(),
"",
ColorConfig::new(false),
Expand Down
10 changes: 6 additions & 4 deletions crates/turborepo-lib/src/commands/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ pub async fn sso_login(
telemetry.track_login_method(LoginMethod::SSO);
let api_client: APIClient = base.api_client()?;
let color_config = base.color_config;
let login_url_config = base.config().login_url().to_string();
let login_url_config = base.opts.api_client_opts.login_url.to_string();
let options = LoginOptions {
existing_token: base.config().token(),
existing_token: base.opts.api_client_opts.token.as_deref(),
sso_team: Some(sso_team),
force,
..LoginOptions::new(
Expand Down Expand Up @@ -72,9 +72,11 @@ pub async fn login(

let api_client: APIClient = base.api_client()?;
let color_config = base.color_config;
let login_url_config = base.config().login_url().to_string();
let login_url_config = base.opts.api_client_opts.login_url.to_string();
let existing_token = base.opts.api_client_opts.token.as_deref();

let options = LoginOptions {
existing_token: base.config().token(),
existing_token,
force,
..LoginOptions::new(
&color_config,
Expand Down
27 changes: 10 additions & 17 deletions crates/turborepo-lib/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ impl CommandBase {
version: &'static str,
color_config: ColorConfig,
) -> Result<Self, cli::Error> {
let config = Self::config_init(&repo_root, &args)?;
let opts = Opts::new(&args, config)?;
let config = Self::load_config(&repo_root, &args)?;
let opts = Opts::new(&repo_root, &args, config)?;

Ok(Self {
repo_root,
Expand All @@ -69,7 +69,7 @@ impl CommandBase {
}
}

fn config_init(
pub fn load_config(
repo_root: &AbsoluteSystemPath,
args: &Args,
) -> Result<ConfigurationOptions, ConfigError> {
Expand Down Expand Up @@ -143,11 +143,10 @@ impl CommandBase {
}

pub fn api_auth(&self) -> Result<Option<APIAuth>, ConfigError> {
let config = self.config();
let team_id = config.team_id();
let team_slug = config.team_slug();
let team_id = self.opts.api_client_opts.team_id.as_ref();
let team_slug = self.opts.api_client_opts.team_slug.as_ref();

let Some(token) = config.token() else {
let Some(token) = &self.opts.api_client_opts.token else {
return Ok(None);
};

Expand All @@ -158,18 +157,12 @@ impl CommandBase {
}))
}

pub fn config(&self) -> &ConfigurationOptions {
&self.opts.config
}

pub fn api_client(&self) -> Result<APIClient, ConfigError> {
let config = &self.opts.config;
let api_url = config.api_url();
let timeout = config.timeout();
let upload_timeout = config.upload_timeout();
let timeout = self.opts.api_client_opts.timeout;
let upload_timeout = self.opts.api_client_opts.upload_timeout;

APIClient::new(
api_url,
&self.opts.api_client_opts.api_url,
if timeout > 0 {
Some(Duration::from_secs(timeout))
} else {
Expand All @@ -181,7 +174,7 @@ impl CommandBase {
None
},
self.version,
config.preflight(),
self.opts.api_client_opts.preflight,
)
.map_err(ConfigError::ApiClient)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/commands/prune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ impl<'a> Prune<'a> {
output_dir: &str,
telemetry: CommandEventBuilder,
) -> Result<Self, Error> {
let allow_missing_package_manager = base.config().allow_no_package_manager();
let allow_missing_package_manager = base.opts().repo_opts.allow_no_package_manager;
telemetry.track_arg_usage(
"dangerously-allow-missing-package-manager",
allow_missing_package_manager,
Expand Down
6 changes: 4 additions & 2 deletions crates/turborepo-lib/src/commands/unlink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ enum UnlinkSpacesResult {
}

fn unlink_remote_caching(base: &mut CommandBase) -> Result<(), cli::Error> {
let needs_disabling = base.config().team_id().is_some() || base.config().team_slug().is_some();
let needs_disabling = base.opts.api_client_opts.team_id.is_some()
|| base.opts.api_client_opts.team_slug.is_some();

let output = if needs_disabling {
let local_config_path = base.local_config_path();
Expand Down Expand Up @@ -56,7 +57,8 @@ fn unlink_remote_caching(base: &mut CommandBase) -> Result<(), cli::Error> {
}

fn unlink_spaces(base: &mut CommandBase) -> Result<(), cli::Error> {
let needs_disabling = base.config().team_id().is_some() || base.config().team_slug().is_some();
let needs_disabling = base.opts().api_client_opts.team_id.is_some()
|| base.opts().api_client_opts.team_slug.is_some();

if needs_disabling {
let local_config_path = base.local_config_path();
Expand Down
4 changes: 2 additions & 2 deletions crates/turborepo-lib/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,8 @@ impl Diagnostic for RemoteCacheDiagnostic {
let (has_team_id, has_team_slug) = {
let base = base.lock().await;
(
base.config().team_id().is_some(),
base.config().team_slug().is_some(),
base.opts().api_client_opts.team_id.is_some(),
base.opts().api_client_opts.team_slug.is_some(),
)
};

Expand Down
Loading

0 comments on commit 2f0a75b

Please sign in to comment.