[RR] Verbose print commands when --verbose is set

crates/moon/src/cli/build.rs

@@ -185,7 +185,7 @@ fn run_build_rr(
         let _lock = FileLock::lock(target_dir)?;
 
         let result = rr_build::execute_build(
-            &BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature),
+            &BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature, cli.verbose),
             build_graph,
             target_dir,
         )?;

crates/moon/src/cli/bundle.rs

@@ -157,7 +157,7 @@ pub fn run_bundle_internal_rr(
         rr_build::generate_metadata(source_dir, target_dir, &_build_meta, RunMode::Bundle)?;
 
         let result = rr_build::execute_build(
-            &BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature),
+            &BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature, cli.verbose),
             build_graph,
             target_dir,
         )?;

crates/moon/src/cli/check.rs

@@ -198,7 +198,7 @@ fn run_check_for_single_file_rr(
     let _lock = FileLock::lock(&raw_target_dir)?;
     rr_build::generate_metadata(&source_dir, &raw_target_dir, &build_meta, RunMode::Check)?;
 
-    let mut cfg = BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature);
+    let mut cfg = BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature, cli.verbose);
     cfg.patch_file = cmd.patch_file.clone();
     cfg.explain_errors |= cmd.explain;
 
@@ -414,7 +414,7 @@ fn run_check_normal_internal_rr(
         // Generate metadata for IDE
         rr_build::generate_metadata(source_dir, target_dir, &build_meta, RunMode::Check)?;
 
-        let mut cfg = BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature);
+        let mut cfg = BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature, cli.verbose);
         cfg.patch_file = cmd.patch_file.clone();
         cfg.explain_errors |= cmd.explain;
         let result = rr_build::execute_build(&cfg, build_graph, target_dir)?;

crates/moon/src/cli/doc.rs

@@ -130,7 +130,7 @@ pub fn run_doc_rr(cli: UniversalFlags, cmd: DocSubcommand) -> anyhow::Result<i32
     rr_build::generate_metadata(&source_dir, &target_dir, &_build_meta, RunMode::Check)?;
 
     // Execute the build
-    let cfg = BuildConfig::from_flags(&BuildFlags::default(), &cli.unstable_feature);
+    let cfg = BuildConfig::from_flags(&BuildFlags::default(), &cli.unstable_feature, cli.verbose);
     let result = rr_build::execute_build(&cfg, build_graph, &target_dir)?;
     result.print_info(cli.quiet, "checking")?;
 

crates/moon/src/cli/run.rs

@@ -700,23 +700,23 @@ fn rr_run_from_plan(
         );
 
         let run_cmd = get_run_cmd(build_meta, &cmd.args)?;
-        rr_build::dry_print_command(run_cmd.command.as_std(), source_dir);
+        rr_build::dry_print_command(run_cmd.command.as_std(), source_dir, false);
         return Ok(0);
     }
 
     let _lock = FileLock::lock(target_dir)?;
 
-    let build_result = rr_build::execute_build(
-        &BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature),
-        build_graph,
-        target_dir,
-    )?;
+    let build_config =
+        BuildConfig::from_flags(&cmd.build_flags, &cli.unstable_feature, cli.verbose);
+    let build_result = rr_build::execute_build(&build_config, build_graph, target_dir)?;
 
     if !build_result.successful() {
         return Ok(build_result.return_code_for_success());
     }
-
     let run_cmd = get_run_cmd(build_meta, &cmd.args)?;
+    if cli.verbose {
+        rr_build::dry_print_command(run_cmd.command.as_std(), source_dir, true);
+    }
 
     // Release the lock before spawning the subprocess
     drop(_lock);

crates/moon/src/cli/test.rs

