Skip to content

fix(builtins): getopts honors local OPTIND/OPTARG #2591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chaliy merged 1 commit into from
Oct 7, 2026
Merged

fix(builtins): getopts honors local OPTIND/OPTARG #2591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 46 additions & 39 deletions crates/bashkit/src/interpreter/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9121,6 +9121,20 @@ impl Interpreter {
self.apply_redirections(result, redirects).await
}

/// `getopts` unsets OPTARG for options without one; a `local OPTARG`
/// stays local (emptied) so the caller's value is untouched.
fn unset_optarg(&mut self) {
if self
.call_stack
.iter()
.any(|frame| frame.locals.contains_key("OPTARG"))
{
self.set_variable("OPTARG".to_string(), String::new());
} else {
self.vars_mut().remove("OPTARG");
}
}

/// Usage: `getopts optstring name [args...]`
///
/// Parses options from positional params (or `args`).
Expand Down Expand Up @@ -9153,16 +9167,13 @@ impl Interpreter {
};

// Get current OPTIND (1-based index into args)
let optind: usize = self
.scoped
.variables
.get("OPTIND")
.and_then(|v| v.parse().ok())
.unwrap_or(1);
// OPTIND and OPTARG follow dynamic scope (`local OPTIND` in a
// function); an empty or unset OPTIND starts over at 1.
let optind: usize = self.expand_variable("OPTIND").parse().unwrap_or(1);

