Repository navigation
Conversation
zaniebot
changed the title
uv runuv run
zaniebot
added
priority:3
16a587a to
75612b1
Compare
Merging this PR will not alter performance
|
75612b1 to
f013569
Compare
zaniebot
added
the
build:skip-release
f013569 to
04ff844
Compare
04ff844 to
2b874d5
Compare
![astral-automations-bot[bot]](https://avatars.githubusercontent.com/in/4307167?s=60&v=4){kind=small}
| for resource_limit in run_resource_limits { | ||
| resource_limit.apply().with_context(|| { |
There was a problem hiding this comment.
[P2] Apply the new resource limits in the child process
apply() changes uv's own limits before .spawn(). If uv's address space already exceeds the requested RLIMIT_AS, spawning a small command can fail with ENOMEM; similarly, RLIMIT_CPU counts CPU time uv already spent preparing the environment. Isolated Linux probes confirmed both failure modes. The test's 1 PiB address-space limit avoids this problem. Validate limits in the parent, apply them in the child's pre-exec path, and cover a realistic memory limit.
| fn u64_to_rlim_t(value: u64) -> Option<rlim_t> { | ||
| rlim_t::try_from(value).ok() |
There was a problem hiding this comment.
[P2] Handle the platform-dependent conversion lint
On Linux and macOS, rlim_t is u64, so this triggers clippy::useless_conversion. An isolated check of the extracted helper with that alias reproduced the warning; the Linux CI job denies warnings. Mirror the #[expect(clippy::useless_conversion)] on rlim_t_to_u64 immediately above, retaining the checked conversion needed on signed platforms.
| #[attr_added_in("0.12.2")] | ||
| pub const UV_RUN_RLIMIT_AS: &'static str = "UV_RUN_RLIMIT_AS"; |
There was a problem hiding this comment.
[P3] Record the actual introduction version for the new variables
All five new variables claim support since 0.12.2, and the reference generator renders this metadata verbatim. However, the available uv 0.12.13 still ignores UV_RUN_RLIMIT_CPU=60, leaving the child's limit unlimited. This advertises support in versions that silently leave limits unset. Update all five annotations to the release introducing this feature.
| .arg(python) | ||
| .arg("-c") | ||
| .arg("import resource; print(resource.getrlimit(resource.RLIMIT_NPROC)[0])") | ||
| .env(EnvVars::UV_RUN_RLIMIT_NPROC, "4096"); |
There was a problem hiding this comment.
[P2] Use a process limit below typical macOS hard limits
This success test assumes the inherited hard RLIMIT_NPROC is at least 4096. macOS installations can have a hard limit of 2666, in which case ResourceLimit::prepare correctly returns ExceedsHardLimit before Python runs. Use a smaller soft limit, such as 128, and update the snapshot so this test exercises the override without requiring an unusually high host limit.
| #[test] | ||
| fn run_resource_limit_overrides() { | ||
| let context = uv_test::test_context!("3.12"); |
There was a problem hiding this comment.
[P3] Gate the new Python-dependent tests
Unlike run_memory_limit_applies_only_to_child, the other four new tests lack a test-python gate. The containing module is gated only on Unix, and test_context!("3.12") requires that interpreter during setup. These cases therefore fail when Python-dependent tests are disabled and Python 3.12 is unavailable. Add the feature gate to the new cases, or gate the resource-limit module.
| process.pre_exec(move || { | ||
| for limit in &resource_limits { | ||
| limit.apply()?; |
There was a problem hiding this comment.
[P3] Retain configured limits in spawn-error diagnostics
prepare() checks representability and the inherited hard limit, but setrlimit can still fail—for example, macOS rejects RLIMIT_NOFILE above kern.maxfilesperproc even with an unlimited hard limit. Returning only errno here routes that failure through the generic Failed to spawn context, losing the environment variable and requested value that the previous implementation reported. Retain the configured names and values in the parent and include them in spawn-error context, while keeping the callback allocation-free.
| #[test] | ||
| fn run_resource_limit_override_invalid() { | ||
| let context = uv_test::test_context!("3.12"); | ||
| let python = &context.python_versions[0].1; |
There was a problem hiding this comment.
[P3] Remove duplicated generic validation scenarios
This test repeats run_open_file_limit_override_invalid through the same parse_integer_environment_variable::<u64> path. Likewise, the new CPU hard-limit failure test repeats the shared ResourceLimit::prepare branch already covered by run_open_file_limit_override_exceeds_hard_limit. The combined success test already verifies CPU-variable wiring. Remove these two CPU error scenarios and retain the existing NOFILE cases; this avoids maintaining duplicate integration setup and diagnostic snapshots without losing coverage of a distinct validation path.
| /// Sets the soft CPU-time limit, in seconds, for commands executed by `uv run`. | ||
| #[attr_added_in("next release")] | ||
| pub const UV_RUN_RLIMIT_CPU: &'static str = "UV_RUN_RLIMIT_CPU"; |
There was a problem hiding this comment.
[P3] Document the Unix restriction for all new limits
The descriptions for UV_RUN_RLIMIT_CPU, UV_RUN_RLIMIT_CORE, and UV_RUN_RLIMIT_FSIZE omit the Unix-only restriction stated for the other limits. These descriptions populate the environment-variable reference, but parsing and application are gated by #[cfg(unix)], so the variables silently do nothing on Windows. Add the same platform note to these three entries.
| if hard != RLIM_INFINITY && target_rlim > hard { | ||
| return Err(OpenFileLimitError::ExceedsHardLimit { target, hard }); |
There was a problem hiding this comment.
[P3] Remove the obsolete open-file error variant
Deleting this function removes the only construction of OpenFileLimitError::ExceedsHardLimit. The remaining adjust_open_file_limit path clamps its target, while configured-limit validation now uses ResourceLimitError::ExceedsHardLimit. Remove the obsolete variant alongside this function so the exported error type reflects the failures its operations can actually produce.
| pub fn new(environment_variable: &'static str, resource: RunResource, value: u64) -> Self { | ||
| Self { | ||
| environment_variable, | ||
| resource, | ||
| value, |
There was a problem hiding this comment.
[P3] Keep resource identity and its environment name together
This constructor accepts the environment-variable name and kernel resource independently, while prepare() derives the diagnostic resource name from that string. The sole caller currently forwards matching pairs from SUPPORTED_RESOURCE_LIMITS, but the public API makes callers responsible for keeping the reported resource aligned with the resource actually applied, including choosing the nix/rustix backend. Represent supported resources with a closed resource-kind enum or an opaque descriptor that owns both mappings, and construct limits from that resource plus the value. This keeps the pairing invariant inside uv-unix without changing the prepared-limit boundary.
This branch was successfully deployed
2 participants
uv runcurrently only exposesRLIMIT_NOFILE(astral-sh#20926), leaving common Unix resource controls unavailable for commands it launches.Add
UV_RUN_RLIMIT_CPU,UV_RUN_RLIMIT_AS,UV_RUN_RLIMIT_FSIZE,UV_RUN_RLIMIT_NPROC, andUV_RUN_RLIMIT_COREalongsideUV_RUN_RLIMIT_NOFILE. Apply configured soft limits immediately before spawning the command, preserve their hard limits, and accept 64-bit resource values.