diff options
| -rw-r--r-- | src/doc/rustc-dev-guide/src/conventions.md | 28 | ||||
| -rw-r--r-- | src/tools/tidy/src/ext_tool_checks.rs | 141 |
2 files changed, 110 insertions, 59 deletions
diff --git a/src/doc/rustc-dev-guide/src/conventions.md b/src/doc/rustc-dev-guide/src/conventions.md index 37af8121cd1..4010e90821f 100644 --- a/src/doc/rustc-dev-guide/src/conventions.md +++ b/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 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() )) }; |