Skip to content
Merged
2 changes: 1 addition & 1 deletion crates/ruff/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ fn is_stdin(files: &[PathBuf], stdin_filename: Option<&Path>) -> bool {
file == Path::new("-")
}

/// Returns the default set of files if none are provided, otherwise returns `None`.
/// Returns the default set of files if none are provided, otherwise returns provided files.
fn resolve_default_files(files: Vec<PathBuf>, is_stdin: bool) -> Vec<PathBuf> {
if files.is_empty() {
if is_stdin {
Expand Down
22 changes: 9 additions & 13 deletions crates/ruff/src/resolve.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::path::Path;

use anyhow::Result;
use anyhow::{bail, Result};
use log::debug;
use path_absolutize::path_dedot;

Expand All @@ -21,10 +21,14 @@ pub fn resolve(
config_arguments: &ConfigArguments,
stdin_filename: Option<&Path>,
) -> Result<PyprojectConfig> {
let Ok(cwd) = std::env::current_dir() else {
bail!("Working directory does not exist")
};

// First priority: if we're running in isolated mode, use the default settings.
if config_arguments.isolated {
let config = config_arguments.transform(Configuration::default());
let settings = config.into_settings(&path_dedot::CWD)?;
let settings = config.into_settings(&cwd)?;
debug!("Isolated mode, not reading any pyproject.toml");
return Ok(PyprojectConfig::new(
PyprojectDiscoveryStrategy::Fixed,
Expand Down Expand Up @@ -58,11 +62,7 @@ pub fn resolve(
// that directory. (With `Strategy::Hierarchical`, we'll end up finding
// the "closest" `pyproject.toml` file for every Python file later on,
// so these act as the "default" settings.)
if let Some(pyproject) = pyproject::find_settings_toml(
stdin_filename
.as_ref()
.unwrap_or(&path_dedot::CWD.as_path()),
)? {
if let Some(pyproject) = pyproject::find_settings_toml(stdin_filename.unwrap_or(&cwd))? {
debug!(
"Using configuration file (via parent) at: {}",
pyproject.display()
Expand Down Expand Up @@ -130,17 +130,13 @@ pub fn resolve(
// `pyproject::find_settings_toml`.)
// However, there may be a `pyproject.toml` with a `requires-python`
// specified, and that is what we look for in this step.
let fallback = find_fallback_target_version(
stdin_filename
.as_ref()
.unwrap_or(&path_dedot::CWD.as_path()),
);
let fallback = find_fallback_target_version(stdin_filename.unwrap_or(&cwd));
if let Some(version) = fallback {
debug!("Derived `target-version` from found `requires-python`: {version:?}");
}
config.target_version = fallback.map(ast::PythonVersion::from);
}
let settings = config.into_settings(&path_dedot::CWD)?;
let settings = config.into_settings(&cwd)?;
Ok(PyprojectConfig::new(
PyprojectDiscoveryStrategy::Hierarchical,
settings,
Expand Down
30 changes: 30 additions & 0 deletions crates/ruff/tests/direxist_guard.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//! Test to verify Ruff's behavior when run from deleted directory.
//! It has to be isolated in a separate module.
//! Tests in the same module become flaky under `cargo test`s parallel execution
//! due to in-test working directory manipulation.

#![cfg(target_family = "unix")]

use std::env::set_current_dir;
use std::process::Command;

use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
const BIN_NAME: &str = "ruff";

#[test]
fn check_in_deleted_directory_errors() {
let temp_dir = tempfile::tempdir().unwrap();
let temp_path = temp_dir.path().to_path_buf();
set_current_dir(&temp_path).unwrap();
drop(temp_dir);

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)).arg("check"), @r###"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @ntBre that changing the working directory is problematic. Let's only override the current working directory for the spanned CLI command.

Suggested change
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)).arg("check"), @r###"
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)).arg("check").current_dir(&temp_path), @r###"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I tried current_dir when I looked at this, and it caused a different panic in the Command invocation, separate from the one we're trying to test. But it would be good to double check, and maybe I missed something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the assert_cmd_snapshot or current_dir calls an unwrap() somewhere. The test fails with:

[...] called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I see. Let's add some module level documentation (or at least test level) explaining why this test has to be its own module.

success: false
exit_code: 2
----- stdout -----

----- stderr -----
ruff failed
Cause: Working directory does not exist
"###);
}
Loading