diff --git a/backend/windmill-common/src/worker.rs b/backend/windmill-common/src/worker.rs index ebb0a723ea..ee8a389bb7 100644 --- a/backend/windmill-common/src/worker.rs +++ b/backend/windmill-common/src/worker.rs @@ -2512,6 +2512,42 @@ pub fn split_python_requirements>(requirements: T) -> Vec .collect() } +/// Byte offset of the comment marker, per pip's rule: a `#` at line start or preceded by +/// whitespace. A `#` elsewhere belongs to the requirement (`pkg @ https://h/p.whl#sha256=…`). +fn requirement_comment_start(line: &str) -> Option { + line.char_indices() + .find(|(i, c)| *c == '#' && (*i == 0 || line[..*i].ends_with(char::is_whitespace))) + .map(|(i, _)| i) +} + +/// The installable requirement carried by one lockfile line, or `None` for a comment, a +/// `-r`/`-e`/`--flag` directive, or a blank. +/// +/// Windmill installs a lockfile one entry at a time as a `uv pip install` argument, so +/// requirements-file syntax a file-level parser would absorb is an unparseable package name +/// here and has to be stripped first. +pub fn requirement_from_lockfile_line(line: &str) -> Option<&str> { + let requirement = match requirement_comment_start(line) { + Some(i) => &line[..i], + None => line, + } + .trim() + // Continuations are stripped, not joined: right for `--generate-hashes` locks, whose + // continued lines are `--hash=` flags this function drops, but a lock continuing onto a + // marker or extra would lose it. + .trim_end_matches('\\') + .trim_end(); + + (!requirement.is_empty() && !requirement.starts_with('-')).then_some(requirement) +} + +/// Whether a lockfile line continues onto the next one. The continued lines reach the +/// installer as entries of their own rather than being joined, so a caller that cares what +/// they carried — `--hash=` pins, for a `--generate-hashes` lock — has to say so itself. +pub fn lockfile_line_has_continuation(line: &str) -> bool { + line.trim_end().ends_with('\\') +} + #[derive(Eq, PartialEq, Clone, Copy, Default, Debug)] #[repr(u32)] pub enum PyVAlias { @@ -2630,6 +2666,52 @@ mod tests { ids.iter().map(|s| s.to_string()).collect() } + /// Fixtures are verbatim `uv pip compile` output (uv 0.11.28): split and inline + /// annotation styles, and `--generate-hashes`. + #[test] + fn test_requirement_from_lockfile_line() { + assert_eq!(requirement_from_lockfile_line(" # via httpx"), None); + assert_eq!(requirement_from_lockfile_line(" # via"), None); + assert_eq!(requirement_from_lockfile_line(" # anyio"), None); + assert_eq!( + requirement_from_lockfile_line(" # via -r .tmp/requirements.in"), + None + ); + assert_eq!( + requirement_from_lockfile_line("anyio==4.15.1 \\"), + Some("anyio==4.15.1") + ); + assert_eq!( + requirement_from_lockfile_line( + " --hash=sha256:6152fdbbf9a77fdec97731721bebf7c4c44f7c29b424b0065826173efc7 \\" + ), + None + ); + assert_eq!(requirement_from_lockfile_line("# py: 3.11"), None); + assert_eq!(requirement_from_lockfile_line("-r other.txt"), None); + assert_eq!( + requirement_from_lockfile_line("--index-url https://x"), + None + ); + assert_eq!(requirement_from_lockfile_line(" "), None); + assert_eq!( + requirement_from_lockfile_line("httpx==0.27.0"), + Some("httpx==0.27.0") + ); + assert_eq!( + requirement_from_lockfile_line("httpx==0.27.0 # via -r requirements.in"), + Some("httpx==0.27.0") + ); + // A `#` not preceded by whitespace is part of the requirement, not a comment. + assert_eq!( + requirement_from_lockfile_line("wmill @ https://h/wmill.whl#sha256=abc"), + Some("wmill @ https://h/wmill.whl#sha256=abc") + ); + + assert!(lockfile_line_has_continuation("anyio==4.15.1 \\")); + assert!(!lockfile_line_has_continuation("anyio==4.15.1")); + } + #[test] fn test_parse_job_oom_score_adj() { assert_eq!(parse_job_oom_score_adj(Some("300")), 300); diff --git a/backend/windmill-worker/src/python_executor.rs b/backend/windmill-worker/src/python_executor.rs index 9f363a7f90..6c82e4ab47 100644 --- a/backend/windmill-worker/src/python_executor.rs +++ b/backend/windmill-worker/src/python_executor.rs @@ -37,8 +37,9 @@ use windmill_common::{ scripts::ScriptLang, utils::calculate_hash, worker::{ - copy_dir_recursively, is_allowed_file_location, pad_string, split_python_requirements, - write_file, Connection, PyVAlias, PythonAnnotations, WORKER_CONFIG, + copy_dir_recursively, is_allowed_file_location, lockfile_line_has_continuation, pad_string, + requirement_from_lockfile_line, split_python_requirements, write_file, Connection, + PyVAlias, PythonAnnotations, WORKER_CONFIG, }, }; @@ -227,9 +228,9 @@ fn filter_pip_local_dependencies(lines: Vec) -> (Vec, Vec, compiled_deps: &[Regex]) -> (Vec, Vec) { - let (ignored, kept): (Vec, Vec) = lines - .into_iter() - .partition(|s| !s.starts_with('#') && compiled_deps.iter().any(|dep| dep.is_match(s))); + let (ignored, kept): (Vec, Vec) = lines.into_iter().partition(|s| { + !s.trim_start().starts_with('#') && compiled_deps.iter().any(|dep| dep.is_match(s)) + }); (kept, ignored) } @@ -2520,11 +2521,23 @@ pub async fn handle_python_reqs( // Find out if there is already cached dependencies // If so, skip them let mut in_cache = vec![]; + if requirements + .iter() + .any(|r| lockfile_line_has_continuation(r)) + { + tracing::warn!(workspace_id = %w_id, job_id = %job_id, "lockfile continues entries across lines; the continued lines are dropped"); + append_logs( + job_id, + w_id, + "\n[!] lockfile continues entries across lines and the continued lines are dropped: `--hash=` pins, extras and markers written that way do not apply\n".to_string(), + conn, + ) + .await; + } for req in &requirements { - // Ignore python version annotation backed into lockfile - if req.starts_with('#') || req.starts_with('-') || req.trim().is_empty() { + let Some(req) = requirement_from_lockfile_line(req) else { continue; - } + }; let py_prefix = &py_version.to_cache_dir(false); let venv_p = format!(