mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-09 16:05:42 +00:00
fix: ignore comments and continuations in python lockfiles (#11035)
* fix: ignore comments and continuations in python lockfiles Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVNKkWNGPuggLucMzFeoa1 * fix: warn when a lockfile's hash pins are not enforced Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVNKkWNGPuggLucMzFeoa1 * fix: state only what the continuation warning can know Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVNKkWNGPuggLucMzFeoa1 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
88c3ebdfc1
commit
90c4e1020a
@@ -2512,6 +2512,42 @@ pub fn split_python_requirements<T: AsRef<str>>(requirements: T) -> Vec<String>
|
||||
.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<usize> {
|
||||
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);
|
||||
|
||||
@@ -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<String>) -> (Vec<String>, Vec<String
|
||||
/// `(kept, ignored)`. A line is ignored when it is not a `#` comment and matches any of
|
||||
/// `compiled_deps`. Kept separate from config/regex loading so it can be unit-tested.
|
||||
fn filter_lines_by_deps(lines: Vec<String>, compiled_deps: &[Regex]) -> (Vec<String>, Vec<String>) {
|
||||
let (ignored, kept): (Vec<String>, Vec<String>) = lines
|
||||
.into_iter()
|
||||
.partition(|s| !s.starts_with('#') && compiled_deps.iter().any(|dep| dep.is_match(s)));
|
||||
let (ignored, kept): (Vec<String>, Vec<String>) = 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!(
|
||||
|
||||
Reference in New Issue
Block a user