Rollup of 6 pull requests

src/doc/rustc-dev-guide/src/conventions.md

@@ -1,4 +1,4 @@
-This file offers some tips on the coding conventions for rustc.  This
+This file offers some tips on the coding conventions for rustc. This
 chapter covers [formatting](#formatting), [coding for correctness](#cc),
 [using crates from crates.io](#cio), and some tips on
 [structuring your PR for easy review](#er).
@@ -25,6 +25,7 @@ pass the <!-- date-check: nov 2022 --> `--edition=2021` argument yourself when c
 `rustfmt` directly.
 
 [fmt]: https://github.com/rust-dev-tools/fmt-rfcs
+
 [`rustfmt`]:https://github.com/rust-lang/rustfmt
 
 ## Formatting C++ code
@@ -40,6 +41,26 @@ When modifying that code, use this command to format it:
 This uses a pinned version of `clang-format`, to avoid relying on the local
 environment.
 
+## Formatting and linting Python code
+
+The Rust repository contains quite a lof of Python code. We try to keep
+it both linted and formatted by the [ruff][ruff] tool.
+
+When modifying Python code, use this command to format it:
+```sh
+./x test tidy --extra-checks=py:fmt --bless
+```
+
+and the following command to run lints:
+```sh
+./x test tidy --extra-checks=py:lint
+```
+
+This uses a pinned version of `ruff`, to avoid relying on the local
+environment.
+
+[ruff]: https://github.com/astral-sh/ruff
+
 <a id="copyright"></a>
 
 <!-- REUSE-IgnoreStart -->
@@ -84,7 +105,7 @@ Using `_` in a match is convenient, but it means that when new
 variants are added to the enum, they may not get handled correctly.
 Ask yourself: if a new variant were added to this enum, what's the
 chance that it would want to use the `_` code, versus having some
-other treatment?  Unless the answer is "low", then prefer an
+other treatment? Unless the answer is "low", then prefer an
 exhaustive match. (The same advice applies to `if let` and `while
 let`, which are effectively tests for a single variant.)
 
@@ -124,7 +145,7 @@ See the [crates.io dependencies][crates] section.
 # How to structure your PR
 
 How you prepare the commits in your PR can make a big difference for the
-reviewer.  Here are some tips.
+reviewer. Here are some tips.
 
 **Isolate "pure refactorings" into their own commit.** For example, if
 you rename a method, then put that rename into its own commit, along
@@ -165,4 +186,5 @@ to the compiler.
   crate-related, often the spelling is changed to `krate`.
 
 [tcx]: ./ty.md
+
 [crates]: ./crates-io.md

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()
         ))
     };