@@ -1432,7 +1432,7 @@ fn rr_test_from_plan(
 
     let _lock = FileLock::lock(target_dir)?;
 
-    let build_config = BuildConfig::from_flags(cmd.build_flags, &cli.unstable_feature);
+    let build_config = BuildConfig::from_flags(cmd.build_flags, &cli.unstable_feature, cli.verbose);
 
     // since n2 build consumes the graph, we back it up for reruns
     let build_graph_backup = cmd.update.then(|| build_graph.clone());
@@ -1449,10 +1449,12 @@ fn rr_test_from_plan(
 
     let mut test_result = crate::run::run_tests(
         build_meta,
+        source_dir,
         target_dir,
         &filter,
         cmd.include_skipped,
         cmd.run_mode == RunMode::Bench,
+        cli.verbose,
     )?;
     let _initial_summary = test_result.summary();
 
@@ -1537,10 +1539,12 @@ fn rr_test_from_plan(
             };
             let new_test_result = crate::run::run_tests(
                 build_meta,
+                source_dir,
                 target_dir,
                 &rerun_filter,
                 cmd.include_skipped,
                 cmd.run_mode == RunMode::Bench,
+                cli.verbose,
             )?;
             let _rerun_summary = new_test_result.summary();
 

crates/moon/src/rr_build/dry_run.rs

@@ -57,7 +57,9 @@ pub fn print_dry_run_all(input: &BuildInput, source_dir: &Path, target_dir: &Pat
 /// Print a command as it would be executed, with the proper escaping.
 ///
 /// This also replaces paths like `print_dry_run` does.
-pub fn dry_print_command(cmd: &Command, source_dir: &Path) {
+///
+/// If `stderr` is true, the command is assumed to write to stderr instead of stdout.
+pub fn dry_print_command(cmd: &Command, source_dir: &Path, stderr: bool) {
     let replacer = moonbuild_debug::graph::PathNormalizer::new(source_dir);
 
     let args = std::iter::once(cmd.get_program())
@@ -67,5 +69,9 @@ pub fn dry_print_command(cmd: &Command, source_dir: &Path) {
         .collect::<Vec<_>>();
 
     let cmd = moonutil::shlex::join_unix(args.iter().map(|x| x.as_ref()));
-    println!("{}", cmd);
+    if stderr {
+        eprintln!("{}", cmd);
+    } else {
+        println!("{}", cmd);
+    }
 }

crates/moon/src/rr_build/mod.rs

@@ -621,19 +621,23 @@ pub struct BuildConfig {
     /// Ask n2 to explain rerun reasons
     pub n2_explain: bool,
 
+    /// Verbose output for build progress and command echo
+    verbose: bool,
+
     /// The patch file to use
     pub patch_file: Option<PathBuf>,
 }
 
 impl BuildConfig {
-    pub fn from_flags(flags: &BuildFlags, unstable_features: &FeatureGate) -> Self {
+    pub fn from_flags(flags: &BuildFlags, unstable_features: &FeatureGate, verbose: bool) -> Self {
         BuildConfig {
             parallelism: flags.jobs,
             no_render: flags.output_style().needs_no_render(),
             render_no_loc: flags.render_no_loc,
             generate_metadata: false,
             explain_errors: false,
             n2_explain: unstable_features.rr_n2_explain,
+            verbose,
             patch_file: None,
         }
     }
@@ -648,6 +652,7 @@ impl Default for BuildConfig {
             generate_metadata: false,
             explain_errors: false,
             n2_explain: false,
+            verbose: false,
             patch_file: None,
         }
     }
@@ -746,7 +751,7 @@ pub fn execute_build_partial(
         PathBuf::new(),
         target_dir.into(),
     );
-    let mut prog_console = create_progress_console(Some(Box::new(callback)), false);
+    let mut prog_console = create_progress_console(Some(Box::new(callback)), cfg.verbose);
     let mut work = n2::work::Work::new(
         build_graph,
         hashes,

crates/moon/src/run/runtest.rs

@@ -138,29 +138,33 @@ impl TestIndex {
 #[instrument(level = "debug", skip(build_meta, filter))]
 pub fn run_tests(
     build_meta: &BuildMeta,
+    source_dir: &Path,
     target_dir: &Path,
     filter: &TestFilter,
     include_skipped: bool,
     bench: bool,
+    verbose: bool,
 ) -> anyhow::Result<ReplaceableTestResults> {
     // Gathering artifacts
     let executables = gather_tests(build_meta);
     debug!(count = executables.len(), "collected test executables");
 
     let rt = default_rt().context("Failed to create runtime")?;
+    let ctx = TestRunCtx {
+        build_meta,
+        rt: &rt,
+        source_dir,
+        target_dir,
+        filter,
+        include_skipped,
+        bench,
+        verbose,
+    };
     let mut stats = ReplaceableTestResults::default();
     let mut total_cases = 0usize;
     for r in executables {
         debug!(target = ?r.target, executable = %r.executable.display(), "running test executable");
-        let res = run_one_test_executable(
-            build_meta,
-            &rt,
-            target_dir,
-            &r,
-            filter,
-            include_skipped,
-            bench,
-        )?;
+        let res = run_one_test_executable(&ctx, &r)?;
         let cases_for_target = res.map.values().map(IndexMap::len).sum::<usize>();
         trace!(target = ?r.target, cases = cases_for_target, "merging test results");
         total_cases += cases_for_target;
@@ -185,6 +189,27 @@ struct TestExecutableToRun<'a> {
     meta: &'a Path,
 }
 
+/// Context for running a single compiled test executable. This is for reducing
+/// the number of parameters shifted around.
+struct TestRunCtx<'a> {
+    /// Build outputs and target backend
+    build_meta: &'a BuildMeta,
+    /// Tokio runtime used to execute the test process
+    rt: &'a Runtime,
+    /// Source directory; used for dry-printing commands when verbose
+    source_dir: &'a Path,
+    /// Target directory; coverage output destination
+    target_dir: &'a Path,
+    /// Package/file/index selection
+    filter: &'a TestFilter,
+    /// Include tests marked as skipped
+    include_skipped: bool,
+    /// Include benchmark cases
+    bench: bool,
+    /// Enable verbose printing
+    verbose: bool,
+}
+
 /// A container of test results corresponding to each test artifact, and
 /// can be replaced by later test runs upon test result updates.
 #[derive(Default, Debug)]
@@ -331,23 +356,22 @@ fn gather_tests(build_meta: &BuildMeta) -> Vec<TestExecutableToRun<'_>> {
     results
 }
 
-#[instrument(level = "debug", skip(build_meta, rt, target_dir, filter))]
+#[instrument(level = "debug", skip(ctx, test))]
 fn run_one_test_executable(
-    build_meta: &BuildMeta,
-    rt: &Runtime, // FIXME: parallel execution
-    target_dir: &Path,
+    ctx: &TestRunCtx<'_>,
     test: &TestExecutableToRun,
-    filter: &TestFilter,
-    include_skipped: bool,
-    bench: bool,
 ) -> Result<TargetTestResult, anyhow::Error> {
-    let (included, file_filt) = filter.check_package(test.target);
+    let (included, file_filt) = ctx.filter.check_package(test.target);
     if !included {
         debug!(target = ?test.target, "skipping test executable due to filter");
         return Ok(TargetTestResult::default());
     }
 
-    let fqn = build_meta.resolve_output.pkg_dirs.fqn(test.target.package);
+    let fqn = ctx
+        .build_meta
+        .resolve_output
+        .pkg_dirs
+        .fqn(test.target.package);
     let pkgname = fqn.to_string();
 
     // Parse test metadata
@@ -365,21 +389,28 @@ fn run_one_test_executable(
         file_filt,
         &meta,
         &mut test_args.file_and_index,
-        include_skipped,
-        bench,
+        ctx.include_skipped,
+        ctx.bench,
     );
     trace!(
         filter_entries = test_args.file_and_index.len(),
         "applied test filter"
     );
 
-    let cmd =
-        crate::run::command_for(build_meta.target_backend, test.executable, Some(&test_args))?;
+    let cmd = crate::run::command_for(
+        ctx.build_meta.target_backend,
+        test.executable,
+        Some(&test_args),
+    )?;
     let mut cov_cap = mk_coverage_capture();
     let mut test_cap = make_test_capture();
+    if ctx.verbose {
+        crate::rr_build::dry_print_command(cmd.command.as_std(), ctx.source_dir, true);
+    }
     info!(package = %test_args.package, executable = %test.executable.display(), "launching test executable");
 
-    let exit_status = rt
+    let exit_status = ctx
+        .rt
         .block_on(crate::run::run(
             &mut [&mut cov_cap, &mut test_cap],
             true,
@@ -396,7 +427,7 @@ fn run_one_test_executable(
         );
     }
 
-    handle_finished_coverage(target_dir, cov_cap)?;
+    handle_finished_coverage(ctx.target_dir, cov_cap)?;
 
     parse_test_results(meta, test_cap).with_context(|| {
         format!(