Auto merge of #8165 - mchernyavsky:force-progress, r=ehuss

Add a term option to configure the progress bar

Closes #7587.

Author: bors

crates/cargo-test-support/src/lib.rs

@@ -1645,6 +1645,7 @@ fn substitute_macros(input: &str) -> String {
         ("[IGNORED]", "     Ignored"),
         ("[INSTALLED]", "   Installed"),
         ("[REPLACED]", "    Replaced"),
+        ("[BUILDING]", "    Building"),
     ];
     let mut result = input.to_owned();
     for &(pat, subst) in &macros {

src/cargo/ops/cargo_compile.rs

@@ -291,6 +291,7 @@ pub fn create_bcx<'a, 'cfg>(
     } = *options;
     let config = ws.config();
 
+    // Perform some pre-flight validation.
     match build_config.mode {
         CompileMode::Test
         | CompileMode::Build
@@ -311,6 +312,7 @@ pub fn create_bcx<'a, 'cfg>(
             }
         }
     }
+    config.validate_term_config()?;
 
     let target_data = RustcTargetData::new(ws, &build_config.requested_kinds)?;
 

src/cargo/util/config/de.rs

@@ -40,13 +40,12 @@ macro_rules! deserialize_method {
     };
 }
 
-impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
-    type Error = ConfigError;
-
-    fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
-    where
-        V: de::Visitor<'de>,
-    {
+impl<'config> Deserializer<'config> {
+    /// This is a helper for getting a CV from a file or env var.
+    ///
+    /// If this returns CV::List, then don't look at the value. Handling lists
+    /// is deferred to ConfigSeqAccess.
+    fn get_cv_with_env(&self) -> Result<Option<CV>, ConfigError> {
         // Determine if value comes from env, cli, or file, and merge env if
         // possible.
         let cv = self.config.get_cv(&self.key)?;
@@ -58,36 +57,53 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
             _ => false,
         };
 
-        if use_env {
-            // Future note: If you ever need to deserialize a non-self describing
-            // map type, this should implement a starts_with check (similar to how
-            // ConfigMapAccess does).
-            let env = env.unwrap();
-            let res: Result<V::Value, ConfigError> = if env == "true" || env == "false" {
-                visitor.visit_bool(env.parse().unwrap())
-            } else if let Ok(env) = env.parse::<i64>() {
-                visitor.visit_i64(env)
-            } else if self.config.cli_unstable().advanced_env
-                && env.starts_with('[')
-                && env.ends_with(']')
-            {
-                visitor.visit_seq(ConfigSeqAccess::new(self.clone())?)
-            } else {
-                // Try to merge if possible.
-                match cv {
-                    Some(CV::List(_cv_list, _cv_def)) => {
-                        visitor.visit_seq(ConfigSeqAccess::new(self.clone())?)
-                    }
-                    _ => {
-                        // Note: CV::Table merging is not implemented, as env
-                        // vars do not support table values.
-                        visitor.visit_str(env)
-                    }
+        if !use_env {
+            return Ok(cv);
+        }
+
+        // Future note: If you ever need to deserialize a non-self describing
+        // map type, this should implement a starts_with check (similar to how
+        // ConfigMapAccess does).
+        let env = env.unwrap();
+        if env == "true" {
+            Ok(Some(CV::Boolean(true, env_def)))
+        } else if env == "false" {
+            Ok(Some(CV::Boolean(false, env_def)))
+        } else if let Ok(i) = env.parse::<i64>() {
+            Ok(Some(CV::Integer(i, env_def)))
+        } else if self.config.cli_unstable().advanced_env
+            && env.starts_with('[')
+            && env.ends_with(']')
+        {
+            // Parsing is deferred to ConfigSeqAccess.
+            Ok(Some(CV::List(Vec::new(), env_def)))
+        } else {
+            // Try to merge if possible.
+            match cv {
+                Some(CV::List(cv_list, _cv_def)) => {
+                    // Merging is deferred to ConfigSeqAccess.
+                    Ok(Some(CV::List(cv_list, env_def)))
                 }
-            };
-            return res.map_err(|e| e.with_key_context(&self.key, env_def));
+                _ => {
+                    // Note: CV::Table merging is not implemented, as env
+                    // vars do not support table values. In the future, we
+                    // could check for `{}`, and interpret it as TOML if
+                    // that seems useful.
+                    Ok(Some(CV::String(env.to_string(), env_def)))
+                }
+            }
         }
+    }
+}
 
+impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
+    type Error = ConfigError;
+
+    fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
+    where
+        V: de::Visitor<'de>,
+    {
+        let cv = self.get_cv_with_env()?;
         if let Some(cv) = cv {
             let res: (Result<V::Value, ConfigError>, Definition) = match cv {
                 CV::Integer(i, def) => (visitor.visit_i64(i), def),

src/cargo/util/config/mod.rs

@@ -176,6 +176,7 @@ pub struct Config {
     build_config: LazyCell<CargoBuildConfig>,
     target_cfgs: LazyCell<Vec<(String, TargetCfgConfig)>>,
     doc_extern_map: LazyCell<RustdocExternMap>,
+    progress_config: ProgressConfig,
 }
 
 impl Config {
@@ -247,6 +248,7 @@ impl Config {
             build_config: LazyCell::new(),
             target_cfgs: LazyCell::new(),
             doc_extern_map: LazyCell::new(),
+            progress_config: ProgressConfig::default(),
         }
     }
 
@@ -459,8 +461,8 @@ impl Config {
 
     /// Get a configuration value by key.
     ///
-    /// This does NOT look at environment variables, the caller is responsible
-    /// for that.
+    /// This does NOT look at environment variables. See `get_cv_with_env` for
+    /// a variant that supports environment variables.
     fn get_cv(&self, key: &ConfigKey) -> CargoResult<Option<ConfigValue>> {
         log::trace!("get cv {:?}", key);
         let vals = self.values()?;
@@ -720,13 +722,9 @@ impl Config {
         let extra_verbose = verbose >= 2;
         let verbose = verbose != 0;
 
-        #[derive(Deserialize, Default)]
-        struct TermConfig {
-            verbose: Option<bool>,
-            color: Option<String>,
-        }
-
-        // Ignore errors in the configuration files.
+        // Ignore errors in the configuration files. We don't want basic
+        // commands like `cargo version` to error out due to config file
+        // problems.
         let term = self.get::<TermConfig>("term").unwrap_or_default();
 
         let color = color.or_else(|| term.color.as_deref());
@@ -754,6 +752,7 @@ impl Config {
 
         self.shell().set_verbosity(verbosity);
         self.shell().set_color_choice(color)?;
+        self.progress_config = term.progress.unwrap_or_default();
         self.extra_verbose = extra_verbose;
         self.frozen = frozen;
         self.locked = locked;
@@ -1192,6 +1191,20 @@ impl Config {
             .try_borrow_with(|| Ok(self.get::<CargoBuildConfig>("build")?))
     }
 
+    pub fn progress_config(&self) -> &ProgressConfig {
+        &self.progress_config
+    }
+
+    /// This is used to validate the `term` table has valid syntax.
+    ///
+    /// This is necessary because loading the term settings happens very
+    /// early, and in some situations (like `cargo version`) we don't want to
+    /// fail if there are problems with the config file.
+    pub fn validate_term_config(&self) -> CargoResult<()> {
+        drop(self.get::<TermConfig>("term")?);
+        Ok(())
+    }
+
     /// Returns a list of [target.'cfg()'] tables.
     ///
     /// The list is sorted by the table name.
@@ -1778,6 +1791,94 @@ pub struct CargoBuildConfig {
     pub out_dir: Option<ConfigRelativePath>,
 }
 
+#[derive(Deserialize, Default)]
+struct TermConfig {
+    verbose: Option<bool>,
+    color: Option<String>,
+    #[serde(default)]
+    #[serde(deserialize_with = "progress_or_string")]
+    progress: Option<ProgressConfig>,
+}
+
+#[derive(Debug, Default, Deserialize)]
+pub struct ProgressConfig {
+    pub when: ProgressWhen,
+    pub width: Option<usize>,
+}
+
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "lowercase")]
+pub enum ProgressWhen {
+    Auto,
+    Never,
+    Always,
+}
+
+impl Default for ProgressWhen {
+    fn default() -> ProgressWhen {
+        ProgressWhen::Auto
+    }
+}
+
+fn progress_or_string<'de, D>(deserializer: D) -> Result<Option<ProgressConfig>, D::Error>
+where
+    D: serde::de::Deserializer<'de>,
+{
+    struct ProgressVisitor;
+
+    impl<'de> serde::de::Visitor<'de> for ProgressVisitor {
+        type Value = Option<ProgressConfig>;
+
+        fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+            formatter.write_str("a string (\"auto\" or \"never\") or a table")
+        }
+
+        fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            match s {
+                "auto" => Ok(Some(ProgressConfig {
+                    when: ProgressWhen::Auto,
+                    width: None,
+                })),
+                "never" => Ok(Some(ProgressConfig {
+                    when: ProgressWhen::Never,
+                    width: None,
+                })),
+                "always" => Err(E::custom("\"always\" progress requires a `width` key")),
+                _ => Err(E::unknown_variant(s, &["auto", "never"])),
+            }
+        }
+
+        fn visit_none<E>(self) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            Ok(None)
+        }
+
+        fn visit_some<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
+        where
+            D: serde::de::Deserializer<'de>,
+        {
+            let pc = ProgressConfig::deserialize(deserializer)?;
+            if let ProgressConfig {
+                when: ProgressWhen::Always,
+                width: None,
+            } = pc
+            {
+                return Err(serde::de::Error::custom(
+                    "\"always\" progress requires a `width` key",
+                ));
+            }
+            Ok(Some(pc))
+        }
+    }
+
+    deserializer.deserialize_option(ProgressVisitor)
+}
+
 /// A type to deserialize a list of strings from a toml file.
 ///
 /// Supports deserializing either a whitespace-separated list of arguments in a

src/cargo/util/progress.rs

@@ -3,6 +3,7 @@ use std::env;
 use std::time::{Duration, Instant};
 
 use crate::core::shell::Verbosity;
+use crate::util::config::ProgressWhen;
 use crate::util::{is_ci, CargoResult, Config};
 
 use unicode_width::UnicodeWidthChar;
@@ -28,6 +29,7 @@ struct State<'cfg> {
     done: bool,
     throttle: Throttle,
     last_line: Option<String>,
+    fixed_width: Option<usize>,
 }
 
 struct Format {
@@ -45,12 +47,26 @@ impl<'cfg> Progress<'cfg> {
             Ok(term) => term == "dumb",
             Err(_) => false,
         };
+        let progress_config = cfg.progress_config();
+        match progress_config.when {
+            ProgressWhen::Always => return Progress::new_priv(name, style, cfg),
+            ProgressWhen::Never => return Progress { state: None },
+            ProgressWhen::Auto => {}
+        }
         if cfg.shell().verbosity() == Verbosity::Quiet || dumb || is_ci() {
             return Progress { state: None };
         }
+        Progress::new_priv(name, style, cfg)
+    }
+
+    fn new_priv(name: &str, style: ProgressStyle, cfg: &'cfg Config) -> Progress<'cfg> {
+        let progress_config = cfg.progress_config();
+        let width = progress_config
+            .width
+            .or_else(|| cfg.shell().err_width().progress_max_width());
 
         Progress {
-            state: cfg.shell().err_width().progress_max_width().map(|n| State {
+            state: width.map(|n| State {
                 config: cfg,
                 format: Format {
                     style,
@@ -61,6 +77,7 @@ impl<'cfg> Progress<'cfg> {
                 done: false,
                 throttle: Throttle::new(),
                 last_line: None,
+                fixed_width: progress_config.width,
             }),
         }
     }
@@ -216,8 +233,10 @@ impl<'cfg> State<'cfg> {
     }
 
     fn try_update_max_width(&mut self) {
-        if let Some(n) = self.config.shell().err_width().progress_max_width() {
-            self.format.max_width = n;
+        if self.fixed_width.is_none() {
+            if let Some(n) = self.config.shell().err_width().progress_max_width() {
+                self.format.max_width = n;
+            }
         }
     }
 }

src/doc/src/reference/config.md

@@ -147,6 +147,8 @@ metadata_key2 = "value"
 [term]
 verbose = false        # whether cargo provides verbose output
 color = 'auto'         # whether cargo colorizes output
+progress.when = 'auto' # whether cargo shows progress bar
+progress.width = 80    # width of progress bar
 ```
 
 ### Environment variables
@@ -903,6 +905,23 @@ Controls whether or not colored output is used in the terminal. Possible values:
 
 Can be overridden with the `--color` command-line option.
 
+##### `term.progress.when`
+* Type: string
+* Default: "auto"
+* Environment: `CARGO_TERM_PROGRESS_WHEN`
+
+Controls whether or not progress bar is shown in the terminal. Possible values:
+
+* `auto` (default): Intelligently guess whether to show progress bar.
+* `always`: Always show progress bar.
+* `never`: Never show progress bar.
+
+##### `term.progress.width`
+* Type: integer
+* Default: none
+* Environment: `CARGO_TERM_PROGRESS_WIDTH`
+
+Sets the width for progress bar.
 
 [`cargo bench`]: ../commands/cargo-bench.md
 [`cargo login`]: ../commands/cargo-login.md

src/doc/src/reference/environment-variables.md

@@ -103,6 +103,8 @@ supported environment variables are:
 * `CARGO_TARGET_<triple>_RUSTFLAGS` — Extra `rustc` flags for a target, see [`target.<triple>.rustflags`].
 * `CARGO_TERM_VERBOSE` — The default terminal verbosity, see [`term.verbose`].
 * `CARGO_TERM_COLOR` — The default color mode, see [`term.color`].
+* `CARGO_TERM_PROGRESS_WHEN` — The default progress bar showing mode, see [`term.progress.when`].
+* `CARGO_TERM_PROGRESS_WIDTH` — The default progress bar width, see [`term.progress.width`].
 
 [`cargo doc`]: ../commands/cargo-doc.md
 [`cargo install`]: ../commands/cargo-install.md
@@ -158,6 +160,8 @@ supported environment variables are:
 [`target.<triple>.rustflags`]: config.md#targettriplerustflags
 [`term.verbose`]: config.md#termverbose
 [`term.color`]: config.md#termcolor
+[`term.progress.when`]: config.md#termprogresswhen
+[`term.progress.width`]: config.md#termprogresswidth
 
 ### Environment variables Cargo sets for crates
 

tests/testsuite/main.rs

@@ -88,6 +88,7 @@ mod profile_custom;
 mod profile_overrides;
 mod profile_targets;
 mod profiles;
+mod progress;
 mod pub_priv;
 mod publish;
 mod publish_lockfile;

tests/testsuite/progress.rs

@@ -0,0 +1,159 @@
+//! Tests for progress bar.
+
+use cargo_test_support::project;
+use cargo_test_support::registry::Package;
+
+#[cargo_test]
+fn bad_progress_config_unknown_when() {
+    let p = project()
+        .file(
+            ".cargo/config",
+            r#"
+            [term]
+            progress = { when = 'unknown' }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr(
+            "\
+[ERROR] error in [..].cargo/config: \
+could not load config key `term.progress.when`
+
+Caused by:
+  unknown variant `unknown`, expected one of `auto`, `never`, `always`
+",
+        )
+        .run();
+}
+
+#[cargo_test]
+fn bad_progress_config_missing_width() {
+    let p = project()
+        .file(
+            ".cargo/config",
+            r#"
+            [term]
+            progress = { when = 'always' }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr(
+            "\
+[ERROR] \"always\" progress requires a `width` key
+",
+        )
+        .run();
+}
+
+#[cargo_test]
+fn bad_progress_config_missing_when() {
+    let p = project()
+        .file(
+            ".cargo/config",
+            r#"
+            [term]
+            progress = { width = 1000 }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("build")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: missing field `when`
+",
+        )
+        .run();
+}
+
+#[cargo_test]
+fn always_shows_progress() {
+    const N: usize = 3;
+    let mut deps = String::new();
+    for i in 1..=N {
+        Package::new(&format!("dep{}", i), "1.0.0").publish();
+        deps.push_str(&format!("dep{} = \"1.0\"\n", i));
+    }
+
+    let p = project()
+        .file(
+            ".cargo/config",
+            r#"
+            [term]
+            progress = { when = 'always', width = 100 }
+            "#,
+        )
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            {}
+            "#,
+                deps
+            ),
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("build")
+        .with_stderr_contains("[DOWNLOADING] [..] crates [..]")
+        .with_stderr_contains("[..][DOWNLOADED] 3 crates ([..]) in [..]")
+        .with_stderr_contains("[BUILDING] [..] [..]/4: [..]")
+        .run();
+}
+
+#[cargo_test]
+fn never_progress() {
+    const N: usize = 3;
+    let mut deps = String::new();
+    for i in 1..=N {
+        Package::new(&format!("dep{}", i), "1.0.0").publish();
+        deps.push_str(&format!("dep{} = \"1.0\"\n", i));
+    }
+
+    let p = project()
+        .file(
+            ".cargo/config",
+            r#"
+            [term]
+            progress = { when = 'never' }
+            "#,
+        )
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            {}
+            "#,
+                deps
+            ),
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("build")
+        .with_stderr_does_not_contain("[DOWNLOADING] [..] crates [..]")
+        .with_stderr_does_not_contain("[..][DOWNLOADED] 3 crates ([..]) in [..]")
+        .with_stderr_does_not_contain("[BUILDING] [..] [..]/4: [..]")
+        .run();
+}