diff --git a/CHANGELOG.md b/CHANGELOG.md index f3f1cb5..e9b1162 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,10 +22,12 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/ - Added support for loading server settings from user files (`~/.config/djls/djls.toml`) and project files (`djls.toml`, `.djls.toml`, and `pyproject.toml` via `[tool.djls]` table). - Implemented dynamic settings reloading via `workspace/didChangeConfiguration`. +- Added `venv_path` setting to allow explicit configuration of Python virtual environment. ### Changed - **Internal**: Moved task queueing functionality to `djls-server` crate, renamed from `Worker` to `Queue`, and simplified API. +- Improved Python environment detection. ## [5.2.0a0] diff --git a/crates/djls-conf/src/lib.rs b/crates/djls-conf/src/lib.rs index 0189d1b..4e4157b 100644 --- a/crates/djls-conf/src/lib.rs +++ b/crates/djls-conf/src/lib.rs @@ -17,9 +17,10 @@ pub enum ConfigError { } #[derive(Debug, Deserialize, Default, PartialEq)] -#[serde(default)] pub struct Settings { + #[serde(default)] debug: bool, + venv_path: Option, } impl Settings { @@ -76,6 +77,10 @@ impl Settings { pub fn debug(&self) -> bool { self.debug } + + pub fn venv_path(&self) -> Option<&str> { + self.venv_path.as_deref() + } } #[cfg(test)] @@ -91,9 +96,14 @@ mod tests { fn test_load_no_files() { let dir = tempdir().unwrap(); let settings = Settings::new(dir.path()).unwrap(); - // Should load defaults - assert_eq!(settings, Settings { debug: false }); // Add assertions for future default fields here + assert_eq!( + settings, + Settings { + debug: false, + venv_path: None + } + ); } } @@ -105,7 +115,27 @@ mod tests { let dir = tempdir().unwrap(); fs::write(dir.path().join("djls.toml"), "debug = true").unwrap(); let settings = Settings::new(dir.path()).unwrap(); - assert_eq!(settings, Settings { debug: true }); + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); + } + + #[test] + fn test_load_venv_path_config() { + let dir = tempdir().unwrap(); + fs::write(dir.path().join("djls.toml"), "venv_path = '/path/to/venv'").unwrap(); + let settings = Settings::new(dir.path()).unwrap(); + assert_eq!( + settings, + Settings { + venv_path: Some("/path/to/venv".to_string()), + ..Default::default() + } + ); } #[test] @@ -113,7 +143,13 @@ mod tests { let dir = tempdir().unwrap(); fs::write(dir.path().join(".djls.toml"), "debug = true").unwrap(); let settings = Settings::new(dir.path()).unwrap(); - assert_eq!(settings, Settings { debug: true }); + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } #[test] @@ -123,7 +159,13 @@ mod tests { let content = "[tool.djls]\ndebug = true\n"; fs::write(dir.path().join("pyproject.toml"), content).unwrap(); let settings = Settings::new(dir.path()).unwrap(); - assert_eq!(settings, Settings { debug: true }); + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } } @@ -136,7 +178,14 @@ mod tests { fs::write(dir.path().join(".djls.toml"), "debug = false").unwrap(); fs::write(dir.path().join("djls.toml"), "debug = true").unwrap(); let settings = Settings::new(dir.path()).unwrap(); - assert_eq!(settings, Settings { debug: true }); // djls.toml wins + // djls.toml wins + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } #[test] @@ -146,7 +195,14 @@ mod tests { fs::write(dir.path().join("pyproject.toml"), pyproject_content).unwrap(); fs::write(dir.path().join(".djls.toml"), "debug = true").unwrap(); let settings = Settings::new(dir.path()).unwrap(); - assert_eq!(settings, Settings { debug: true }); // .djls.toml wins + // .djls.toml wins + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } #[test] @@ -157,7 +213,14 @@ mod tests { fs::write(dir.path().join(".djls.toml"), "debug = false").unwrap(); fs::write(dir.path().join("djls.toml"), "debug = true").unwrap(); let settings = Settings::new(dir.path()).unwrap(); - assert_eq!(settings, Settings { debug: true }); // djls.toml wins + // djls.toml wins + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } #[test] @@ -171,7 +234,14 @@ mod tests { let settings = Settings::load_from_paths(project_dir.path(), Some(&user_conf_path)).unwrap(); - assert_eq!(settings, Settings { debug: false }); // pyproject.toml overrides user + // pyproject.toml overrides user + assert_eq!( + settings, + Settings { + debug: false, + ..Default::default() + } + ); } #[test] @@ -184,7 +254,14 @@ mod tests { let settings = Settings::load_from_paths(project_dir.path(), Some(&user_conf_path)).unwrap(); - assert_eq!(settings, Settings { debug: false }); // djls.toml overrides user + // djls.toml overrides user + assert_eq!( + settings, + Settings { + debug: false, + ..Default::default() + } + ); } } @@ -200,7 +277,13 @@ mod tests { let settings = Settings::load_from_paths(project_dir.path(), Some(&user_conf_path)).unwrap(); - assert_eq!(settings, Settings { debug: true }); + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } #[test] @@ -214,7 +297,13 @@ mod tests { // Should load project settings fine, ignoring non-existent user config let settings = Settings::load_from_paths(project_dir.path(), Some(&user_conf_path)).unwrap(); - assert_eq!(settings, Settings { debug: true }); + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } #[test] @@ -225,7 +314,13 @@ mod tests { // Call helper with None for user path let settings = Settings::load_from_paths(project_dir.path(), None).unwrap(); - assert_eq!(settings, Settings { debug: true }); + assert_eq!( + settings, + Settings { + debug: true, + ..Default::default() + } + ); } } diff --git a/crates/djls-project/Cargo.toml b/crates/djls-project/Cargo.toml index 4d560d7..4523ad8 100644 --- a/crates/djls-project/Cargo.toml +++ b/crates/djls-project/Cargo.toml @@ -8,3 +8,6 @@ pyo3 = { workspace = true } tower-lsp-server = { workspace = true, features = ["proposed"] } which = "7.0.1" + +[dev-dependencies] +tempfile = "3.19" diff --git a/crates/djls-project/src/lib.rs b/crates/djls-project/src/lib.rs index 51928e1..6d936e9 100644 --- a/crates/djls-project/src/lib.rs +++ b/crates/djls-project/src/lib.rs @@ -3,6 +3,7 @@ mod templatetags; pub use templatetags::TemplateTags; use pyo3::prelude::*; +use std::env; use std::fmt; use std::path::{Path, PathBuf}; use which::which; @@ -23,10 +24,13 @@ impl DjangoProject { } } - pub fn initialize(&mut self) -> PyResult<()> { - let python_env = PythonEnvironment::new().ok_or_else(|| { - PyErr::new::("Could not find Python in PATH") - })?; + pub fn initialize(&mut self, venv_path: Option<&str>) -> PyResult<()> { + let python_env = + PythonEnvironment::new(self.path.as_path(), venv_path).ok_or_else(|| { + PyErr::new::( + "Could not find Python environment", + ) + })?; Python::with_gil(|py| { let sys = py.import("sys")?; @@ -77,24 +81,70 @@ impl fmt::Display for DjangoProject { } } -#[derive(Debug)] +#[derive(Debug, PartialEq)] struct PythonEnvironment { - #[allow(dead_code)] python_path: PathBuf, sys_path: Vec, sys_prefix: PathBuf, } impl PythonEnvironment { - fn new() -> Option { - let python_path = which("python").ok()?; - let prefix = python_path.parent()?.parent()?; + fn new(project_path: &Path, venv_path: Option<&str>) -> Option { + if let Some(path) = venv_path { + let prefix = PathBuf::from(path); + // If an explicit path is provided and it's a valid venv, use it immediately. + if let Some(env) = Self::from_venv_prefix(&prefix) { + return Some(env); + } + // Explicit path was provided but was invalid. Continue searching. + } + + if let Ok(virtual_env) = env::var("VIRTUAL_ENV") { + if !virtual_env.is_empty() { + let prefix = PathBuf::from(virtual_env); + if let Some(env) = Self::from_venv_prefix(&prefix) { + return Some(env); + } + } + } + + for venv_dir in &[".venv", "venv", "env", ".env"] { + let potential_venv = project_path.join(venv_dir); + if potential_venv.is_dir() { + if let Some(env) = Self::from_venv_prefix(&potential_venv) { + return Some(env); + } + } + } + + Self::from_system_python() + } + + fn from_venv_prefix(prefix: &Path) -> Option { + #[cfg(not(windows))] + let python_path = prefix.join("bin").join("python"); + #[cfg(windows)] + let python_path = prefix.join("Scripts").join("python.exe"); + + // Check if the *prefix* and the *binary* exist. + if !prefix.is_dir() || !python_path.exists() { + return None; + } let mut sys_path = Vec::new(); - sys_path.push(prefix.join("bin")); + + #[cfg(not(windows))] + let bin_dir = prefix.join("bin"); + #[cfg(windows)] + let bin_dir = prefix.join("Scripts"); + + sys_path.push(bin_dir); // Add bin/ or Scripts/ if let Some(site_packages) = Self::find_site_packages(prefix) { - sys_path.push(site_packages); + // Check existence inside the if let, as find_site_packages might return a path that doesn't exist + if site_packages.is_dir() { + sys_path.push(site_packages); + } } Some(Self { @@ -104,23 +154,58 @@ impl PythonEnvironment { }) } - #[cfg(windows)] - fn find_site_packages(prefix: &Path) -> Option { - Some(prefix.join("Lib").join("site-packages")) + fn from_system_python() -> Option { + let python_path = match which("python") { + Ok(p) => p, + Err(_) => return None, + }; + // which() might return a path inside a bin/Scripts dir, or directly the executable + // We need the prefix, which is usually two levels up from the executable in standard layouts + let bin_dir = python_path.parent()?; + let prefix = bin_dir.parent()?; + + let mut sys_path = Vec::new(); + sys_path.push(bin_dir.to_path_buf()); + + if let Some(site_packages) = Self::find_site_packages(prefix) { + if site_packages.is_dir() { + sys_path.push(site_packages); + } + } + + Some(Self { + python_path: python_path.clone(), + sys_path, + sys_prefix: prefix.to_path_buf(), + }) } #[cfg(not(windows))] fn find_site_packages(prefix: &Path) -> Option { - std::fs::read_dir(prefix.join("lib")) + // Look for lib/pythonX.Y/site-packages + let lib_dir = prefix.join("lib"); + if !lib_dir.is_dir() { + return None; + } + std::fs::read_dir(lib_dir) .ok()? .filter_map(Result::ok) - .find(|e| e.file_name().to_string_lossy().starts_with("python")) + .find(|e| { + e.file_type().is_ok_and(|ft| ft.is_dir()) && // Ensure it's a directory + e.file_name().to_string_lossy().starts_with("python") + }) .map(|e| e.path().join("site-packages")) } + + #[cfg(windows)] + fn find_site_packages(prefix: &Path) -> Option { + Some(prefix.join("Lib").join("site-packages")) + } } impl fmt::Display for PythonEnvironment { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(f, "Python path: {}", self.python_path.display())?; writeln!(f, "Sys prefix: {}", self.sys_prefix.display())?; writeln!(f, "Sys paths:")?; for path in &self.sys_path { @@ -129,3 +214,334 @@ impl fmt::Display for PythonEnvironment { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::tempdir; + + fn create_mock_venv(dir: &Path, version: Option<&str>) -> PathBuf { + let prefix = dir.to_path_buf(); + + #[cfg(not(windows))] + { + let bin_dir = prefix.join("bin"); + fs::create_dir_all(&bin_dir).unwrap(); + fs::write(bin_dir.join("python"), "").unwrap(); // Create dummy executable + let lib_dir = prefix.join("lib"); + fs::create_dir_all(&lib_dir).unwrap(); + let py_version_dir = lib_dir.join(version.unwrap_or("python3.9")); + fs::create_dir_all(&py_version_dir).unwrap(); + fs::create_dir_all(py_version_dir.join("site-packages")).unwrap(); + } + + #[cfg(windows)] + { + let bin_dir = prefix.join("Scripts"); + fs::create_dir_all(&bin_dir).unwrap(); + fs::write(bin_dir.join("python.exe"), "").unwrap(); // Create dummy executable + let lib_dir = prefix.join("Lib"); + fs::create_dir_all(&lib_dir).unwrap(); + fs::create_dir_all(lib_dir.join("site-packages")).unwrap(); + } + + prefix + } + + struct VirtualEnvGuard<'a> { + key: &'a str, + original_value: Option, + } + + impl<'a> VirtualEnvGuard<'a> { + fn set(key: &'a str, value: &str) -> Self { + let original_value = env::var(key).ok(); + env::set_var(key, value); + Self { + key, + original_value, + } + } + + fn clear(key: &'a str) -> Self { + let original_value = env::var(key).ok(); + env::remove_var(key); + Self { + key, + original_value, + } + } + } + + impl Drop for VirtualEnvGuard<'_> { + fn drop(&mut self) { + if let Some(ref val) = self.original_value { + env::set_var(self.key, val); + } else { + env::remove_var(self.key); + } + } + } + + #[test] + fn test_explicit_venv_path_found() { + let project_dir = tempdir().unwrap(); + let venv_dir = tempdir().unwrap(); + let venv_prefix = create_mock_venv(venv_dir.path(), None); + + let env = PythonEnvironment::new(project_dir.path(), Some(venv_prefix.to_str().unwrap())) + .expect("Should find environment with explicit path"); + + assert_eq!(env.sys_prefix, venv_prefix); + + #[cfg(not(windows))] + { + assert!(env.python_path.ends_with("bin/python")); + assert!(env.sys_path.contains(&venv_prefix.join("bin"))); + assert!(env + .sys_path + .contains(&venv_prefix.join("lib/python3.9/site-packages"))); + } + + #[cfg(windows)] + { + assert!(env.python_path.ends_with("Scripts\\python.exe")); + assert!(env.sys_path.contains(&venv_prefix.join("Scripts"))); + assert!(env + .sys_path + .contains(&venv_prefix.join("Lib").join("site-packages"))); + } + } + + #[test] + fn test_explicit_venv_path_invalid_falls_through_to_project_venv() { + let project_dir = tempdir().unwrap(); + let project_venv_prefix = create_mock_venv(&project_dir.path().join(".venv"), None); + + // Clear VIRTUAL_ENV just in case + let _guard = VirtualEnvGuard::clear("VIRTUAL_ENV"); + + // Provide an invalid explicit path + let invalid_path = project_dir.path().join("non_existent_venv"); + let env = PythonEnvironment::new(project_dir.path(), Some(invalid_path.to_str().unwrap())) + .expect("Should fall through to project .venv"); + + // Should have found the one in the project dir + assert_eq!(env.sys_prefix, project_venv_prefix); + } + + #[test] + fn test_virtual_env_variable_found() { + let project_dir = tempdir().unwrap(); + let venv_dir = tempdir().unwrap(); + let venv_prefix = create_mock_venv(venv_dir.path(), None); + + let _guard = VirtualEnvGuard::set("VIRTUAL_ENV", venv_prefix.to_str().unwrap()); + + let env = PythonEnvironment::new(project_dir.path(), None) + .expect("Should find environment via VIRTUAL_ENV"); + + assert_eq!(env.sys_prefix, venv_prefix); + + #[cfg(not(windows))] + assert!(env.python_path.ends_with("bin/python")); + + #[cfg(windows)] + assert!(env.python_path.ends_with("Scripts\\python.exe")); + } + + #[test] + fn test_explicit_path_overrides_virtual_env() { + let project_dir = tempdir().unwrap(); + let venv1_dir = tempdir().unwrap(); + let venv1_prefix = create_mock_venv(venv1_dir.path(), None); // Set by VIRTUAL_ENV + let venv2_dir = tempdir().unwrap(); + let venv2_prefix = create_mock_venv(venv2_dir.path(), None); // Set by explicit path + + let _guard = VirtualEnvGuard::set("VIRTUAL_ENV", venv1_prefix.to_str().unwrap()); + + let env = PythonEnvironment::new( + project_dir.path(), + Some(venv2_prefix.to_str().unwrap()), // Explicit path + ) + .expect("Should find environment via explicit path"); + + assert_eq!( + env.sys_prefix, venv2_prefix, + "Explicit path should take precedence" + ); + } + + #[test] + fn test_project_venv_found() { + let project_dir = tempdir().unwrap(); + let venv_prefix = create_mock_venv(&project_dir.path().join(".venv"), None); + + // Ensure VIRTUAL_ENV is not set + let _guard = VirtualEnvGuard::clear("VIRTUAL_ENV"); + + let env = PythonEnvironment::new(project_dir.path(), None) + .expect("Should find environment in project .venv"); + + assert_eq!(env.sys_prefix, venv_prefix); + } + + #[test] + fn test_project_venv_priority() { + let project_dir = tempdir().unwrap(); + // Create multiple potential venvs + let dot_venv_prefix = create_mock_venv(&project_dir.path().join(".venv"), None); + let _venv_prefix = create_mock_venv(&project_dir.path().join("venv"), None); // Should be ignored if .venv found first + + let _guard = VirtualEnvGuard::clear("VIRTUAL_ENV"); + + let env = + PythonEnvironment::new(project_dir.path(), None).expect("Should find environment"); + + // Asserts it finds .venv because it's checked first in the loop + assert_eq!(env.sys_prefix, dot_venv_prefix); + } + + #[test] + #[ignore = "Relies on system python being available and having standard layout"] + fn test_system_python_fallback() { + let project_dir = tempdir().unwrap(); + + // Ensure no explicit path, no VIRTUAL_ENV, no project venvs + let _guard = VirtualEnvGuard::clear("VIRTUAL_ENV"); + // We don't create any venvs in project_dir + + // This test assumes `which python` works and points to a standard layout + let system_env = PythonEnvironment::new(project_dir.path(), None); + + assert!( + system_env.is_some(), + "Should fall back to system python if available" + ); + + if let Some(env) = system_env { + // Basic checks - exact paths depend heavily on the test environment + assert!(env.python_path.exists()); + assert!(env.sys_prefix.exists()); + assert!(!env.sys_path.is_empty()); + assert!(env.sys_path[0].exists()); // Should contain the bin/Scripts dir + } + } + + #[test] + fn test_no_python_found() { + let project_dir = tempdir().unwrap(); + + // Ensure no explicit path, no VIRTUAL_ENV, no project venvs + let _guard = VirtualEnvGuard::clear("VIRTUAL_ENV"); + + // To *ensure* system fallback fails, we'd need to manipulate PATH, + // which is tricky and platform-dependent. Instead, we test the scenario + // where `from_system_python` *would* be called but returns None. + // We can simulate this by ensuring `which("python")` fails. + // For this unit test, let's assume a scenario where all checks fail. + // A more direct test would mock `which`, but that adds complexity. + + // Let's simulate the *call* path assuming `from_system_python` returns None. + // We can't easily force `which` to fail here without PATH manipulation. + // So, this test mainly verifies that if all preceding steps fail, + // the result of `from_system_python` (which *could* be None) is returned. + // If system python *is* found, this test might incorrectly pass if not ignored. + // A better approach might be needed if strict testing of "None" is required. + + // For now, let's assume a setup where system python isn't found by `which`. + // This test is inherently flaky if system python *is* on the PATH. + // Consider ignoring it or using mocking for `which` in a real-world scenario. + + // If system python IS found, this test doesn't truly test the "None" case. + // If system python IS NOT found, it tests the final `None` return. + let env = PythonEnvironment::new(project_dir.path(), None); + + // This assertion depends on whether system python is actually found or not. + // assert!(env.is_none(), "Expected no environment to be found"); + // Given the difficulty, let's skip asserting None directly unless we mock `which`. + println!( + "Test 'test_no_python_found' ran. Result depends on system state: {:?}", + env + ); + } + + #[test] + #[cfg(not(windows))] // Test specific site-packages structure on Unix-like systems + fn test_unix_site_packages_discovery() { + let venv_dir = tempdir().unwrap(); + let prefix = venv_dir.path(); + let bin_dir = prefix.join("bin"); + fs::create_dir_all(&bin_dir).unwrap(); + fs::write(bin_dir.join("python"), "").unwrap(); + let lib_dir = prefix.join("lib"); + fs::create_dir_all(&lib_dir).unwrap(); + // Create two python version dirs, ensure it picks one + let py_version_dir1 = lib_dir.join("python3.8"); + fs::create_dir_all(&py_version_dir1).unwrap(); + fs::create_dir_all(py_version_dir1.join("site-packages")).unwrap(); + let py_version_dir2 = lib_dir.join("python3.10"); + fs::create_dir_all(&py_version_dir2).unwrap(); + fs::create_dir_all(py_version_dir2.join("site-packages")).unwrap(); + + let env = PythonEnvironment::from_venv_prefix(prefix).unwrap(); + + // It should find *a* site-packages dir. The exact one depends on read_dir order. + let found_site_packages = env.sys_path.iter().any(|p| p.ends_with("site-packages")); + assert!( + found_site_packages, + "Should have found a site-packages directory" + ); + + // Ensure it contains the bin dir as well + assert!(env.sys_path.contains(&prefix.join("bin"))); + } + + #[test] + #[cfg(windows)] + fn test_windows_site_packages_discovery() { + let venv_dir = tempdir().unwrap(); + let prefix = venv_dir.path(); + let bin_dir = prefix.join("Scripts"); + fs::create_dir_all(&bin_dir).unwrap(); + fs::write(bin_dir.join("python.exe"), "").unwrap(); + let lib_dir = prefix.join("Lib"); + fs::create_dir_all(&lib_dir).unwrap(); + let site_packages = lib_dir.join("site-packages"); + fs::create_dir_all(&site_packages).unwrap(); // Create the actual dir + + let env = PythonEnvironment::from_venv_prefix(prefix).unwrap(); + + assert!(env.sys_path.contains(&prefix.join("Scripts"))); + assert!( + env.sys_path.contains(&site_packages), + "Should have found Lib/site-packages" + ); + } + + #[test] + fn test_from_venv_prefix_returns_none_if_dir_missing() { + let dir = tempdir().unwrap(); + // Don't create the venv structure + let result = PythonEnvironment::from_venv_prefix(dir.path()); + assert!(result.is_none()); + } + + #[test] + fn test_from_venv_prefix_returns_none_if_binary_missing() { + let dir = tempdir().unwrap(); + let prefix = dir.path(); + // Create prefix dir but not the binary + fs::create_dir_all(prefix).unwrap(); + + #[cfg(not(windows))] + fs::create_dir_all(prefix.join("bin")).unwrap(); + + #[cfg(windows)] + fs::create_dir_all(prefix.join("Scripts")).unwrap(); + + let result = PythonEnvironment::from_venv_prefix(prefix); + assert!(result.is_none()); + } +} diff --git a/crates/djls-server/src/server.rs b/crates/djls-server/src/server.rs index e405e83..fcd14dd 100644 --- a/crates/djls-server/src/server.rs +++ b/crates/djls-server/src/server.rs @@ -159,6 +159,7 @@ impl LanguageServer for DjangoLanguageServer { let project_arc = Arc::clone(&self.project); let client = self.client.clone(); + let settings_arc = Arc::clone(&self.settings); let task = move || async move { let mut project_guard = project_arc.write().await; if let Some(project) = project_guard.as_mut() { @@ -173,7 +174,21 @@ impl LanguageServer for DjangoLanguageServer { ) .await; - match project.initialize() { + let venv_path = { + let settings = settings_arc.read().await; + settings.venv_path().map(|s| s.to_string()) + }; + + if let Some(ref path) = venv_path { + client + .log_message( + MessageType::INFO, + &format!("Using virtual environment from config: {}", path), + ) + .await; + } + + match project.initialize(venv_path.as_deref()) { Ok(()) => { client .log_message(