Skip to content

Commit

Permalink
chore(Turborepo): Move more CLI stuff out of run code (#7085)
Browse files Browse the repository at this point in the history
### Description

 - setup `api_auth` and `api_client` outside of a run
 - remove the lifetime from `Opts` and related structs
- keep a copy of `Opts` on `Run`. This is likely not the final
structure, but is an intermediate step.
 - keep a copy of `api_auth` on `Run`
- Move most of the calculations on `Opts` into the constructor for
`Run`. This should likely eventually be a `TryFrom<CommandBase>` instead
of `new`. The goal is to be able to make `Run` itself immutable since
all of the config and options will have been resolved ahead of time

### Testing Instructions

Existing test suite

Closes TURBO-2135

---------

Co-authored-by: Greg Soltis <Greg Soltis>
  • Loading branch information
Greg Soltis committed Jan 25, 2024
1 parent a1a2b6b commit 59350bf
Show file tree
Hide file tree
Showing 13 changed files with 149 additions and 144 deletions.
6 changes: 6 additions & 0 deletions crates/turborepo-api-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ pub struct APIAuth {
pub team_slug: Option<String>,
}

pub fn is_linked(api_auth: &Option<APIAuth>) -> bool {
api_auth
.as_ref()
.map_or(false, |api_auth| api_auth.is_linked())
}

#[async_trait]
impl Client for APIClient {
async fn get_user(&self, token: &str) -> Result<UserResponse> {
Expand Down
6 changes: 3 additions & 3 deletions crates/turborepo-cache/src/async_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ mod tests {
skip_filesystem: true,
workers: 10,
remote_cache_opts: Some(RemoteCacheOpts {
team_id: "my-team".to_string(),
unused_team_id: Some("my-team".to_string()),
signature: false,
}),
};
Expand Down Expand Up @@ -290,7 +290,7 @@ mod tests {
skip_filesystem: false,
workers: 10,
remote_cache_opts: Some(RemoteCacheOpts {
team_id: "my-team".to_string(),
unused_team_id: Some("my-team".to_string()),
signature: false,
}),
};
Expand Down Expand Up @@ -374,7 +374,7 @@ mod tests {
skip_filesystem: false,
workers: 10,
remote_cache_opts: Some(RemoteCacheOpts {
team_id: "my-team".to_string(),
unused_team_id: Some("my-team".to_string()),
signature: false,
}),
};
Expand Down
15 changes: 9 additions & 6 deletions crates/turborepo-cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ mod test_cases;
use std::{backtrace, backtrace::Backtrace};

pub use async_cache::AsyncCache;
use camino::Utf8Path;
use camino::Utf8PathBuf;
use serde::{Deserialize, Serialize};
use thiserror::Error;

Expand Down Expand Up @@ -95,8 +95,8 @@ pub struct CacheHitMetadata {
}

#[derive(Debug, Default)]
pub struct CacheOpts<'a> {
pub override_dir: Option<&'a Utf8Path>,
pub struct CacheOpts {
pub override_dir: Option<Utf8PathBuf>,
pub remote_cache_read_only: bool,
pub skip_remote: bool,
pub skip_filesystem: bool,
Expand All @@ -106,12 +106,15 @@ pub struct CacheOpts<'a> {

#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct RemoteCacheOpts {
team_id: String,
unused_team_id: Option<String>,
signature: bool,
}

impl RemoteCacheOpts {
pub fn new(team_id: String, signature: bool) -> Self {
Self { team_id, signature }
pub fn new(unused_team_id: Option<String>, signature: bool) -> Self {
Self {
unused_team_id,
signature,
}
}
}
8 changes: 7 additions & 1 deletion crates/turborepo-cache/src/multiplexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,13 @@ impl CacheMultiplexer {
}

let fs_cache = use_fs_cache
.then(|| FSCache::new(opts.override_dir, repo_root, analytics_recorder.clone()))
.then(|| {
FSCache::new(
opts.override_dir.as_deref(),
repo_root,
analytics_recorder.clone(),
)
})
.transpose()?;

let http_cache = use_http_cache
Expand Down
11 changes: 10 additions & 1 deletion crates/turborepo-lib/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1196,7 +1196,16 @@ pub async fn run(
} else {
use crate::commands::run;
event.track_run_code_path(CodePath::Rust);
let exit_code = run::run(base, event).await?;
let exit_code = run::run(base, event)
.await
.inspect(|code| {
if *code != 0 {
error!("run failed: command exited ({code})");
}
})
.inspect_err(|err| {
error!("run failed: {err}");
})?;
Ok(Payload::Rust(Ok(exit_code)))
}
}
Expand Down
22 changes: 6 additions & 16 deletions crates/turborepo-lib/src/commands/run.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use tracing::{debug, error};
use tracing::debug;
use turborepo_telemetry::events::command::CommandEventBuilder;

use crate::{commands::CommandBase, run, run::Run, signal::SignalHandler};
Expand Down Expand Up @@ -30,10 +30,11 @@ pub async fn run(base: CommandBase, telemetry: CommandEventBuilder) -> Result<i3

let handler = SignalHandler::new(signal);

let mut run = Run::new(base);
let api_auth = base.api_auth()?;
let api_client = base.api_client()?;
let mut run = Run::new(base, api_auth)?;
debug!("using the experimental rust codepath");
debug!("configured run struct: {:?}", run);
let run_fut = run.run(&handler, telemetry);
let run_fut = run.run(&handler, telemetry, api_client);
let handler_fut = handler.done();
tokio::select! {
biased;
Expand All @@ -46,18 +47,7 @@ pub async fn run(base: CommandBase, telemetry: CommandEventBuilder) -> Result<i3
result = run_fut => {
// Run finished so close the signal handler
handler.close().await;
match result {
Ok(code) => {
if code != 0 {
error!("run failed: command exited ({code})")
}
Ok(code)
},
Err(err) => {
error!("run failed: {}", err);
Err(err)
}
}
result
},
}
}
48 changes: 24 additions & 24 deletions crates/turborepo-lib/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ pub enum Error {
}

#[derive(Debug)]
pub struct Opts<'a> {
pub cache_opts: CacheOpts<'a>,
pub run_opts: RunOpts<'a>,
pub struct Opts {
pub cache_opts: CacheOpts,
pub run_opts: RunOpts,
pub runcache_opts: RunCacheOpts,
pub scope_opts: ScopeOpts,
}

impl<'a> Opts<'a> {
impl Opts {
pub fn synthesize_command(&self) -> String {
let mut cmd = format!("turbo run {}", self.run_opts.tasks.join(" "));
for pattern in &self.scope_opts.filter_patterns {
Expand Down Expand Up @@ -79,7 +79,7 @@ impl<'a> Opts<'a> {
}
}

impl<'a> TryFrom<&'a Args> for Opts<'a> {
impl<'a> TryFrom<&'a Args> for Opts {
type Error = self::Error;

fn try_from(args: &'a Args) -> Result<Self, Self::Error> {
Expand Down Expand Up @@ -118,19 +118,19 @@ impl<'a> From<&'a RunArgs> for RunCacheOpts {
}

#[derive(Debug)]
pub struct RunOpts<'a> {
pub(crate) tasks: &'a [String],
pub struct RunOpts {
pub(crate) tasks: Vec<String>,
pub(crate) concurrency: u32,
pub(crate) parallel: bool,
pub(crate) env_mode: EnvMode,
// Whether or not to infer the framework for each workspace.
pub(crate) framework_inference: bool,
pub profile: Option<&'a str>,
pub profile: Option<String>,
pub(crate) continue_on_error: bool,
pub(crate) pass_through_args: &'a [String],
pub(crate) pass_through_args: Vec<String>,
pub(crate) only: bool,
pub(crate) dry_run: Option<DryRunMode>,
pub graph: Option<GraphOpts<'a>>,
pub graph: Option<GraphOpts>,
pub(crate) daemon: Option<bool>,
pub(crate) single_package: bool,
pub log_prefix: ResolvedLogPrefix,
Expand All @@ -140,25 +140,25 @@ pub struct RunOpts<'a> {
pub is_github_actions: bool,
}

impl<'a> RunOpts<'a> {
impl RunOpts {
pub fn args_for_task(&self, task_id: &TaskId) -> Option<Vec<String>> {
if !self.pass_through_args.is_empty()
&& self
.tasks
.iter()
.any(|task| task.as_str() == task_id.task())
{
Some(Vec::from(self.pass_through_args))
Some(self.pass_through_args.clone())
} else {
None
}
}
}

#[derive(Debug)]
pub enum GraphOpts<'a> {
pub enum GraphOpts {
Stdout,
File(&'a str),
File(String),
}

#[derive(Debug, Clone, Copy)]
Expand All @@ -175,7 +175,7 @@ pub enum ResolvedLogPrefix {

const DEFAULT_CONCURRENCY: u32 = 10;

impl<'a> TryFrom<&'a RunArgs> for RunOpts<'a> {
impl<'a> TryFrom<&'a RunArgs> for RunOpts {
type Error = self::Error;

fn try_from(args: &'a RunArgs) -> Result<Self, Self::Error> {
Expand All @@ -188,7 +188,7 @@ impl<'a> TryFrom<&'a RunArgs> for RunOpts<'a> {

let graph = args.graph.as_deref().map(|file| match file {
"" => GraphOpts::Stdout,
f => GraphOpts::File(f),
f => GraphOpts::File(f.to_string()),
});

let (is_github_actions, log_order, log_prefix) = match args.log_order {
Expand All @@ -209,7 +209,7 @@ impl<'a> TryFrom<&'a RunArgs> for RunOpts<'a> {
};

Ok(Self {
tasks: args.tasks.as_slice(),
tasks: args.tasks.clone(),
log_prefix,
log_order,
summarize: args.summarize,
Expand All @@ -218,9 +218,9 @@ impl<'a> TryFrom<&'a RunArgs> for RunOpts<'a> {
env_mode: args.env_mode,
concurrency,
parallel: args.parallel,
profile: args.profile.as_deref(),
profile: args.profile.clone(),
continue_on_error: args.continue_execution,
pass_through_args: args.pass_through_args.as_ref(),
pass_through_args: args.pass_through_args.clone(),
only: args.only,
daemon: args.daemon(),
single_package: args.single_package,
Expand Down Expand Up @@ -341,10 +341,10 @@ impl<'a> TryFrom<&'a RunArgs> for ScopeOpts {
}
}

impl<'a> From<&'a RunArgs> for CacheOpts<'a> {
impl<'a> From<&'a RunArgs> for CacheOpts {
fn from(run_args: &'a RunArgs) -> Self {
CacheOpts {
override_dir: run_args.cache_dir.as_deref(),
override_dir: run_args.cache_dir.clone(),
skip_filesystem: run_args.remote_only,
remote_cache_read_only: run_args.remote_cache_read_only,
workers: run_args.cache_workers,
Expand All @@ -353,7 +353,7 @@ impl<'a> From<&'a RunArgs> for CacheOpts<'a> {
}
}

impl<'a> RunOpts<'a> {
impl RunOpts {
pub fn should_redirect_stderr_to_stdout(&self) -> bool {
// If we're running on Github Actions, force everything to stdout
// so as not to have out-of-order log lines
Expand Down Expand Up @@ -514,14 +514,14 @@ mod test {
)]
fn test_synthesize_command(opts_input: TestCaseOpts, expected: &str) {
let run_opts = RunOpts {
tasks: &opts_input.tasks,
tasks: opts_input.tasks,
concurrency: 10,
parallel: opts_input.parallel,
env_mode: crate::cli::EnvMode::Loose,
framework_inference: true,
profile: None,
continue_on_error: opts_input.continue_on_error,
pass_through_args: &opts_input.pass_through_args,
pass_through_args: opts_input.pass_through_args,
only: opts_input.only,
dry_run: opts_input.dry_run,
graph: None,
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/run/graph_visualizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub enum Error {

pub(crate) fn write_graph(
ui: UI,
graph_opts: GraphOpts<'_>,
graph_opts: &GraphOpts,
engine: &Engine,
single_package: bool,
cwd: &AbsoluteSystemPath,
Expand Down
Loading

1 comment on commit 59350bf

@vercel
Copy link

@vercel vercel bot commented on 59350bf Jan 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.