Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Gracefully handle errors in CLI (#12747)

This commit is contained in:
Micha Reiser 2024-08-08 13:02:47 +02:00 committed by GitHub
parent 6d9205e346
commit 2daa914334
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 97 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

125
crates/red_knot/src/main.rs
View File

@ -1,9 +1,11 @@
use std::process::ExitCode;
use std::process::{ExitCode, Termination};
use std::sync::Mutex;
use anyhow::{anyhow, Context};
use clap::Parser;
use colored::Colorize;
use crossbeam::channel as crossbeam_channel;
use salsa::plumbing::ZalsaDatabase;
use red_knot_server::run_server;
use red_knot_workspace::db::RootDatabase;
@ -12,7 +14,7 @@ use red_knot_workspace::watch;
use red_knot_workspace::watch::WorkspaceWatcher;
use red_knot_workspace::workspace::WorkspaceMetadata;
use ruff_db::program::{ProgramSettings, SearchPathSettings};
use ruff_db::system::{OsSystem, System, SystemPathBuf};
use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf};
use target_version::TargetVersion;
use crate::logging::{setup_tracing, Verbosity};
@ -86,30 +88,25 @@ pub enum Command {
}
#[allow(clippy::print_stdout, clippy::unnecessary_wraps, clippy::print_stderr)]
pub fn main() -> ExitCode {
match run() {
Ok(status) => status.into(),
Err(error) => {
{
use std::io::Write;
pub fn main() -> ExitStatus {
run().unwrap_or_else(|error| {
use std::io::Write;
// Use `writeln` instead of `eprintln` to avoid panicking when the stderr pipe is broken.
let mut stderr = std::io::stderr().lock();
// Use `writeln` instead of `eprintln` to avoid panicking when the stderr pipe is broken.
let mut stderr = std::io::stderr().lock();
// This communicates that this isn't a linter error but ruff itself hard-errored for
// some reason (e.g. failed to resolve the configuration)
writeln!(stderr, "{}", "ruff failed".red().bold()).ok();
// Currently we generally only see one error, but e.g. with io errors when resolving
// the configuration it is help to chain errors ("resolving configuration failed" ->
// "failed to read file: subdir/pyproject.toml")
for cause in error.chain() {
writeln!(stderr, " {} {cause}", "Cause:".bold()).ok();
}
}
ExitStatus::Error.into()
// This communicates that this isn't a linter error but Red Knot itself hard-errored for
// some reason (e.g. failed to resolve the configuration)
writeln!(stderr, "{}", "ruff failed".red().bold()).ok();
// Currently we generally only see one error, but e.g. with io errors when resolving
// the configuration it is help to chain errors ("resolving configuration failed" ->
// "failed to read file: subdir/pyproject.toml")
for cause in error.chain() {
writeln!(stderr, " {} {cause}", "Cause:".bold()).ok();
}
}
ExitStatus::Error
})
}
fn run() -> anyhow::Result<ExitStatus> {
@ -132,28 +129,43 @@ fn run() -> anyhow::Result<ExitStatus> {
countme::enable(verbosity.is_trace());
let _guard = setup_tracing(verbosity)?;
let cwd = if let Some(cwd) = current_directory {
let canonicalized = cwd.as_utf8_path().canonicalize_utf8().unwrap();
SystemPathBuf::from_utf8_path_buf(canonicalized)
} else {
let cwd = std::env::current_dir().unwrap();
SystemPathBuf::from_path_buf(cwd).unwrap()
// The base path to which all CLI arguments are relative to.
let cli_base_path = {
let cwd = std::env::current_dir().context("Failed to get the current working directory")?;
SystemPathBuf::from_path_buf(cwd).map_err(|path| anyhow!("The current working directory '{}' contains non-unicode characters. Red Knot only supports unicode paths.", path.display()))?
};
let cwd = current_directory
.map(|cwd| {
if cwd.as_std_path().is_dir() {
Ok(SystemPath::absolute(&cwd, &cli_base_path))
} else {
Err(anyhow!(
"Provided current-directory path '{cwd}' is not a directory."
))
}
})
.transpose()?
.unwrap_or_else(|| cli_base_path.clone());
let system = OsSystem::new(cwd.clone());
let workspace_metadata =
WorkspaceMetadata::from_path(system.current_directory(), &system).unwrap();
let workspace_metadata = WorkspaceMetadata::from_path(system.current_directory(), &system)?;
let site_packages = if let Some(venv_path) = venv_path {
let venv_path = system.canonicalize_path(&venv_path).unwrap_or(venv_path);
assert!(
system.is_directory(&venv_path),
"Provided venv-path {venv_path} is not a directory!"
);
site_packages_dirs_of_venv(&venv_path, &system).unwrap()
} else {
vec![]
};
// TODO: Verify the remaining search path settings eagerly.
let site_packages = venv_path
.map(|venv_path| {
let venv_path = SystemPath::absolute(venv_path, &cli_base_path);
if system.is_directory(&venv_path) {
Ok(site_packages_dirs_of_venv(&venv_path, &system)?)
} else {
Err(anyhow!(
"Provided venv-path {venv_path} is not a directory!"
))
}
})
.transpose()?
.unwrap_or_default();
// TODO: Respect the settings from the workspace metadata. when resolving the program settings.
let program_settings = ProgramSettings {
@ -207,9 +219,9 @@ pub enum ExitStatus {
Error = 2,
}
impl From<ExitStatus> for ExitCode {
fn from(status: ExitStatus) -> Self {
ExitCode::from(status as u8)
impl Termination for ExitStatus {
fn report(self) -> ExitCode {
ExitCode::from(self as u8)
}
}
@ -262,12 +274,11 @@ impl MainLoop {
result
}
#[allow(clippy::print_stderr)]
fn main_loop(&mut self, db: &mut RootDatabase) -> ExitStatus {
// Schedule the first check.
tracing::debug!("Starting main loop");
let mut revision = 0usize;
let mut revision = 0u64;
while let Ok(message) = self.receiver.recv() {
match message {
@ -282,7 +293,7 @@ impl MainLoop {
// Send the result back to the main loop for printing.
sender
.send(MainLoopMessage::CheckCompleted { result, revision })
.ok();
.unwrap();
}
});
}
@ -291,17 +302,20 @@ impl MainLoop {
result,
revision: check_revision,
} => {
let has_diagnostics = !result.is_empty();
if check_revision == revision {
eprintln!("{}", result.join("\n"));
for diagnostic in result {
tracing::error!("{}", diagnostic);
}
} else {
tracing::debug!("Discarding check result for outdated revision: current: {revision}, result revision: {check_revision}");
}
if self.watcher.is_none() {
return if result.is_empty() {
ExitStatus::Success
} else {
return if has_diagnostics {
ExitStatus::Failure
} else {
ExitStatus::Success
};
}
@ -318,6 +332,10 @@ impl MainLoop {
self.sender.send(MainLoopMessage::CheckWorkspace).unwrap();
}
MainLoopMessage::Exit => {
// Cancel any pending queries and wait for them to complete.
// TODO: Don't use Salsa internal APIs
// [Zulip-Thread](https://salsa.zulipchat.com/#narrow/stream/333573-salsa-3.2E0/topic/Expose.20an.20API.20to.20cancel.20other.20queries)
let _ = db.zalsa_mut();
return ExitStatus::Success;
}
}
@ -344,10 +362,7 @@ impl MainLoopCancellationToken {
#[derive(Debug)]
enum MainLoopMessage {
CheckWorkspace,
CheckCompleted {
result: Vec<String>,
revision: usize,
},
CheckCompleted { result: Vec<String>, revision: u64 },
ApplyChanges(Vec<watch::ChangeEvent>),
Exit,
}

39
crates/red_knot_workspace/src/site_packages.rs
View File

@ -34,12 +34,17 @@ fn site_packages_dir_from_sys_prefix(
sys_prefix_path: &SystemPath,
system: &dyn System,
) -> Result<SystemPathBuf, SitePackagesDiscoveryError> {
tracing::debug!("Searching for site-packages directory in '{sys_prefix_path}'");
if cfg!(target_os = "windows") {
let site_packages = sys_prefix_path.join("Lib/site-packages");
return system
.is_directory(&site_packages)
.then_some(site_packages)
.ok_or(SitePackagesDiscoveryError::NoSitePackagesDirFound);
return if system.is_directory(&site_packages) {
tracing::debug!("Resolved site-packages directory to '{site_packages}'");
Ok(site_packages)
} else {
Err(SitePackagesDiscoveryError::NoSitePackagesDirFound)
};
}
// In the Python standard library's `site.py` module (used for finding `site-packages`
@ -68,11 +73,12 @@ fn site_packages_dir_from_sys_prefix(
let Ok(entry) = entry_result else {
continue;
};
if !entry.file_type().is_directory() {
continue;
}
let path = entry.path();
let mut path = entry.into_path();
// The `python3.x` part of the `site-packages` path can't be computed from
// the `--target-version` the user has passed, as they might be running Python 3.12 locally
@ -84,21 +90,18 @@ fn site_packages_dir_from_sys_prefix(
// the `pyvenv.cfg` file anyway, in which case we could switch to that method
// rather than iterating through the whole directory until we find
// an entry where the last component of the path starts with `python3.`
if !path
.components()
.next_back()
.is_some_and(|last_part| last_part.as_str().starts_with("python3."))
{
let name = path
.file_name()
.expect("File name to be non-null because path is guaranteed to be a child of `lib`");
if !name.starts_with("python3.") {
continue;
}
let site_packages_candidate = path.join("site-packages");
if system.is_directory(&site_packages_candidate) {
tracing::debug!(
"Resoled site-packages directory: {}",
site_packages_candidate
);
return Ok(site_packages_candidate);
path.push("site-packages");
if system.is_directory(&path) {
tracing::debug!("Resolved site-packages directory to '{path}'");
return Ok(path);
}
}
Err(SitePackagesDiscoveryError::NoSitePackagesDirFound)
@ -106,7 +109,7 @@ fn site_packages_dir_from_sys_prefix(
#[derive(Debug, thiserror::Error)]
pub enum SitePackagesDiscoveryError {
#[error("Failed to search the virtual environment directory for `site-packages` due to {0}")]
#[error("Failed to search the virtual environment directory for `site-packages`")]
CouldNotReadLibDirectory(#[from] io::Error),
#[error("Could not find the `site-packages` directory in the virtual environment")]
NoSitePackagesDirFound,

14
crates/red_knot_workspace/src/workspace/metadata.rs
View File

@ -22,15 +22,13 @@ pub struct PackageMetadata {
impl WorkspaceMetadata {
/// Discovers the closest workspace at `path` and returns its metadata.
pub fn from_path(path: &SystemPath, system: &dyn System) -> anyhow::Result<WorkspaceMetadata> {
let root = if system.is_file(path) {
path.parent().unwrap().to_path_buf()
} else {
path.to_path_buf()
};
assert!(
system.is_directory(path),
"Workspace root path must be a directory"
);
tracing::debug!("Searching for workspace in '{path}'");
if !system.is_directory(&root) {
anyhow::bail!("no workspace found at {:?}", root);
}
let root = path.to_path_buf();
// TODO: Discover package name from `pyproject.toml`.
let package_name: Name = path.file_name().unwrap_or("<root>").into();