Rollup merge of #135950 - Kobzol:tidy-python-improvements, r=onur-ozkan

Tidy Python improvements

Fixes display of Python formatting diffs in tidy, and refactors the code to make it simpler and more robust. Also documents Python formatting and linting in the Rustc dev guide.

Fixes: https://github.com/rust-lang/rust/issues/135942

r? `@onur-ozkan`

1 files changed, 85 insertions, 56 deletions

diff --git a/src/tools/tidy/src/ext_tool_checks.rs b/src/tools/tidy/src/ext_tool_checks.rs
index 6269d91c7ec..2a4cff1d12e 100644
--- a/src/tools/tidy/src/ext_tool_checks.rs
+++ b/src/tools/tidy/src/ext_tool_checks.rs
@@ -89,74 +89,45 @@ fn check_impl(
 
     if python_lint {
         eprintln!("linting python files");
-        let mut cfg_args_ruff = cfg_args.clone();
-        let mut file_args_ruff = file_args.clone();
-
-        let mut cfg_path = root_path.to_owned();
-        cfg_path.extend(RUFF_CONFIG_PATH);
-        let mut cache_dir = outdir.to_owned();
-        cache_dir.extend(RUFF_CACHE_PATH);
-
-        cfg_args_ruff.extend([
-            "--config".as_ref(),
-            cfg_path.as_os_str(),
-            "--cache-dir".as_ref(),
-            cache_dir.as_os_str(),
-        ]);
-
-        if file_args_ruff.is_empty() {
-            file_args_ruff.push(root_path.as_os_str());
-        }
-
-        let mut args = merge_args(&cfg_args_ruff, &file_args_ruff);
-        args.insert(0, "check".as_ref());
-        let res = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
+        let py_path = py_path.as_ref().unwrap();
+        let res = run_ruff(root_path, outdir, py_path, &cfg_args, &file_args, &["check".as_ref()]);
 
         if res.is_err() && show_diff {
             eprintln!("\npython linting failed! Printing diff suggestions:");
 
-            args.insert(1, "--diff".as_ref());
-            let _ = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
+            let _ = run_ruff(root_path, outdir, py_path, &cfg_args, &file_args, &[
+                "check".as_ref(),
+                "--diff".as_ref(),
+            ]);
         }
         // Rethrow error
         let _ = res?;
     }
 
     if python_fmt {
-        let mut cfg_args_ruff = cfg_args.clone();
-        let mut file_args_ruff = file_args.clone();
-
+        let mut args: Vec<&OsStr> = vec!["format".as_ref()];
         if bless {
             eprintln!("formatting python files");
         } else {
             eprintln!("checking python file formatting");
-            cfg_args_ruff.push("--check".as_ref());
-        }
-
-        let mut cfg_path = root_path.to_owned();
-        cfg_path.extend(RUFF_CONFIG_PATH);
-        let mut cache_dir = outdir.to_owned();
-        cache_dir.extend(RUFF_CACHE_PATH);
-
-        cfg_args_ruff.extend(["--config".as_ref(), cfg_path.as_os_str()]);
-
-        if file_args_ruff.is_empty() {
-            file_args_ruff.push(root_path.as_os_str());
+            args.push("--check".as_ref());
         }
 
-        let mut args = merge_args(&cfg_args_ruff, &file_args_ruff);
-        args.insert(0, "format".as_ref());
-        let res = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
+        let py_path = py_path.as_ref().unwrap();
+        let res = run_ruff(root_path, outdir, py_path, &cfg_args, &file_args, &args);
 
-        if res.is_err() && show_diff {
-            eprintln!("\npython formatting does not match! Printing diff:");
-
-            args.insert(0, "--diff".as_ref());
-            let _ = py_runner(py_path.as_ref().unwrap(), true, None, "ruff", &args);
-        }
         if res.is_err() && !bless {
+            if show_diff {
+                eprintln!("\npython formatting does not match! Printing diff:");
+
+                let _ = run_ruff(root_path, outdir, py_path, &cfg_args, &file_args, &[
+                    "format".as_ref(),
+                    "--diff".as_ref(),
+                ]);
+            }
             eprintln!("rerun tidy with `--extra-checks=py:fmt --bless` to reformat Python code");
         }
+
         // Rethrow error
         let _ = res?;
     }
@@ -247,6 +218,38 @@ fn check_impl(
     Ok(())
 }
 
+fn run_ruff(
+    root_path: &Path,
+    outdir: &Path,
+    py_path: &Path,
+    cfg_args: &[&OsStr],
+    file_args: &[&OsStr],
+    ruff_args: &[&OsStr],
+) -> Result<(), Error> {
+    let mut cfg_args_ruff = cfg_args.into_iter().copied().collect::<Vec<_>>();
+    let mut file_args_ruff = file_args.into_iter().copied().collect::<Vec<_>>();
+
+    let mut cfg_path = root_path.to_owned();
+    cfg_path.extend(RUFF_CONFIG_PATH);
+    let mut cache_dir = outdir.to_owned();
+    cache_dir.extend(RUFF_CACHE_PATH);
+
+    cfg_args_ruff.extend([
+        "--config".as_ref(),
+        cfg_path.as_os_str(),
+        "--cache-dir".as_ref(),
+        cache_dir.as_os_str(),
+    ]);
+
+    if file_args_ruff.is_empty() {
+        file_args_ruff.push(root_path.as_os_str());
+    }
+
+    let mut args: Vec<&OsStr> = ruff_args.into_iter().copied().collect();
+    args.extend(merge_args(&cfg_args_ruff, &file_args_ruff));
+    py_runner(py_path, true, None, "ruff", &args)
+}
+
 /// Helper to create `cfg1 cfg2 -- file1 file2` output
 fn merge_args<'a>(cfg_args: &[&'a OsStr], file_args: &[&'a OsStr]) -> Vec<&'a OsStr> {
     let mut args = cfg_args.to_owned();
@@ -321,8 +324,16 @@ fn get_or_create_venv(venv_path: &Path, src_reqs_path: &Path) -> Result<PathBuf,
 fn create_venv_at_path(path: &Path) -> Result<(), Error> {
     /// Preferred python versions in order. Newest to oldest then current
     /// development versions
-    const TRY_PY: &[&str] =
-        &["python3.11", "python3.10", "python3.9", "python3", "python", "python3.12", "python3.13"];
+    const TRY_PY: &[&str] = &[
+        "python3.13",
+        "python3.12",
+        "python3.11",
+        "python3.10",
+        "python3.9",
+        "python3",
+        "python",
+        "python3.14",
+    ];
 
     let mut sys_py = None;
     let mut found = Vec::new();
@@ -357,22 +368,40 @@ fn create_venv_at_path(path: &Path) -> Result<(), Error> {
         return Err(ret);
     };
 
-    eprintln!("creating virtual environment at '{}' using '{sys_py}'", path.display());
-    let out = Command::new(sys_py).args(["-m", "virtualenv"]).arg(path).output().unwrap();
+    // First try venv, which should be packaged in the Python3 standard library.
+    // If it is not available, try to create the virtual environment using the
+    // virtualenv package.
+    if try_create_venv(sys_py, path, "venv").is_ok() {
+        return Ok(());
+    }
+    try_create_venv(sys_py, path, "virtualenv")
+}
+
+fn try_create_venv(python: &str, path: &Path, module: &str) -> Result<(), Error> {
+    eprintln!(
+        "creating virtual environment at '{}' using '{python}' and '{module}'",
+        path.display()
+    );
+    let out = Command::new(python).args(["-m", module]).arg(path).output().unwrap();
 
     if out.status.success() {
         return Ok(());
     }
 
     let stderr = String::from_utf8_lossy(&out.stderr);
-    let err = if stderr.contains("No module named virtualenv") {
+    let err = if stderr.contains(&format!("No module named {module}")) {
         Error::Generic(format!(
-            "virtualenv not found: you may need to install it \
-                               (`{sys_py} -m pip install virtualenv`)"
+            r#"{module} not found: you may need to install it:
+`{python} -m pip install {module}`
+If you see an error about "externally managed environment" when running the above command,
+either install `{module}` using your system package manager
+(e.g. `sudo apt-get install {python}-{module}`) or create a virtual environment manually, install
+`{module}` in it and then activate it before running tidy.
+"#
         ))
     } else {
         Error::Generic(format!(
-            "failed to create venv at '{}' using {sys_py}: {stderr}",
+            "failed to create venv at '{}' using {python} -m {module}: {stderr}",
             path.display()
         ))
     };