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

refactor(turborepo): change the ui config to use an enum #8919

Merged
merged 5 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/turborepo-lib/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
run::watch::WatchClient,
shim::TurboState,
tracing::TurboSubscriber,
turbo_json::UI as ConfigUI,
turbo_json::UIMode,
};

mod error;
Expand Down Expand Up @@ -185,7 +185,7 @@ pub struct Args {
pub heap: Option<String>,
/// Specify whether to use the streaming UI or TUI
#[clap(long, global = true, value_enum)]
pub ui: Option<ConfigUI>,
pub ui: Option<UIMode>,
/// Override the login endpoint
#[clap(long, global = true, value_parser)]
pub login: Option<String>,
Expand Down
7 changes: 2 additions & 5 deletions crates/turborepo-lib/src/commands/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ use turborepo_repository::{
package_graph::PackageGraph, package_json::PackageJson, package_manager::PackageManager,
};

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

#[derive(Debug, Serialize)]
#[serde(rename_all = "camelCase")]
Expand All @@ -21,7 +18,7 @@ struct ConfigOutput<'a> {
upload_timeout: u64,
enabled: bool,
spaces_id: Option<&'a str>,
ui: bool,
ui: UIMode,
package_manager: PackageManager,
daemon: Option<bool>,
env_mode: EnvMode,
Expand Down
5 changes: 3 additions & 2 deletions crates/turborepo-lib/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use turborepo_ui::UI;
use crate::{
cli::Command,
config::{ConfigurationOptions, Error as ConfigError, TurborepoConfigBuilder},
turbo_json::UIMode,
Args,
};

Expand Down Expand Up @@ -70,10 +71,10 @@ impl CommandBase {
.with_token(self.args.token.clone())
.with_timeout(self.args.remote_cache_timeout)
.with_preflight(self.args.preflight.then_some(true))
.with_ui(self.args.ui.map(|ui| ui.use_tui()).or_else(|| {
.with_ui(self.args.ui.or_else(|| {
self.args.execution_args.as_ref().and_then(|args| {
if !args.log_order.compatible_with_tui() {
Some(false)
Some(UIMode::Stream)
} else {
// If the argument is compatible with the TUI this does not mean we should
// override other configs
Expand Down
26 changes: 16 additions & 10 deletions crates/turborepo-lib/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use turborepo_auth::{TURBO_TOKEN_DIR, TURBO_TOKEN_FILE, VERCEL_TOKEN_DIR, VERCEL
use turborepo_dirs::{config_dir, vercel_config_dir};
use turborepo_errors::TURBO_SITE;

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

#[derive(Debug, Error, Diagnostic)]
Expand Down Expand Up @@ -207,7 +207,7 @@ pub struct ConfigurationOptions {
pub(crate) enabled: Option<bool>,
pub(crate) spaces_id: Option<String>,
#[serde(rename = "ui")]
pub(crate) ui: Option<bool>,
pub(crate) ui: Option<UIMode>,
#[serde(rename = "dangerouslyDisablePackageManagerCheck")]
pub(crate) allow_no_package_manager: Option<bool>,
pub(crate) daemon: Option<bool>,
Expand Down Expand Up @@ -276,8 +276,12 @@ impl ConfigurationOptions {
self.spaces_id.as_deref()
}

pub fn ui(&self) -> bool {
self.ui.unwrap_or(false) && atty::is(atty::Stream::Stdout)
pub fn ui(&self) -> UIMode {
if atty::is(atty::Stream::Stdout) {
return UIMode::Stream;
}

self.ui.unwrap_or(UIMode::Stream)
}

pub fn allow_no_package_manager(&self) -> bool {
Expand Down Expand Up @@ -323,7 +327,7 @@ impl ResolvedConfigurationOptions for RawTurboJson {
.experimental_spaces
.and_then(|spaces| spaces.id)
.map(|spaces_id| spaces_id.into());
opts.ui = self.ui.map(|ui| ui.use_tui());
opts.ui = self.ui;
opts.allow_no_package_manager = self.allow_no_package_manager;
opts.daemon = self.daemon.map(|daemon| *daemon.as_inner());
opts.env_mode = self.env_mode;
Expand Down Expand Up @@ -449,7 +453,8 @@ fn get_env_var_config(
let ui = output_map
.get("ui")
.map(|s| s.as_str())
.and_then(truth_env_var);
.and_then(truth_env_var)
.map(|ui| if ui { UIMode::Tui } else { UIMode::Stream });

let allow_no_package_manager = output_map
.get("allow_no_package_manager")
Expand Down Expand Up @@ -535,7 +540,7 @@ fn get_override_env_var_config(
.and_then(|value| {
// If either of these are truthy, then we disable the TUI
if value == "true" || value == "1" {
Some(false)
Some(UIMode::Stream)
} else {
None
}
Expand Down Expand Up @@ -692,7 +697,7 @@ impl TurborepoConfigBuilder {
create_builder!(with_enabled, enabled, Option<bool>);
create_builder!(with_preflight, preflight, Option<bool>);
create_builder!(with_timeout, timeout, Option<u64>);
create_builder!(with_ui, ui, Option<bool>);
create_builder!(with_ui, ui, Option<UIMode>);
create_builder!(
with_allow_no_package_manager,
allow_no_package_manager,
Expand Down Expand Up @@ -809,6 +814,7 @@ mod test {
get_env_var_config, get_override_env_var_config, ConfigurationOptions,
TurborepoConfigBuilder, DEFAULT_API_URL, DEFAULT_LOGIN_URL, DEFAULT_TIMEOUT,
},
turbo_json::UIMode,
};

#[test]
Expand Down Expand Up @@ -864,7 +870,7 @@ mod test {
assert_eq!(turbo_teamid, config.team_id.unwrap());
assert_eq!(turbo_token, config.token.unwrap());
assert_eq!(turbo_remote_cache_timeout, config.timeout.unwrap());
assert_eq!(Some(true), config.ui);
assert_eq!(Some(UIMode::Tui), config.ui);
assert_eq!(Some(true), config.allow_no_package_manager);
assert_eq!(Some(true), config.daemon);
assert_eq!(Some(EnvMode::Strict), config.env_mode);
Expand Down Expand Up @@ -915,7 +921,7 @@ mod test {
let config = get_override_env_var_config(&env).unwrap();
assert_eq!(vercel_artifacts_token, config.token.unwrap());
assert_eq!(vercel_artifacts_owner, config.team_id.unwrap());
assert_eq!(Some(false), config.ui);
assert_eq!(Some(UIMode::Stream), config.ui);
}

#[test]
Expand Down
22 changes: 13 additions & 9 deletions crates/turborepo-lib/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use thiserror::Error;
use turborepo_errors::Spanned;
use turborepo_repository::package_graph::{PackageGraph, PackageName};

use crate::{run::task_id::TaskId, task_graph::TaskDefinition};
use crate::{run::task_id::TaskId, task_graph::TaskDefinition, turbo_json::UIMode};

#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum TaskNode {
Expand Down Expand Up @@ -383,7 +383,7 @@ impl Engine<Built> {
&self,
package_graph: &PackageGraph,
concurrency: u32,
experimental_ui: bool,
ui_mode: UIMode,
) -> Result<(), Vec<ValidateError>> {
// TODO(olszewski) once this is hooked up to a real run, we should
// see if using rayon to parallelize would provide a speedup
Expand Down Expand Up @@ -480,7 +480,7 @@ impl Engine<Built> {
})
}

validation_errors.extend(self.validate_interactive(experimental_ui));
validation_errors.extend(self.validate_interactive(ui_mode));

match validation_errors.is_empty() {
true => Ok(()),
Expand All @@ -489,10 +489,10 @@ impl Engine<Built> {
}

// Validates that UI is setup if any interactive tasks will be executed
fn validate_interactive(&self, experimental_ui: bool) -> Vec<ValidateError> {
fn validate_interactive(&self, ui_mode: UIMode) -> Vec<ValidateError> {
// If experimental_ui is being used, then we don't need check for interactive
// tasks
if experimental_ui {
if matches!(ui_mode, UIMode::Tui) {
return Vec::new();
}
self.task_definitions
Expand Down Expand Up @@ -666,17 +666,21 @@ mod test {
let graph = graph_builder.build().await.unwrap();

// if our limit is less than, it should fail
engine.validate(&graph, 1, false).expect_err("not enough");
engine
.validate(&graph, 1, UIMode::Stream)
.expect_err("not enough");

// if our limit is less than, it should fail
engine.validate(&graph, 2, false).expect_err("not enough");
engine
.validate(&graph, 2, UIMode::Stream)
.expect_err("not enough");

// we have two persistent tasks, and a slot for all other tasks, so this should
// pass
engine.validate(&graph, 3, false).expect("ok");
engine.validate(&graph, 3, UIMode::Stream).expect("ok");

// if our limit is greater, then it should pass
engine.validate(&graph, 4, false).expect("ok");
engine.validate(&graph, 4, UIMode::Stream).expect("ok");
}

#[tokio::test]
Expand Down
18 changes: 7 additions & 11 deletions crates/turborepo-lib/src/run/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::{
run::{scope, task_access::TaskAccess, task_id::TaskName, Error, Run, RunCache},
shim::TurboState,
signal::{SignalHandler, SignalSubscriber},
turbo_json::TurboJson,
turbo_json::{TurboJson, UIMode},
DaemonConnector,
};

Expand All @@ -56,7 +56,7 @@ pub struct RunBuilder {
repo_root: AbsoluteSystemPathBuf,
ui: UI,
version: &'static str,
experimental_ui: bool,
ui_mode: UIMode,
api_client: APIClient,
analytics_sender: Option<AnalyticsSender>,
// In watch mode, we can have a changed package that we want to serve as an entrypoint.
Expand All @@ -77,13 +77,13 @@ impl RunBuilder {
let allow_missing_package_manager = config.allow_no_package_manager();

let version = base.version();
let experimental_ui = config.ui();
let ui_mode = config.ui();
let processes = ProcessManager::new(
// We currently only use a pty if the following are met:
// - we're attached to a tty
atty::is(atty::Stream::Stdout) &&
// - if we're on windows, we're using the UI
(!cfg!(windows) || experimental_ui),
(!cfg!(windows) || matches!(ui_mode, UIMode::Tui)),
);
let CommandBase { repo_root, ui, .. } = base;

Expand All @@ -94,7 +94,7 @@ impl RunBuilder {
repo_root,
ui,
version,
experimental_ui,
ui_mode,
api_auth,
analytics_sender: None,
entrypoint_packages: None,
Expand Down Expand Up @@ -384,7 +384,7 @@ impl RunBuilder {
Ok(Run {
version: self.version,
ui: self.ui,
experimental_ui: self.experimental_ui,
ui_mode: self.ui_mode,
start_at,
processes: self.processes,
run_telemetry,
Expand Down Expand Up @@ -439,11 +439,7 @@ impl RunBuilder {

if !self.opts.run_opts.parallel {
engine
.validate(
pkg_dep_graph,
self.opts.run_opts.concurrency,
self.experimental_ui,
)
.validate(pkg_dep_graph, self.opts.run_opts.concurrency, self.ui_mode)
.map_err(Error::EngineValidation)?;
}

Expand Down
10 changes: 5 additions & 5 deletions crates/turborepo-lib/src/run/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use crate::{
signal::SignalHandler,
task_graph::Visitor,
task_hash::{get_external_deps_hash, get_internal_deps_hash, PackageInputsHashes},
turbo_json::TurboJson,
turbo_json::{TurboJson, UIMode},
DaemonClient, DaemonConnector,
};

Expand All @@ -69,7 +69,7 @@ pub struct Run {
task_access: TaskAccess,
daemon: Option<DaemonClient<DaemonConnector>>,
should_print_prelude: bool,
experimental_ui: bool,
ui_mode: UIMode,
}

impl Run {
Expand Down Expand Up @@ -172,12 +172,12 @@ impl Run {
self.ui
}

pub fn has_experimental_ui(&self) -> bool {
self.experimental_ui
pub fn has_tui(&self) -> bool {
self.ui_mode.use_tui()
}

pub fn should_start_ui(&self) -> Result<bool, Error> {
Ok(self.experimental_ui
Ok(self.ui_mode.use_tui()
&& self.opts.run_opts.dry_run.is_none()
&& tui::terminal_big_enough()?)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/run/watch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl WatchClient {

let mut events = client.package_changes().await?;

if !self.run.has_experimental_ui() {
if !self.run.has_tui() {
self.run.print_run_prelude();
}

Expand Down
20 changes: 10 additions & 10 deletions crates/turborepo-lib/src/turbo_json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub struct RawTurboJson {
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) remote_cache: Option<RawRemoteCacheOptions>,
#[serde(skip_serializing_if = "Option::is_none", rename = "ui")]
pub ui: Option<UI>,
pub ui: Option<UIMode>,
#[serde(
skip_serializing_if = "Option::is_none",
rename = "dangerouslyDisablePackageManagerCheck"
Expand Down Expand Up @@ -169,22 +169,22 @@ impl DerefMut for Pipeline {
}
}

#[derive(Serialize, Debug, Copy, Clone, Deserializable, PartialEq, Eq, ValueEnum)]
#[derive(Serialize, Deserialize, Debug, Copy, Clone, Deserializable, PartialEq, Eq, ValueEnum)]
#[serde(rename_all = "camelCase")]
pub enum UI {
/// Use the TUI interface
pub enum UIMode {
/// Use the terminal user interface
Tui,
/// Use the standard output stream
Stream,
}

impl Default for UI {
impl Default for UIMode {
fn default() -> Self {
Self::Tui
}
}

impl UI {
impl UIMode {
pub fn use_tui(&self) -> bool {
matches!(self, Self::Tui)
}
Expand Down Expand Up @@ -749,7 +749,7 @@ mod tests {
use turborepo_repository::package_json::PackageJson;
use turborepo_unescape::UnescapedString;

use super::{Pipeline, RawTurboJson, Spanned, UI};
use super::{Pipeline, RawTurboJson, Spanned, UIMode};
use crate::{
cli::OutputLogsMode,
run::task_id::TaskName,
Expand Down Expand Up @@ -1079,10 +1079,10 @@ mod tests {
assert_eq!(actual, expected);
}

#[test_case(r#"{ "ui": "tui" }"#, Some(UI::Tui) ; "tui")]
#[test_case(r#"{ "ui": "stream" }"#, Some(UI::Stream) ; "stream")]
#[test_case(r#"{ "ui": "tui" }"#, Some(UIMode::Tui) ; "tui")]
#[test_case(r#"{ "ui": "stream" }"#, Some(UIMode::Stream) ; "stream")]
#[test_case(r#"{}"#, None ; "missing")]
fn test_ui(json: &str, expected: Option<UI>) {
fn test_ui(json: &str, expected: Option<UIMode>) {
let json = RawTurboJson::parse(json, AnchoredSystemPath::new("").unwrap()).unwrap();
assert_eq!(json.ui, expected);
}
Expand Down
2 changes: 1 addition & 1 deletion turborepo-tests/integration/tests/config.t
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Run test run
"uploadTimeout": 60,
"enabled": true,
"spacesId": null,
"ui": false,
"ui": "stream",
"packageManager": "npm",
"daemon": null,
"envMode": "strict"
Expand Down
Loading
Loading