Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(Turborepo): Move more CLI stuff out of run code #7085

Merged
merged 14 commits into from
Jan 25, 2024
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
5 changes: 4 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,10 @@ 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.map_err(|e| {
error!("run failed: {e}");
Copy link
Member

Choose a reason for hiding this comment

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

Will this cause us to stutter our errors?

I see that this logic moved from the run command itself. I don't care where it lives, but I feel it's easier to reason about if both error messages live close to each other instead of being spread across two functions. Maybe use inspect_err in the same way we use inspect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion. Moved to inspect_err.

Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually I want to remove all of this and have a single print out in turborepo-lib/src/lib.rs that prints the miette formatted version. That's in #6956

e
})?;
Ok(Payload::Rust(Ok(exit_code)))
}
}
Expand Down
22 changes: 8 additions & 14 deletions crates/turborepo-lib/src/commands/run.rs
Original file line number Diff line number Diff line change
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,11 @@ 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.inspect(|code| {
if *code != 0 {
error!("run failed: command exited ({code})");
}
}
})
},
}
}
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
Loading