From e7f752cd0af4cd4357e339db6663a3599d9982ed Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 22 Sep 2023 10:29:56 -0700 Subject: [PATCH 1/3] remove quiet field from package graph builder --- .../src/package_graph/builder.rs | 30 ++++--------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/crates/turborepo-lib/src/package_graph/builder.rs b/crates/turborepo-lib/src/package_graph/builder.rs index f9011192f0fda..e2f0a9e85fede 100644 --- a/crates/turborepo-lib/src/package_graph/builder.rs +++ b/crates/turborepo-lib/src/package_graph/builder.rs @@ -27,7 +27,6 @@ pub struct PackageGraphBuilder<'a> { package_manager: Option, package_jsons: Option>, lockfile: Option>, - quiet: bool, } #[derive(Debug, thiserror::Error)] @@ -67,7 +66,6 @@ impl<'a> PackageGraphBuilder<'a> { package_manager: None, package_jsons: None, lockfile: None, - quiet: false, } } @@ -82,11 +80,6 @@ impl<'a> PackageGraphBuilder<'a> { self } - pub fn with_quiet(mut self) -> Self { - self.quiet = true; - self - } - #[allow(dead_code)] pub fn with_package_jsons( mut self, @@ -125,7 +118,6 @@ struct BuildState<'a, S> { node_lookup: HashMap, lockfile: Option>, package_jsons: Option>, - quiet: bool, state: std::marker::PhantomData, } @@ -164,7 +156,6 @@ impl<'a> BuildState<'a, ResolvedPackageManager> { package_manager, package_jsons, lockfile, - quiet, } = builder; let package_manager = package_manager.map_or_else( || PackageManager::get_package_manager(repo_root, Some(&root_package_json)), @@ -187,7 +178,6 @@ impl<'a> BuildState<'a, ResolvedPackageManager> { workspaces, lockfile, package_jsons, - quiet, workspace_graph: Graph::new(), node_lookup: HashMap::new(), state: std::marker::PhantomData, @@ -258,7 +248,6 @@ impl<'a> BuildState<'a, ResolvedPackageManager> { workspace_graph, node_lookup, lockfile, - quiet, .. } = self; Ok(BuildState { @@ -269,7 +258,6 @@ impl<'a> BuildState<'a, ResolvedPackageManager> { workspace_graph, node_lookup, lockfile, - quiet, package_jsons: None, state: std::marker::PhantomData, }) @@ -369,13 +357,11 @@ impl<'a> BuildState<'a, ResolvedWorkspaces> { let lockfile = match self.populate_lockfile() { Ok(lockfile) => Some(lockfile), Err(e) => { - if !self.quiet { - warn!( - "Issues occurred when constructing package graph. Turbo will function, \ - but some features may not be available: {}", - e - ); - } + warn!( + "Issues occurred when constructing package graph. Turbo will function, but \ + some features may not be available: {}", + e + ); None } }; @@ -387,7 +373,6 @@ impl<'a> BuildState<'a, ResolvedWorkspaces> { workspaces, workspace_graph, node_lookup, - quiet, .. } = self; Ok(BuildState { @@ -398,7 +383,6 @@ impl<'a> BuildState<'a, ResolvedWorkspaces> { workspace_graph, node_lookup, lockfile, - quiet, package_jsons: None, state: std::marker::PhantomData, }) @@ -447,9 +431,7 @@ impl<'a> BuildState<'a, ResolvedLockfile> { fn build(mut self) -> PackageGraph { if let Err(e) = self.populate_transitive_dependencies() { - if !self.quiet { - warn!("Unable to calculate transitive closures: {}", e); - } + warn!("Unable to calculate transitive closures: {}", e); } let Self { package_manager, From 45312be8a23aff639e8e1f38cbe0cb55d12ec345 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 22 Sep 2023 10:56:27 -0700 Subject: [PATCH 2/3] add sink as option for ui output --- crates/turborepo-lib/src/opts.rs | 8 ++++ crates/turborepo-lib/src/run/mod.rs | 1 + .../turborepo-lib/src/task_graph/visitor.rs | 44 +++++++++---------- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/crates/turborepo-lib/src/opts.rs b/crates/turborepo-lib/src/opts.rs index 93a070700a7e9..ba40690d52a35 100644 --- a/crates/turborepo-lib/src/opts.rs +++ b/crates/turborepo-lib/src/opts.rs @@ -273,6 +273,14 @@ impl<'a> From<&'a RunArgs> for CacheOpts<'a> { } } +impl<'a> RunOpts<'a> { + 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 + matches!(self.log_order, ResolvedLogOrder::Grouped) && self.is_github_actions + } +} + impl ScopeOpts { pub fn get_filters(&self) -> Vec { [ diff --git a/crates/turborepo-lib/src/run/mod.rs b/crates/turborepo-lib/src/run/mod.rs index 75aa7e8bbba08..4b7e1d8efc38e 100644 --- a/crates/turborepo-lib/src/run/mod.rs +++ b/crates/turborepo-lib/src/run/mod.rs @@ -265,6 +265,7 @@ impl<'a> Run<'a> { &global_hash, global_env_mode, self.base.ui, + false, ); visitor.visit(engine.clone()).await?; diff --git a/crates/turborepo-lib/src/task_graph/visitor.rs b/crates/turborepo-lib/src/task_graph/visitor.rs index 869a1e66ec30c..edff968300d93 100644 --- a/crates/turborepo-lib/src/task_graph/visitor.rs +++ b/crates/turborepo-lib/src/task_graph/visitor.rs @@ -72,6 +72,7 @@ impl<'a> Visitor<'a> { global_hash: &'a str, global_env_mode: EnvMode, ui: UI, + silent: bool, ) -> Self { let task_hasher = TaskHasher::new( package_inputs_hashes, @@ -79,7 +80,7 @@ impl<'a> Visitor<'a> { env_at_execution_start, global_hash, ); - let sink = Self::sink(opts); + let sink = Self::sink(opts, silent); let color_cache = ColorSelector::default(); Self { @@ -95,11 +96,6 @@ impl<'a> Visitor<'a> { } } - pub fn silent(mut self) -> Self { - self.silent = true; - self - } - pub async fn visit(&self, engine: Arc) -> Result<(), Error> { let concurrency = self.opts.run_opts.concurrency as usize; let (node_sender, mut node_stream) = mpsc::channel(concurrency); @@ -191,15 +187,12 @@ impl<'a> Visitor<'a> { let prefix = self.prefix(&info); let pretty_prefix = self.color_cache.prefix_with_color(&task_hash, &prefix); let ui = self.ui; - let silent = self.silent; tasks.push(tokio::spawn(async move { let _task_cache = task_cache; - if !silent { - let mut prefixed_ui = - Self::prefixed_ui(ui, is_github_actions, &output_client, pretty_prefix); - prefixed_ui.output(command); - } + let mut prefixed_ui = + Self::prefixed_ui(ui, is_github_actions, &output_client, pretty_prefix); + prefixed_ui.output(command); callback.send(Ok(())).unwrap(); })); } @@ -214,18 +207,15 @@ impl<'a> Visitor<'a> { Ok(()) } - fn sink(opts: &Opts) -> OutputSink { - let err_writer = match matches!( - opts.run_opts.log_order, - crate::opts::ResolvedLogOrder::Grouped - ) && opts.run_opts.is_github_actions - { - // If we're running on Github Actions, force everything to stdout - // so as not to have out-of-order log lines - true => std::io::stdout().into(), - false => std::io::stderr().into(), + fn sink(opts: &Opts, silent: bool) -> OutputSink { + let (out, err) = if silent { + (std::io::sink().into(), std::io::sink().into()) + } else if opts.run_opts.should_redirect_stderr_to_stdout() { + (std::io::stdout().into(), std::io::stdout().into()) + } else { + (std::io::stdout().into(), std::io::stderr().into()) }; - OutputSink::new(std::io::stdout().into(), err_writer) + OutputSink::new(out, err) } fn output_client(&self) -> OutputClient { @@ -283,6 +273,7 @@ impl<'a> Visitor<'a> { enum StdWriter { Out(std::io::Stdout), Err(std::io::Stderr), + Null(std::io::Sink), } impl StdWriter { @@ -290,6 +281,7 @@ impl StdWriter { match self { StdWriter::Out(out) => out, StdWriter::Err(err) => err, + StdWriter::Null(null) => null, } } } @@ -306,6 +298,12 @@ impl From for StdWriter { } } +impl From for StdWriter { + fn from(value: std::io::Sink) -> Self { + Self::Null(value) + } +} + impl std::io::Write for StdWriter { fn write(&mut self, buf: &[u8]) -> std::io::Result { self.writer().write(buf) From 275ae202ab3dd4fa7fdf8340bf84211a17925894 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 22 Sep 2023 11:59:08 -0700 Subject: [PATCH 3/3] remove unused silent field --- crates/turborepo-lib/src/task_graph/visitor.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/turborepo-lib/src/task_graph/visitor.rs b/crates/turborepo-lib/src/task_graph/visitor.rs index edff968300d93..700ee6219fea0 100644 --- a/crates/turborepo-lib/src/task_graph/visitor.rs +++ b/crates/turborepo-lib/src/task_graph/visitor.rs @@ -35,8 +35,6 @@ pub struct Visitor<'a> { sink: OutputSink, color_cache: ColorSelector, ui: UI, - // For testing purposes we need to be able to silence the output - silent: bool, } #[derive(Debug, thiserror::Error)] @@ -92,7 +90,6 @@ impl<'a> Visitor<'a> { sink, color_cache, ui, - silent: false, } }