From 7d5bd0d6be4494019e75acaff1a60df282ec1d88 Mon Sep 17 00:00:00 2001 From: nibon7 Date: Sat, 16 Dec 2023 22:55:44 +0800 Subject: [PATCH] Allow `int` type as a valid limit value (#11346) # Description This PR allows `int` type as a valid limit value for `ulimit`, so there is no need to use `into string` to convert limit values in the tests. # User-Facing Changes N/A # Tests + Formatting Make sure you've run and fixed any issues with these commands: - [x] add `commands::ulimit::limit_set_invalid3` - [x] add `commands::ulimit::limit_set_invalid4` - [x] add `commands::ulimit::limit_set_invalid5` - [x] `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - [x] `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - [x] `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library --- crates/nu-command/src/platform/ulimit.rs | 90 +++++++++++++--------- crates/nu-command/tests/commands/ulimit.rs | 82 +++++++++++++++++--- 2 files changed, 126 insertions(+), 46 deletions(-) diff --git a/crates/nu-command/src/platform/ulimit.rs b/crates/nu-command/src/platform/ulimit.rs index bbdc861d15..d058393b84 100644 --- a/crates/nu-command/src/platform/ulimit.rs +++ b/crates/nu-command/src/platform/ulimit.rs @@ -4,7 +4,7 @@ use nu_protocol::{ ast::Call, engine::{Command, EngineState, Stack}, Category, Example, IntoPipelineData, PipelineData, Record, ShellError, Signature, Span, - Spanned, SyntaxShape, Type, Value, + SyntaxShape, Type, Value, }; use once_cell::sync::Lazy; @@ -331,13 +331,21 @@ fn fill_record( /// Set limits fn set_limits( - spanned_limit: &Spanned, + limit_value: &Value, res: &ResourceInfo, soft: bool, hard: bool, + call_span: Span, ) -> Result<(), ShellError> { let (mut soft_limit, mut hard_limit) = getrlimit(res.resource)?; - let new_limit = parse_limit(spanned_limit, res.multiplier, soft, soft_limit, hard_limit)?; + let new_limit = parse_limit( + limit_value, + res.multiplier, + soft, + soft_limit, + hard_limit, + call_span, + )?; if hard { hard_limit = new_limit; @@ -418,45 +426,55 @@ fn getrlimit(res: Resource) -> Result<(rlim_t, rlim_t), ShellError> { /// Parse user input fn parse_limit( - spanned_limit: &Spanned, + limit_value: &Value, multiplier: rlim_t, soft: bool, soft_limit: rlim_t, hard_limit: rlim_t, + call_span: Span, ) -> Result { - let limit = &spanned_limit.item; - let span = spanned_limit.span; - - if limit.eq("unlimited") { - Ok(RLIM_INFINITY) - } else if limit.eq("soft") { - if soft { - Ok(hard_limit) - } else { - Ok(soft_limit) - } - } else if limit.eq("hard") { - Ok(hard_limit) - } else { - let v = limit - .parse::() - .map_err(|e| ShellError::CantConvert { + match limit_value { + Value::Int { val, internal_span } => { + let value = rlim_t::try_from(*val).map_err(|e| ShellError::CantConvert { to_type: "rlim_t".into(), - from_type: "String".into(), - span, + from_type: "i64".into(), + span: *internal_span, help: Some(e.to_string()), })?; - let (value, overflow) = v.overflowing_mul(multiplier); - if overflow { - return Err(ShellError::OperatorOverflow { - msg: "Multiple overflow".into(), - span, - help: String::new(), - }); - } else { - Ok(value) + let (limit, overflow) = value.overflowing_mul(multiplier); + if overflow { + Ok(RLIM_INFINITY) + } else { + Ok(limit) + } } + Value::String { val, internal_span } => { + if val == "unlimited" { + Ok(RLIM_INFINITY) + } else if val == "soft" { + if soft { + Ok(hard_limit) + } else { + Ok(soft_limit) + } + } else if val == "hard" { + Ok(hard_limit) + } else { + return Err(ShellError::IncorrectValue { + msg: "Only unlimited, soft and hard are supported for strings".into(), + val_span: *internal_span, + call_span, + }); + } + } + _ => Err(ShellError::TypeMismatch { + err_message: format!( + "string or int required, you provide {}", + limit_value.get_type() + ), + span: limit_value.span(), + }), } } @@ -481,7 +499,7 @@ impl Command for ULimit { .switch("soft", "Sets soft resource limit", Some('S')) .switch("hard", "Sets hard resource limit", Some('H')) .switch("all", "Prints all current limits", Some('a')) - .optional("limit", SyntaxShape::String, "Limit value.") + .optional("limit", SyntaxShape::Any, "Limit value.") .category(Category::Platform); for res in RESOURCE_ARRAY.iter() { @@ -508,12 +526,12 @@ impl Command for ULimit { soft = true; } - if let Some(spanned_limit) = call.opt::>(engine_state, stack, 0)? { + if let Some(limit_value) = call.opt::(engine_state, stack, 0)? { let mut set_default_limit = true; for res in RESOURCE_ARRAY.iter() { if call.has_flag(res.name) { - set_limits(&spanned_limit, res, soft, hard)?; + set_limits(&limit_value, res, soft, hard, call.head)?; if set_default_limit { set_default_limit = false; @@ -524,7 +542,7 @@ impl Command for ULimit { // Set `RLIMIT_FSIZE` limit if no resource flag provided. if set_default_limit { let res = ResourceInfo::default(); - set_limits(&spanned_limit, &res, hard, soft)?; + set_limits(&limit_value, &res, hard, soft, call.head)?; } Ok(PipelineData::Empty) diff --git a/crates/nu-command/tests/commands/ulimit.rs b/crates/nu-command/tests/commands/ulimit.rs index d39ea44080..50bc532241 100644 --- a/crates/nu-command/tests/commands/ulimit.rs +++ b/crates/nu-command/tests/commands/ulimit.rs @@ -7,9 +7,9 @@ fn limit_set_soft1() { let actual = nu!( cwd: dirs.test(), pipeline( " - let soft = (ulimit -s | first | get soft | into string); + let soft = (ulimit -s | first | get soft); ulimit -s -H $soft; - let hard = (ulimit -s | first | get hard | into string); + let hard = (ulimit -s | first | get hard); $soft == $hard " )); @@ -24,9 +24,9 @@ fn limit_set_soft2() { let actual = nu!( cwd: dirs.test(), pipeline( " - let soft = (ulimit -s | first | get soft | into string); + let soft = (ulimit -s | first | get soft); ulimit -s -H soft; - let hard = (ulimit -s | first | get hard | into string); + let hard = (ulimit -s | first | get hard); $soft == $hard " )); @@ -41,9 +41,9 @@ fn limit_set_hard1() { let actual = nu!( cwd: dirs.test(), pipeline( " - let hard = (ulimit -s | first | get hard | into string); + let hard = (ulimit -s | first | get hard); ulimit -s $hard; - let soft = (ulimit -s | first | get soft | into string); + let soft = (ulimit -s | first | get soft); $soft == $hard " )); @@ -58,9 +58,9 @@ fn limit_set_hard2() { let actual = nu!( cwd: dirs.test(), pipeline( " - let hard = (ulimit -s | first | get hard | into string); + let hard = (ulimit -s | first | get hard); ulimit -s hard; - let soft = (ulimit -s | first | get soft | into string); + let soft = (ulimit -s | first | get soft); $soft == $hard " )); @@ -79,7 +79,7 @@ fn limit_set_invalid1() { match $hard { \"unlimited\" => { echo \"unlimited\" }, $x => { - let new = ($x + 1 | into string); + let new = $x + 1; ulimit -s $new } } @@ -96,6 +96,21 @@ fn limit_set_invalid1() { #[test] fn limit_set_invalid2() { Playground::setup("limit_set_invalid2", |dirs, _sandbox| { + let actual = nu!( + cwd: dirs.test(), + " + let val = -100; + ulimit -c $val + " + ); + + assert!(actual.err.contains("can't convert i64 to rlim_t")); + }); +} + +#[test] +fn limit_set_invalid3() { + Playground::setup("limit_set_invalid3", |dirs, _sandbox| { let actual = nu!( cwd: dirs.test(), " @@ -103,6 +118,53 @@ fn limit_set_invalid2() { " ); - assert!(actual.err.contains("Can't convert to rlim_t.")); + assert!(actual + .err + .contains("Only unlimited, soft and hard are supported for strings")); + }); +} + +#[test] +fn limit_set_invalid4() { + Playground::setup("limit_set_invalid4", |dirs, _sandbox| { + let actual = nu!( + cwd: dirs.test(), + " + ulimit -c 100.0 + " + ); + + assert!(actual.err.contains("string or int required")); + }); +} + +#[test] +fn limit_set_invalid5() { + use nix::sys::resource::rlim_t; + + let max = (rlim_t::MAX / 1024) + 1; + + Playground::setup("limit_set_invalid5", |dirs, _sandbox| { + let actual = nu!( + cwd: dirs.test(), pipeline( + format!( + " + let hard = (ulimit -c | first | get hard); + match $hard {{ + \"unlimited\" => {{ + ulimit -c -S 0; + ulimit -c {max}; + ulimit -c + | first + | get soft + }}, + _ => {{ + echo \"unlimited\" + }} + }} + ").as_str() + )); + + assert!(actual.out.eq("unlimited")); }); }