// Check if we're past the end
if optind < 1 || optind > parse_args.len() {
self.insert_variable_checked(varname.clone(), "?".to_string());
self.set_variable(varname.clone(), "?".to_string());
return Ok(ExecResult {
stdout: crate::StreamData::new(),
stderr: crate::StreamData::new(),
Expand All @@ -9176,10 +9187,9 @@ impl Interpreter {

// Check if this is an option (starts with -)
if !current_arg.starts_with('-') || current_arg == "-" || current_arg == "--" {
self.insert_variable_checked(varname.clone(), "?".to_string());
self.set_variable(varname.clone(), "?".to_string());
if current_arg == "--" {
self.vars_mut()
.insert("OPTIND".to_string(), (optind + 1).to_string());
self.set_variable("OPTIND".to_string(), (optind + 1).to_string());
}
return Ok(ExecResult {
stdout: crate::StreamData::new(),
Expand All @@ -9199,10 +9209,9 @@ impl Interpreter {

if char_idx >= opt_chars.len() {
// Should not happen, but advance
self.vars_mut()
.insert("OPTIND".to_string(), (optind + 1).to_string());
self.set_variable("OPTIND".to_string(), (optind + 1).to_string());
self.getopts_char_idx = 0;
self.insert_variable_checked(varname.clone(), "?".to_string());
self.set_variable(varname.clone(), "?".to_string());
return Ok(ExecResult {
stdout: crate::StreamData::new(),
stderr: crate::StreamData::new(),
Expand All @@ -9219,36 +9228,31 @@ impl Interpreter {
// Check if this option is in the optstring
if let Some(pos) = spec.find(opt_char) {
let needs_arg = spec.get(pos + 1..pos + 2) == Some(":");
self.insert_variable_checked(varname.clone(), opt_char.to_string());
self.set_variable(varname.clone(), opt_char.to_string());

if needs_arg {
// Option needs an argument
if char_idx + 1 < opt_chars.len() {
// Rest of current arg is the argument
let arg_val: String = opt_chars[char_idx + 1..].iter().collect();
self.insert_variable_checked("OPTARG".to_string(), arg_val);
self.vars_mut()
.insert("OPTIND".to_string(), (optind + 1).to_string());
self.set_variable("OPTARG".to_string(), arg_val);
self.set_variable("OPTIND".to_string(), (optind + 1).to_string());
self.getopts_char_idx = 0;
} else if optind < parse_args.len() {
// Next arg is the argument
self.vars_mut()
.insert("OPTARG".to_string(), parse_args[optind].clone());
self.vars_mut()
.insert("OPTIND".to_string(), (optind + 2).to_string());
self.set_variable("OPTARG".to_string(), parse_args[optind].clone());
self.set_variable("OPTIND".to_string(), (optind + 2).to_string());
self.getopts_char_idx = 0;
} else {
// Missing argument
self.vars_mut().remove("OPTARG");
self.vars_mut()
.insert("OPTIND".to_string(), (optind + 1).to_string());
self.unset_optarg();
self.set_variable("OPTIND".to_string(), (optind + 1).to_string());
self.getopts_char_idx = 0;
if silent {
self.insert_variable_checked(varname.clone(), ":".to_string());
self.vars_mut()
.insert("OPTARG".to_string(), opt_char.to_string());
self.set_variable(varname.clone(), ":".to_string());
self.set_variable("OPTARG".to_string(), opt_char.to_string());
} else {
self.insert_variable_checked(varname.clone(), "?".to_string());
self.set_variable(varname.clone(), "?".to_string());
let mut result = ExecResult::ok(String::new());
result.stderr = format!(
"bash: getopts: option requires an argument -- '{}'\n",
Expand All @@ -9261,34 +9265,33 @@ impl Interpreter {
}
} else {
// No argument needed
self.vars_mut().remove("OPTARG");
self.unset_optarg();
if char_idx + 1 < opt_chars.len() {
// More chars in this arg
// More chars in this arg: OPTIND stays on it, as in bash.
self.set_variable("OPTIND".to_string(), optind.to_string());
self.getopts_char_idx = char_idx + 1;
} else {
// Move to next arg
self.vars_mut()
.insert("OPTIND".to_string(), (optind + 1).to_string());
self.set_variable("OPTIND".to_string(), (optind + 1).to_string());
self.getopts_char_idx = 0;
}
}
} else {
// Unknown option
self.vars_mut().remove("OPTARG");
self.unset_optarg();
if char_idx + 1 < opt_chars.len() {
self.set_variable("OPTIND".to_string(), optind.to_string());
self.getopts_char_idx = char_idx + 1;
} else {
self.vars_mut()
.insert("OPTIND".to_string(), (optind + 1).to_string());
self.set_variable("OPTIND".to_string(), (optind + 1).to_string());
self.getopts_char_idx = 0;
}

if silent {
self.insert_variable_checked(varname.clone(), "?".to_string());
self.vars_mut()
.insert("OPTARG".to_string(), opt_char.to_string());
self.set_variable(varname.clone(), "?".to_string());
self.set_variable("OPTARG".to_string(), opt_char.to_string());
} else {
self.insert_variable_checked(varname.clone(), "?".to_string());
self.set_variable(varname.clone(), "?".to_string());
let mut result = ExecResult::ok(String::new());
result.stderr = format!("bash: getopts: illegal option -- '{}'\n", opt_char).into();
result = self.apply_redirections(result, redirects).await?;
Expand Down Expand Up @@ -10611,6 +10614,10 @@ impl Interpreter {
if resolved == "SRANDOM" {
return;
}
// Assigning OPTIND restarts `getopts` within an option group.
if resolved == "OPTIND" {
self.getopts_char_idx = 0;
}
if resolved == "RANDOM" {
self.random_state
.store(value.parse::<u32>().unwrap_or(0), Ordering::Relaxed);
Expand Down
43 changes: 43 additions & 0 deletions crates/bashkit/tests/spec_cases/bash/getopts-local-optind.test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
### getopts_local_optind_in_function
parse() {
local OPTIND opt verbose=0 out=""
while getopts ":vo:h" opt; do
case $opt in
v) verbose=1 ;;
o) out=$OPTARG ;;
:) echo "missing arg for -$OPTARG"; return 2 ;;
\?) echo "invalid -$OPTARG"; return 2 ;;
esac
done
shift $((OPTIND - 1))
echo "verbose=$verbose out=$out rest=$*"
}
parse -v -o out.txt a b
parse -vo x
parse -x
parse -o
getopts "a" o -a; echo "top: $o OPTIND=$OPTIND"
### expect
verbose=1 out=out.txt rest=a b
verbose=1 out=x rest=
invalid -x
missing arg for -o
top: a OPTIND=2
### end

### getopts_optind_reset_restarts_group
getopts "ab" o -ab; echo "$o $OPTIND"
OPTIND=1
getopts "ab" o -ba; echo "$o $OPTIND"
### expect
a 1
b 1
### end

### getopts_local_optarg
f() { local OPTARG; getopts "x:" o -x val; echo "in: $OPTARG"; }
OPTARG=global; f; echo "out: $OPTARG"
### expect
in: val
out: global
### end
1 change: 1 addition & 0 deletions knowledge/log.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* **Fix**: Quoted or escaped glob characters are literal everywhere. `echo a\*`, `a"*"` and `a'?'` no longer glob; `case`/`[[ == ]]` patterns match `"$x"`, `'a*'`, `a\*` and `\[s\]` literally; `"*"zz*` with no match prints `*zz*` (escapes no longer leak); `"$x"zz*` no longer reads `$xzz`; `'a'$x` expands and `'a'*` globs; `a{1,2}"*"` still brace-expands. The lexer escapes quoted glob characters in mixed words (the existing `QuotedGlobWord` convention, now with quote markers); `expand_word` returns unescaped text for non-glob uses; patterns use `expand_pattern_word`. Known gap: data from an unquoted expansion inside such a word loses `\` before glob characters when nothing matches.
* **Feature**: `fmt` builtin, on the uutils `fmt` engine (MIT, vendored: Knuth-Plass line breaking, paragraphs, `-c`/`-t`/`-s`/`-u`/`-p`, `-w`/`-g`, `-WIDTH`). Width rules and error messages follow GNU. Line breaks can differ from GNU `fmt` on long paragraphs (L-FMT-001); GNU's GPL code was not ported. Step 4 of the level-up roadmap (missing commands). Spec: `fmt.test.sh`. See foundations/builtins "fmt".
* **Perf**: The CPython snapshot ships as xz (4.99 MB, was ~6.7 MB as gzip; decoded by `lzma-rs` in build.rs). The room went to preloading `urllib.request` (http.client, email), `tomllib`, `configparser` and `xml.etree.ElementTree`: their imports went from 0.1-0.47 s per call to ~0; `print(1)` unchanged at ~4.1 ms, first call ~20 ms. `doctest` works again (a `pdb` stand-in; `breakpoint()` prints a notice), `mailbox` dropped. See [CPython WebAssembly Runtime](runtimes/cpython-wasm.md#startup-is-the-product-constraint).
* **Fix**: `getopts` reads and writes `OPTIND`/`OPTARG` through dynamic scope, so the standard `local OPTIND` pattern in a function parses its options (it saw the global and parsed nothing, then `shift $((OPTIND-1))` failed). An unset or empty `OPTIND` starts at 1. `OPTIND` stays on an option group (`-ab`) until it is done, and assigning `OPTIND` restarts the group. Debian-oracle case `getopts-loop` now matches bash.
* **Feature**: `import requests`, `import httpx` and `import httpx2` work in CPython. They are bashkit's own compact implementations of the common API over the HTTP bridge, preloaded in the snapshot (import cost ~0 vs ~3.1 s for vendored upstream requests). http.client read timeouts now surface from `getresponse()`. L-CPY-003 updated, L-CPY-010 added. See [CPython WebAssembly Runtime](runtimes/cpython-wasm.md#requests-and-httpx-bashkits-own-modules).
* **Fix**: A backslash outside quotes keeps `$` and `` ` `` literal. `echo \$HOME` printed the home directory; it now prints `$HOME`, as do `a\$x`, `\${x}` and `x=\$y` (the lexer emits the same NUL literal marker as quoted `"\$"`). Found while working gap telemetry.
* **Fix**: `$'\x1e'` and `$'\x1f'` bytes (record/unit separators) are kept as data. They collided with the lexer's quote-boundary markers and were silently dropped, which broke the common `SEP=$'\x1f'` composite-key idiom for associative arrays (`v["$section"$'\x1f'"$key"]`). Decoded ANSI-C content now NUL-escapes those bytes like `$`. Found by gap telemetry (`config_ini_merge` replay printed nothing).
Expand Down
Loading