Use Rust nightly for formatting (#2348)

* Support specifying a working dir to a build command

* Use nightly for formatting

* Pass valid TERM in from environment

Rustfmt depends on a valid setting, and not just the var to be non-empty.

* Wrap comment

build/configure/src/rust.rs

@@ -113,6 +113,7 @@ pub fn check_rust(build: &mut Build) -> Result<()> {
         CargoFormat {
             inputs: inputs.clone(),
             check_only: true,
+            working_dir: Some("cargo/format"),
         },
     )?;
     build.add(
@@ -120,6 +121,7 @@ pub fn check_rust(build: &mut Build) -> Result<()> {
         CargoFormat {
             inputs: inputs.clone(),
             check_only: false,
+            working_dir: Some("cargo/format"),
         },
     )?;
 

build/ninja_gen/src/build.rs

@@ -214,6 +214,7 @@ struct BuildStatement<'a> {
     rule_variables: Vec<(String, String)>,
     output_stamp: bool,
     env_vars: Vec<String>,
+    working_dir: Option<String>,
     release: bool,
     bypass_runner: bool,
 }
@@ -237,6 +238,7 @@ impl BuildStatement<'_> {
             output_subsets: Default::default(),
             output_stamp: false,
             env_vars: Default::default(),
+            working_dir: None,
             release,
             bypass_runner: action.bypass_runner(),
         };
@@ -296,6 +298,9 @@ impl BuildStatement<'_> {
                 write!(&mut buf, "--env={var} ").unwrap();
             }
         }
+        if let Some(working_dir) = &self.working_dir {
+            write!(&mut buf, "--cwd={working_dir} ").unwrap();
+        }
         buf.push_str(&command);
         buf
     }
@@ -360,6 +365,11 @@ pub trait FilesHandle {
     /// for each command, `constant_value` should reference a `$variable` you
     /// have defined.
     fn add_env_var(&mut self, key: &str, constant_value: &str);
+    /// Set the current working dir for the provided command(s).
+    /// Note this is defined once for the rule, so if the value should change
+    /// for each command, `constant_value` should reference a `$variable` you
+    /// have defined.
+    fn set_working_dir(&mut self, constant_value: &str);
 
     fn release_build(&self) -> bool;
 }
@@ -448,6 +458,10 @@ impl FilesHandle for BuildStatement<'_> {
     fn add_env_var(&mut self, key: &str, constant_value: &str) {
         self.env_vars.push(format!("{key}={constant_value}"));
     }
+
+    fn set_working_dir(&mut self, constant_value: &str) {
+        self.working_dir = Some(constant_value.to_owned());
+    }
 }
 
 fn to_ninja_target_string(explicit: &[String], implicit: &[String]) -> String {

build/ninja_gen/src/cargo.rs

@@ -173,17 +173,21 @@ impl BuildAction for CargoClippy {
 pub struct CargoFormat {
     pub inputs: BuildInput,
     pub check_only: bool,
+    pub working_dir: Option<&'static str>,
 }
 
 impl BuildAction for CargoFormat {
     fn command(&self) -> &str {
-        // the empty config file prevents warnings about nightly features
+        "cargo fmt $mode --all"
-        "cargo fmt $mode -- --config-path=.rustfmt-empty.toml --color always"
     }
 
     fn files(&mut self, build: &mut impl FilesHandle) {
         build.add_inputs("", &self.inputs);
         build.add_variable("mode", if self.check_only { "--check" } else { "" });
+        if let Some(working_dir) = self.working_dir {
+            build.set_working_dir("$working_dir");
+            build.add_variable("working_dir", working_dir);
+        }
         build.add_output_stamp(format!(
             "tests/cargo_format.{}",
             if self.check_only { "check" } else { "fmt" }

build/runner/src/build.rs

@@ -67,7 +67,7 @@ pub fn run_build(args: BuildArgs) {
         // commands will not show colors by default, as we do not provide a tty
         .env("FORCE_COLOR", "1")
         .env("MYPY_FORCE_COLOR", "1")
-        .env("TERM", "1")
+        .env("TERM", std::env::var("TERM").unwrap_or_default())
         // Prevents 'Warn: You must provide the URL of lib/mappings.wasm'.
         // Updating svelte-check or its deps will likely remove the need for it.
         .env("NODE_OPTIONS", "--no-experimental-fetch");

build/runner/src/run.rs

@@ -13,6 +13,8 @@ pub struct RunArgs {
     stamp: Option<String>,
     #[arg(long, value_parser = split_env)]
     env: Vec<(String, String)>,
+    #[arg(long)]
+    cwd: Option<String>,
     #[arg(trailing_var_arg = true)]
     args: Vec<String>,
 }
@@ -22,7 +24,7 @@ pub struct RunArgs {
 pub fn run_commands(args: RunArgs) {
     let commands = split_args(args.args);
     for command in commands {
-        run_silent(&mut build_command(command, &args.env));
+        run_silent(&mut build_command(command, &args.env, &args.cwd));
     }
     if let Some(stamp_file) = args.stamp {
         std::fs::write(stamp_file, b"").expect("unable to write stamp file");
@@ -37,12 +39,19 @@ fn split_env(s: &str) -> Result<(String, String), std::io::Error> {
     }
 }
 
-fn build_command(command_and_args: Vec<String>, env: &[(String, String)]) -> Command {
+fn build_command(
+    command_and_args: Vec<String>,
+    env: &[(String, String)],
+    cwd: &Option<String>,
+) -> Command {
     let mut command = Command::new(&command_and_args[0]);
     command.args(&command_and_args[1..]);
     for (k, v) in env {
         command.env(k, v);
     }
+    if let Some(cwd) = cwd {
+        command.current_dir(cwd);
+    }
     command
 }
 

cargo/README.md

@@ -1,3 +1,5 @@
 This folder used to contain Bazel-specific Rust integration, but now only
-contains our license-checking script, which should be run when using
+contains:
-cargo update.
+
+- our license-checking script, which should be run when using cargo update.
+- a nightly toolchain definition for formatting

cargo/format/rust-toolchain.toml

@@ -0,0 +1,4 @@
+[toolchain]
+channel = "nightly-2023-01-24"
+profile = "minimal"
+components = ["rustfmt"]

rslib/src/sync/http_client/io_monitor.rs

@@ -201,9 +201,9 @@ mod test {
     use super::*;
     use crate::sync::error::HttpError;
 
-    /// The delays in the tests are aggressively short, and false positives slip through
+    /// The delays in the tests are aggressively short, and false positives slip
-    /// on a loaded system - especially on Windows. Fix by applying a universal
+    /// through on a loaded system - especially on Windows. Fix by applying
-    /// multiplier.
+    /// a universal multiplier.
     fn millis(millis: u64) -> Duration {
         Duration::from_millis(millis * if cfg!(windows) { 10 } else { 5 })
     }