From 3be3d24b0d7872eec699a226b9cf650e4f8b2eb0 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Wed, 7 Oct 2026 19:53:27 +0000 Subject: [PATCH] fix(builtins): getopts honors local OPTIND/OPTARG getopts read and wrote the global OPTIND/OPTARG, so `local OPTIND` in a function made every option loop parse nothing. Use dynamic scope, start at 1 on empty OPTIND, keep OPTIND on an option group, and restart the group when OPTIND is assigned. Claude-Session: https://claude.ai/code/session_019aFikmptPc91Fj4N2iDXQA --- crates/bashkit/src/interpreter/mod.rs | 85 ++++++++++--------- .../bash/getopts-local-optind.test.sh | 43 ++++++++++ knowledge/log.md | 1 + 3 files changed, 90 insertions(+), 39 deletions(-) create mode 100644 crates/bashkit/tests/spec_cases/bash/getopts-local-optind.test.sh diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 97b8eddf5..664433657 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -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`). @@ -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(), @@ -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(), @@ -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(), @@ -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", @@ -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?; @@ -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::().unwrap_or(0), Ordering::Relaxed); diff --git a/crates/bashkit/tests/spec_cases/bash/getopts-local-optind.test.sh b/crates/bashkit/tests/spec_cases/bash/getopts-local-optind.test.sh new file mode 100644 index 000000000..1765adcea --- /dev/null +++ b/crates/bashkit/tests/spec_cases/bash/getopts-local-optind.test.sh @@ -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 diff --git a/knowledge/log.md b/knowledge/log.md index f34bc252e..417d17ffc 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -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).