From b110471f88e448853100e6f26c19027f0153305d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:58:47 +0000 Subject: [PATCH 1/9] shell: keep variables in one table with an export attribute --- docs/runtime/shell.mdx | 19 +++++ src/runtime/shell/EnvMap.rs | 112 ++++++++++++++++++++++++-- src/runtime/shell/EnvStr.rs | 20 ++++- src/runtime/shell/builtin/export.rs | 52 ++++++++---- src/runtime/shell/builtin/which.rs | 6 +- src/runtime/shell/interpreter.rs | 58 ++++++------- src/runtime/shell/states/Cmd.rs | 10 +-- src/runtime/shell/states/Expansion.rs | 7 +- src/runtime/shell/subproc.rs | 25 +++--- test/js/bun/shell/bunshell.test.ts | 112 ++++++++++++++++++++++++++ 10 files changed, 339 insertions(+), 82 deletions(-) diff --git a/docs/runtime/shell.mdx b/docs/runtime/shell.mdx index 4cc6ed402917..540814c28814 100644 --- a/docs/runtime/shell.mdx +++ b/docs/runtime/shell.mdx @@ -327,6 +327,24 @@ const foo = "bar123; rm -rf /tmp"; await $`FOO=${foo} bun -e 'console.log(process.env.FOO)'`; // bar123; rm -rf /tmp\n ``` +A variable from the environment is exported, and so is each name you pass to `export`. A child process receives the current value of every exported variable. A plain assignment to any other name stays in the shell: + +```js +import { $ } from "bun"; + +// FOO gets the export attribute and keeps its value +await $`FOO=foo; export FOO; bun -e 'console.log(process.env.FOO)'`; // foo\n + +// NODE_ENV comes from the environment, so the new value reaches `bun` +await $`NODE_ENV=production; bun -e 'console.log(process.env.NODE_ENV)'`.env({ + ...process.env, + NODE_ENV: "development", +}); // production\n + +// BAR is not exported +await $`BAR=bar; bun -e 'console.log(process.env.BAR)'`; // undefined\n +``` + ### Changing the environment variables By default, all commands use `process.env` as their environment variables. @@ -466,6 +484,7 @@ For cross-platform compatibility, Bun Shell implements a set of builtin commands - `rm`: remove files and directories - `echo`: print text - `pwd`: print the working directory +- `export`: export variables to child processes - `bun`: run bun in bun - `cat` - `touch` diff --git a/src/runtime/shell/EnvMap.rs b/src/runtime/shell/EnvMap.rs index 2ee1d91182a0..98182c284e4e 100644 --- a/src/runtime/shell/EnvMap.rs +++ b/src/runtime/shell/EnvMap.rs @@ -1,4 +1,4 @@ -use bun_collections::array_hash_map::{self, ArrayHashContext, ArrayHashMap, Iter}; +use bun_collections::array_hash_map::{self, ArrayHashContext, ArrayHashMap}; use crate::shell::EnvStr; @@ -6,8 +6,6 @@ pub(crate) struct EnvMap { map: EnvMapInner, } -pub(crate) type Iterator<'a> = Iter<'a, EnvStr, EnvStr>; - // Named `EnvMapInner` to avoid rustc confusing it with the unrelated mmap // `sys::c::MapType` / `sys::posix::MapType` in diagnostic suggestions. type EnvMapInner = ArrayHashMap; @@ -84,10 +82,6 @@ impl EnvMap { *result.value_ptr = val; } - pub(crate) fn iterator(&mut self) -> Iterator<'_> { - self.map.iterator() - } - pub(crate) fn iter(&self) -> impl core::iter::Iterator { self.map.keys().iter().zip(self.map.values()) } @@ -132,3 +126,107 @@ impl Drop for EnvMap { // map storage freed by its own Drop } } + +// `VarTable` slot attributes. Zero is an exported variable that has a value, +// so an inherited environment needs no marking. +const LOCAL: u8 = 1 << 0; +const UNSET: u8 = 1 << 1; + +const _: () = assert!(core::mem::size_of::() == 16); + +/// Every shell variable, one slot per name. The attribute bits of the slot say +/// if the name is exported and if it has a value. +pub(crate) struct VarTable { + map: EnvMap, +} + +impl VarTable { + /// Every entry of `env` becomes an exported variable. + pub(crate) fn from_exported(env: EnvMap) -> VarTable { + VarTable { map: env } + } + + pub(crate) fn memory_cost(&self) -> usize { + self.map.memory_cost() + } + + pub(crate) fn clone(&self) -> VarTable { + VarTable { + map: self.map.clone(), + } + } + + /// `NAME=value`. The name keeps its export attribute. + /// NOTE: This will `.ref()` value, like `EnvMap::insert`. + pub(crate) fn set(&mut self, name: EnvStr, value: EnvStr) { + let slot = self.map.map.get_or_put(name).expect("OOM"); + let attrs = if slot.found_existing { + slot.value_ptr.deref(); + slot.value_ptr.var_attrs() & !UNSET + } else { + name.ref_(); + LOCAL + }; + value.ref_(); + *slot.value_ptr = value.with_var_attrs(attrs); + } + + /// `export NAME=value`, or `export NAME` for `None`: the name keeps its value. + /// NOTE: This will `.ref()` value, like `EnvMap::insert`. + pub(crate) fn export(&mut self, name: EnvStr, value: Option) { + let slot = self.map.map.get_or_put(name).expect("OOM"); + if !slot.found_existing { + name.ref_(); + } + match value { + Some(value) => { + slot.value_ptr.deref(); + value.ref_(); + *slot.value_ptr = value.with_var_attrs(0); + } + None if slot.found_existing => { + let attrs = slot.value_ptr.var_attrs() & !LOCAL; + *slot.value_ptr = slot.value_ptr.with_var_attrs(attrs); + } + None => *slot.value_ptr = slot.value_ptr.with_var_attrs(UNSET), + } + } + + /// The value of `$NAME`. + /// NOTE: Make sure you deref the string when done! + pub(crate) fn get(&self, name: EnvStr) -> Option { + let slot = *self.map.map.get(&name)?; + if slot.var_attrs() & UNSET != 0 { + return None; + } + let value = slot.with_var_attrs(0); + value.ref_(); + Some(value) + } + + /// The value a child process inherits for `name`. + /// NOTE: Make sure you deref the string when done! + pub(crate) fn get_exported(&self, name: EnvStr) -> Option { + let slot = *self.map.map.get(&name)?; + if slot.var_attrs() != 0 { + return None; + } + slot.ref_(); + Some(slot) + } + + /// The environment of a child process: exported names that have a value. + pub(crate) fn exported(&self) -> impl core::iter::Iterator { + self.map.iter().filter(|(_, slot)| slot.var_attrs() == 0) + } + + /// Every exported name. The value is `None` until the name is assigned. + pub(crate) fn exported_names( + &self, + ) -> impl core::iter::Iterator)> + '_ { + self.map + .iter() + .filter(|(_, slot)| slot.var_attrs() & LOCAL == 0) + .map(|(name, slot)| (*name, (slot.var_attrs() & UNSET == 0).then_some(*slot))) + } +} diff --git a/src/runtime/shell/EnvStr.rs b/src/runtime/shell/EnvStr.rs index fc26e45cc520..d32d12691668 100644 --- a/src/runtime/shell/EnvStr.rs +++ b/src/runtime/shell/EnvStr.rs @@ -13,7 +13,8 @@ use super::ref_counted_str::RefCountedStr; /// Packed `u128` layout (LSB-first): /// - bits 0..48 : `ptr` (u48) -/// - bits 48..64 : `tag` (u16) +/// - bits 48..62 : `tag` +/// - bits 62..64 : variable attributes, zero outside a `VarTable` slot /// - bits 64..128 : `len` (usize) #[repr(transparent)] #[derive(Copy, Clone, Eq, PartialEq)] @@ -34,7 +35,9 @@ pub(crate) enum Tag { const PTR_MASK: u128 = (1u128 << 48) - 1; const TAG_SHIFT: u32 = 48; -const TAG_MASK: u128 = 0xFFFF; +const TAG_MASK: u128 = 0x3FFF; +const VAR_ATTRS_SHIFT: u32 = 62; +const VAR_ATTRS_MASK: u128 = 0b11; const LEN_SHIFT: u32 = 64; impl EnvStr { @@ -70,6 +73,19 @@ impl EnvStr { (self.0 >> LEN_SHIFT) as u64 as usize } + #[inline] + pub(super) fn var_attrs(self) -> u8 { + ((self.0 >> VAR_ATTRS_SHIFT) & VAR_ATTRS_MASK) as u8 + } + + #[inline] + pub(super) fn with_var_attrs(self, attrs: u8) -> EnvStr { + EnvStr( + (self.0 & !(VAR_ATTRS_MASK << VAR_ATTRS_SHIFT)) + | ((attrs as u128 & VAR_ATTRS_MASK) << VAR_ATTRS_SHIFT), + ) + } + #[inline] pub(crate) fn init_slice(str: &[u8]) -> EnvStr { if str.is_empty() { diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 8e03dac1ed17..40f66018a997 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -25,42 +25,66 @@ impl Export { // No args: print all exported vars. return Self::print_all(interp, cmd); } + Self::assign_prefixes(interp, cmd, argc); for i in 0..argc { let s = Builtin::of(interp, cmd).arg_bytes(i); if s.is_empty() { continue; } let (name, value) = match bun_core::strings::index_of_char_usize(s, b'=') { - Some(eq) => (&s[..eq], &s[eq + 1..]), - None => (s, &b""[..]), + Some(eq) => (&s[..eq], Some(&s[eq + 1..])), + None => (s, None), }; // The argv backing is freed when the Cmd retires, // so the key/value MUST be duplicated into ref-counted storage — - // `init_slice` here would leave dangling EnvStr in `export_env`. + // `init_slice` here would leave dangling EnvStr in the variable table. let label = EnvStr::dupe_ref_counted(name); - let val = EnvStr::dupe_ref_counted(value); - let shell = interp.as_cmd(cmd).base.shell; - // SAFETY: shell env outlives the Cmd node. - unsafe { (*shell).export_env.insert(label, val) }; + let val = value.map(EnvStr::dupe_ref_counted); + let shell = interp.as_cmd_mut(cmd).base.shell_mut(); + shell.vars.export(label, val); label.deref(); - val.deref(); + if let Some(val) = val { + val.deref(); + } } Builtin::done(interp, cmd, 0) } + /// `NAME=v export NAME`: the prefix on this command assigns the operand. + fn assign_prefixes(interp: &Interpreter, cmd: NodeId, argc: usize) { + let assigns = interp.as_cmd(cmd).ast_node().assigns; + for assign in assigns { + let is_operand = (0..argc).any(|i| { + let s = Builtin::of(interp, cmd).arg_bytes(i); + let eq = bun_core::strings::index_of_char_usize(s, b'=').unwrap_or(s.len()); + &s[..eq] == assign.label + }); + if !is_operand { + continue; + } + let name = EnvStr::init_slice(assign.label); + let shell = interp.as_cmd_mut(cmd).base.shell_mut(); + if let Some(value) = shell.cmd_local_env.get(name) { + shell.vars.set(name, value); + value.deref(); + } + } + } + fn print_all(interp: &Interpreter, cmd: NodeId) -> Yield { - let mut entries: Vec<(EnvStr, EnvStr)> = Builtin::shell(interp, cmd) - .export_env - .iter() - .map(|(k, v)| (*k, *v)) + let mut entries: Vec<(EnvStr, Option)> = Builtin::shell(interp, cmd) + .vars + .exported_names() .collect(); index_sort::sort_slice_by(&mut entries, |a, b| a.0.slice().cmp(b.0.slice())); let mut buf = Vec::new(); for (k, v) in &entries { buf.extend_from_slice(k.slice()); - buf.push(b'='); - buf.extend_from_slice(v.slice()); + if let Some(v) = v { + buf.push(b'='); + buf.extend_from_slice(v.slice()); + } buf.push(b'\n'); } diff --git a/src/runtime/shell/builtin/which.rs b/src/runtime/shell/builtin/which.rs index 27a3e606443c..aecb61ee29e9 100644 --- a/src/runtime/shell/builtin/which.rs +++ b/src/runtime/shell/builtin/which.rs @@ -203,10 +203,10 @@ struct SearchEnv { impl SearchEnv { fn load(interp: &Interpreter, cmd: NodeId) -> Self { let shell = Builtin::shell(interp, cmd); - // `EnvMap::get` refs the returned string; balance it. + // `get_exported` refs the returned string; balance it. let path_env = shell - .export_env - .get(EnvStr::init_slice(b"PATH")) + .vars + .get_exported(EnvStr::init_slice(b"PATH")) .map(|s| { let v = s.slice().to_vec(); s.deref(); diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index d1735dad6f74..1cd8f49a0ae2 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -33,7 +33,7 @@ use std::sync::atomic::{AtomicU32, Ordering}; use bun_sys::{self, Fd}; -pub(crate) use crate::shell::env_map::EnvMap; +pub(crate) use crate::shell::env_map::{EnvMap, VarTable}; use crate::shell::io::IO; use crate::shell::states::assigns::Assigns; use crate::shell::states::r#async::Async; @@ -561,9 +561,8 @@ impl Interpreter { root_shell: JsCell::new(ShellExecEnv { _buffered_stdout: Bufio::default(), _buffered_stderr: Bufio::default(), - shell_env: EnvMap::init(), cmd_local_env: EnvMap::init(), - export_env, + vars: VarTable::from_exported(export_env), __prev_cwd: cwd_arr.clone(), __cwd: cwd_arr, cwd_fd, @@ -1798,9 +1797,8 @@ pub(crate) fn throw_shell_err( pub(crate) struct ShellExecEnv { pub(crate) _buffered_stdout: Bufio, pub(crate) _buffered_stderr: Bufio, - pub(crate) shell_env: EnvMap, pub(crate) cmd_local_env: EnvMap, - pub(crate) export_env: EnvMap, + pub(crate) vars: VarTable, pub(crate) __prev_cwd: Vec, pub(crate) __cwd: Vec, pub(crate) cwd_fd: Fd, @@ -1840,9 +1838,8 @@ pub(crate) enum ShellExecEnvKind { impl ShellExecEnv { pub(crate) fn memory_cost(&self) -> usize { let mut size = core::mem::size_of::(); - size += self.shell_env.memory_cost(); size += self.cmd_local_env.memory_cost(); - size += self.export_env.memory_cost(); + size += self.vars.memory_cost(); size += self.__cwd.capacity(); size += self.__prev_cwd.capacity(); size += self._buffered_stderr.memory_cost(); @@ -1919,7 +1916,7 @@ impl ShellExecEnv { /// Heap-allocates a /// fresh env for a subshell/pipeline child: dups `cwd_fd`, clones - /// `shell_env`/`export_env`, gives it a fresh empty `cmd_local_env`, and + /// `vars`, gives it a fresh empty `cmd_local_env`, and /// borrows or owns buffered stdout/stderr per `kind` (subshell/pipeline /// borrow the parent's buffers so output bubbles up; cmd-subst owns). /// @@ -1959,9 +1956,8 @@ impl ShellExecEnv { let duped = Box::new(ShellExecEnv { _buffered_stdout: stdout, _buffered_stderr: stderr, - shell_env: self.shell_env.clone(), cmd_local_env: EnvMap::init(), - export_env: self.export_env.clone(), + vars: self.vars.clone(), __prev_cwd: self.__prev_cwd.clone(), __cwd: self.__cwd.clone(), cwd_fd: dupedfd, @@ -2003,9 +1999,8 @@ impl ShellExecEnv { } // EnvMap has a Drop impl; replace with fresh to free now and leave // valid state for any later `Drop` of the outer `Interpreter` box. - self.shell_env = EnvMap::init(); self.cmd_local_env = EnvMap::init(); - self.export_env = EnvMap::init(); + self.vars = VarTable::from_exported(EnvMap::init()); self.__cwd = Vec::new(); self.__prev_cwd = Vec::new(); closefd(self.cwd_fd); @@ -2037,7 +2032,7 @@ impl ShellExecEnv { /// Resolves `new_cwd_` (absolute, or relative to current `cwd()`), opens it /// with `O_DIRECTORY`, and on success rotates `__cwd`/`__prev_cwd`/`cwd_fd`. - /// Always writes `PWD` into `export_env`; `OLDPWD` is written only when + /// Always exports `PWD`; `OLDPWD` is written only when /// `!in_init` (the very first cwd has no meaningful "previous"). pub(crate) fn change_cwd_impl( &mut self, @@ -2125,22 +2120,22 @@ impl ShellExecEnv { // `init()` with `in_init = true`) still exports the resolved cwd. // Note: reshaped for borrowck — materialize the EnvStr (which // erases the slice lifetime into a packed ptr) before taking - // `&mut self.export_env`. + // `&mut self.vars`. use crate::shell::env_str::EnvStr; if !in_init { let oldpwd = EnvStr::init_slice(self.prev_cwd()); - self.export_env - .insert(EnvStr::init_slice(b"OLDPWD"), oldpwd); + self.vars + .export(EnvStr::init_slice(b"OLDPWD"), Some(oldpwd)); } let pwd = EnvStr::init_slice(self.cwd()); - self.export_env.insert(EnvStr::init_slice(b"PWD"), pwd); + self.vars.export(EnvStr::init_slice(b"PWD"), Some(pwd)); Ok(()) } - /// Routes `label = value` into an env map depending on where the - /// assignment appeared (`FOO=1 cmd` → cmd-local, bare `FOO=1` → shell). - /// NOTE: `EnvMap::insert` `.ref()`s the value, so callers should deref + /// Routes `label = value` depending on where the assignment appeared + /// (`FOO=1 cmd` → cmd-local, bare `FOO=1` → the variable table). + /// NOTE: both targets `.ref()` the value, so callers should deref /// `value` after the call. pub(crate) fn assign_var( &mut self, @@ -2150,13 +2145,13 @@ impl ShellExecEnv { ) { match assign_ctx { AssignCtx::Cmd => self.cmd_local_env.insert(label, value), - AssignCtx::Shell => self.shell_env.insert(label, value), + AssignCtx::Shell => self.vars.set(label, value), } } - /// Looks up `$HOME` (`$USERPROFILE` on Windows) in `shell_env` first, then - /// `export_env`. Falls back to `""` (or `/data/local/tmp` on Android) so - /// `cd` with no args / `~` expansion never sees a null. + /// Looks up `$HOME` (`$USERPROFILE` on Windows). Falls back to `""` (or + /// `/data/local/tmp` on Android) so `cd` with no args / `~` expansion + /// never sees a null. pub(crate) fn get_homedir(&self) -> crate::shell::env_str::EnvStr { use crate::shell::env_str::EnvStr; let key = if cfg!(windows) { @@ -2164,16 +2159,13 @@ impl ShellExecEnv { } else { EnvStr::init_slice(b"HOME") }; - self.shell_env - .get(key) - .or_else(|| self.export_env.get(key)) - .unwrap_or_else(|| { - EnvStr::init_slice(if bun_core::env::IS_ANDROID { - b"/data/local/tmp" - } else { - b"" - }) + self.vars.get(key).unwrap_or_else(|| { + EnvStr::init_slice(if bun_core::env::IS_ANDROID { + b"/data/local/tmp" + } else { + b"" }) + }) } } diff --git a/src/runtime/shell/states/Cmd.rs b/src/runtime/shell/states/Cmd.rs index aaf8a1f47af4..eb7c86a02e24 100644 --- a/src/runtime/shell/states/Cmd.rs +++ b/src/runtime/shell/states/Cmd.rs @@ -478,13 +478,11 @@ impl Cmd { // SAFETY: `shell_ptr` is the live env owned by this Cmd's scope chain. spawn_args.cwd = unsafe { &*shell_ptr }.cwd(); - // Fill env from export_env + cmd_local_env. + // Fill env from the exported variables + cmd_local_env. { - let env = interp.as_cmd_mut(this).base.shell_mut(); - let mut iter = env.export_env.iterator(); - spawn_args.fill_env::(&mut iter); - let mut iter = env.cmd_local_env.iterator(); - spawn_args.fill_env::(&mut iter); + let env = interp.as_cmd(this).base.shell(); + spawn_args.fill_env::(env.vars.exported()); + spawn_args.fill_env::(env.cmd_local_env.iter()); } // Resolve argv[0] via PATH (`bun_which::which`). diff --git a/src/runtime/shell/states/Expansion.rs b/src/runtime/shell/states/Expansion.rs index 2d066ce88c3c..492c37748122 100644 --- a/src/runtime/shell/states/Expansion.rs +++ b/src/runtime/shell/states/Expansion.rs @@ -488,12 +488,7 @@ impl Expansion { *has_quoted_empty = true; } ast::SimpleAtom::Var(label) => { - // Spec `expandVar`: shell_env first, then export_env, else "". - let key = EnvStr::init_slice(label); - if let Some(v) = shell.shell_env.get(key) { - out.extend_from_slice(v.slice()); - v.deref(); - } else if let Some(v) = shell.export_env.get(EnvStr::init_slice(label)) { + if let Some(v) = shell.vars.get(EnvStr::init_slice(label)) { out.extend_from_slice(v.slice()); v.deref(); } diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index fa99bc8fe0db..5f83d4d92612 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1442,26 +1442,29 @@ impl<'a> SpawnArgs<'a> { out } - /// `object_iter` should be a some type with the following fields: - /// - `next() bool` - pub(crate) fn fill_env( + /// Appends one `key=value` entry per pair of `env` to the child's environment. + pub(crate) fn fill_env<'e, const DISABLE_PATH_LOOKUP_FOR_ARV0: bool>( &mut self, - env_iter: &mut crate::shell::env_map::Iterator<'_>, + env: impl Iterator, ) { self.override_env = true; - // Note: `bun_collections::array_hash_map::Iter` doesn't impl - // `ExactSizeIterator`; use `size_hint` for the reservation. - self.env_array - .reserve_exact(env_iter.size_hint().0.saturating_sub(self.env_array.len())); + // The exported variables are a filtered view with a lower bound of 0, + // so reserve for the upper bound. + self.env_array.reserve_exact( + env.size_hint() + .1 + .unwrap_or(0) + .saturating_sub(self.env_array.len()), + ); if DISABLE_PATH_LOOKUP_FOR_ARV0 { // If the env object does not include a $PATH, it must disable path lookup for argv[0] self.path = b""; } - while let Some(entry) = env_iter.next() { - let key = entry.key_ptr.slice(); - let value = entry.value_ptr.slice(); + for (key, value) in env { + let key = key.slice(); + let value = value.slice(); // Build a NUL-terminated `key=value` string in the spawn arena. // Bumpalo owns the bytes; freed when the spawn arena is reset. diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index e88cc22243a9..9adbcfff2d32 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1276,6 +1276,118 @@ booga" }); }); + // A variable has one value and an export attribute. `export NAME` keeps the + // value, and a plain assignment keeps the attribute. + describe("export attribute", () => { + // Prints NAME=value for each NAME argument, as a child process sees it. + const childEnv = "console.log(process.argv.slice(1).map(k => k + '=' + (process.env[k] ?? '')).join(' '))"; + const env = { ...bunEnv, OUTER: "fromenv", APPEND: "base", CLEARED: "full" }; + + TestBuilder.command` + A=1; export A + export OUTER + export KEEP=1; export KEEP + export NOVALUE + export LATER; LATER=late + export EMPTY= + X=1; Y=2; export X Y + echo "$A $OUTER $KEEP [$NOVALUE] $LATER [$EMPTY] $X $Y" + ${BUN} -e ${childEnv} A OUTER KEEP NOVALUE LATER EMPTY X Y + ` + .env(env) + .stdout( + "1 fromenv 1 [] late [] 1 2\nA=1 OUTER=fromenv KEEP=1 NOVALUE= LATER=late EMPTY= X=1 Y=2\n", + ) + .testMini() + .runAsTest("export NAME keeps the value of the name"); + + TestBuilder.command` + OUTER=changed + APPEND="$APPEND+more" + CLEARED= + export T=5; T=6 + U=5; export U; U=7 + FRESH=1 + echo "$OUTER $APPEND [$CLEARED] $T $U $FRESH" + ${BUN} -e ${childEnv} OUTER APPEND CLEARED T U FRESH + ` + .env(env) + .stdout("changed base+more [] 6 7 1\nOUTER=changed APPEND=base+more CLEARED= T=6 U=7 FRESH=\n") + .testMini() + .runAsTest("a plain assignment keeps the export attribute of the name"); + + TestBuilder.command` + FOO=a; export FOO=b; echo $FOO + BAR=a; export BAR=b; BAR=c; echo $BAR + ${BUN} -e ${childEnv} FOO BAR + ` + .stdout("b\nc\nFOO=b BAR=c\n") + .testMini() + .runAsTest("export NAME=value replaces a plain assignment to the name"); + + TestBuilder.command` + OUTER=changed; FRESH=1; export PENDING + (${BUN} -e ${childEnv} OUTER FRESH) + ${BUN} -e ${childEnv} OUTER FRESH | cat + echo $(${BUN} -e ${childEnv} OUTER FRESH) + (OUTER=inner; export FRESH; export NESTED=1; PENDING=1; ${BUN} -e ${childEnv} OUTER FRESH NESTED PENDING) + ${BUN} -e ${childEnv} OUTER FRESH NESTED PENDING + ` + .env(env) + .stdout( + [ + "OUTER=changed FRESH=", + "OUTER=changed FRESH=", + "OUTER=changed FRESH=", + "OUTER=inner FRESH=1 NESTED=1 PENDING=1", + "OUTER=changed FRESH= NESTED= PENDING=", + "", + ].join("\n"), + ) + .testMini() + .runAsTest("a subshell, a pipeline and a command substitution copy values and attributes"); + + TestBuilder.command`PWD=junk; OLDPWD=junk; cd .; echo $PWD; echo $OLDPWD; pwd` + .stdout(out => { + const [pwd, oldpwd, cwd] = out.split("\n"); + expect({ pwd, oldpwd }).toEqual({ pwd: cwd, oldpwd: cwd }); + }) + .testMini() + .runAsTest("cd sets PWD and OLDPWD over a plain assignment"); + + TestBuilder.command`OUTER=changed; export NOVALUE; export` + .env(env) + .stdout(out => { + const lines = out.split("\n").filter(line => line.startsWith("OUTER") || line.startsWith("NOVALUE")); + expect(lines).toEqual(["NOVALUE", "OUTER=changed"]); + }) + .testMini() + .runAsTest("export with no operand lists the current value of each exported name"); + + // Only the prefix on the `export` command itself is an assignment to its operand. + TestBuilder.command` + A=3 export A; echo $A + B=1 export B=2 B; echo $B + C=4 true; export C; echo "[$C]" + ` + .stdout("3\n2\n[]\n") + .testMini() + .runAsTest("NAME=value export NAME assigns the value to the name"); + + test.skipIf(isWindows)("a plain assignment to PATH is used by command lookup and by which", async () => { + using dir = tempDir("shell-path-assign", { + "onlyintool": "#!/bin/sh\necho from-onlyintool\n", + }); + chmodSync(join(String(dir), "onlyintool"), 0o755); + const toolDir = String(dir); + + const { stdout, stderr, exitCode } = await $`PATH=${toolDir}; onlyintool; which onlyintool`.quiet().nothrow(); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toBe(`from-onlyintool\n${toolDir}/onlyintool\n`); + expect(exitCode).toBe(0); + }); + }); + describe("cd & pwd", () => { test("cd", async () => { const { stdout } = await $`cd ${temp_dir} && ls`; From 5277ee915d72b86615873e98354eafcbf4ccdfd6 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:55:31 +0000 Subject: [PATCH 2/9] [autofix.ci] apply automated fixes --- src/runtime/shell/builtin/export.rs | 6 ++---- test/js/bun/shell/bunshell.test.ts | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 40f66018a997..1ded76196e64 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -72,10 +72,8 @@ impl Export { } fn print_all(interp: &Interpreter, cmd: NodeId) -> Yield { - let mut entries: Vec<(EnvStr, Option)> = Builtin::shell(interp, cmd) - .vars - .exported_names() - .collect(); + let mut entries: Vec<(EnvStr, Option)> = + Builtin::shell(interp, cmd).vars.exported_names().collect(); index_sort::sort_slice_by(&mut entries, |a, b| a.0.slice().cmp(b.0.slice())); let mut buf = Vec::new(); diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 9adbcfff2d32..cf91b284e887 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1295,9 +1295,7 @@ booga" ${BUN} -e ${childEnv} A OUTER KEEP NOVALUE LATER EMPTY X Y ` .env(env) - .stdout( - "1 fromenv 1 [] late [] 1 2\nA=1 OUTER=fromenv KEEP=1 NOVALUE= LATER=late EMPTY= X=1 Y=2\n", - ) + .stdout("1 fromenv 1 [] late [] 1 2\nA=1 OUTER=fromenv KEEP=1 NOVALUE= LATER=late EMPTY= X=1 Y=2\n") .testMini() .runAsTest("export NAME keeps the value of the name"); From c6f88fe823384883dfc08ac1ef71df66d2f9c690 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 15 Oct 2026 16:38:56 +0000 Subject: [PATCH 3/9] shell: shorten the variable table comments, make the cd test independent of path spelling --- src/runtime/shell/EnvMap.rs | 21 ++++++++------------- src/runtime/shell/EnvStr.rs | 4 ++-- src/runtime/shell/interpreter.rs | 9 ++------- src/runtime/shell/subproc.rs | 3 +-- test/js/bun/shell/bunshell.test.ts | 5 ++--- 5 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/runtime/shell/EnvMap.rs b/src/runtime/shell/EnvMap.rs index 98182c284e4e..7880cda79b2c 100644 --- a/src/runtime/shell/EnvMap.rs +++ b/src/runtime/shell/EnvMap.rs @@ -127,21 +127,20 @@ impl Drop for EnvMap { } } -// `VarTable` slot attributes. Zero is an exported variable that has a value, -// so an inherited environment needs no marking. +/// Slot attribute: the name has a value and is not exported. const LOCAL: u8 = 1 << 0; +/// Slot attribute: the name is exported and has no value yet. const UNSET: u8 = 1 << 1; const _: () = assert!(core::mem::size_of::() == 16); -/// Every shell variable, one slot per name. The attribute bits of the slot say -/// if the name is exported and if it has a value. +/// Every shell variable: one slot per name, with its value and its attribute bits. pub(crate) struct VarTable { map: EnvMap, } impl VarTable { - /// Every entry of `env` becomes an exported variable. + /// Every entry of `env` becomes an exported variable: its attribute bits are already zero. pub(crate) fn from_exported(env: EnvMap) -> VarTable { VarTable { map: env } } @@ -156,8 +155,7 @@ impl VarTable { } } - /// `NAME=value`. The name keeps its export attribute. - /// NOTE: This will `.ref()` value, like `EnvMap::insert`. + /// `NAME=value`: the name keeps its export attribute. Refs `value`, like `EnvMap::insert`. pub(crate) fn set(&mut self, name: EnvStr, value: EnvStr) { let slot = self.map.map.get_or_put(name).expect("OOM"); let attrs = if slot.found_existing { @@ -171,8 +169,7 @@ impl VarTable { *slot.value_ptr = value.with_var_attrs(attrs); } - /// `export NAME=value`, or `export NAME` for `None`: the name keeps its value. - /// NOTE: This will `.ref()` value, like `EnvMap::insert`. + /// `export NAME=value`, or `export NAME` for `None`, which keeps the value. Refs `value`. pub(crate) fn export(&mut self, name: EnvStr, value: Option) { let slot = self.map.map.get_or_put(name).expect("OOM"); if !slot.found_existing { @@ -192,8 +189,7 @@ impl VarTable { } } - /// The value of `$NAME`. - /// NOTE: Make sure you deref the string when done! + /// The value of `$NAME`. Deref it when done, like `EnvMap::get`. pub(crate) fn get(&self, name: EnvStr) -> Option { let slot = *self.map.map.get(&name)?; if slot.var_attrs() & UNSET != 0 { @@ -204,8 +200,7 @@ impl VarTable { Some(value) } - /// The value a child process inherits for `name`. - /// NOTE: Make sure you deref the string when done! + /// The value a child process inherits for `name`. Deref it when done. pub(crate) fn get_exported(&self, name: EnvStr) -> Option { let slot = *self.map.map.get(&name)?; if slot.var_attrs() != 0 { diff --git a/src/runtime/shell/EnvStr.rs b/src/runtime/shell/EnvStr.rs index d32d12691668..919151e7e37b 100644 --- a/src/runtime/shell/EnvStr.rs +++ b/src/runtime/shell/EnvStr.rs @@ -13,8 +13,7 @@ use super::ref_counted_str::RefCountedStr; /// Packed `u128` layout (LSB-first): /// - bits 0..48 : `ptr` (u48) -/// - bits 48..62 : `tag` -/// - bits 62..64 : variable attributes, zero outside a `VarTable` slot +/// - bits 48..64 : `tag` (u16) /// - bits 64..128 : `len` (usize) #[repr(transparent)] #[derive(Copy, Clone, Eq, PartialEq)] @@ -36,6 +35,7 @@ pub(crate) enum Tag { const PTR_MASK: u128 = (1u128 << 48) - 1; const TAG_SHIFT: u32 = 48; const TAG_MASK: u128 = 0x3FFF; +// The top two bits of the tag field are the attributes of a `VarTable` slot, zero anywhere else. const VAR_ATTRS_SHIFT: u32 = 62; const VAR_ATTRS_MASK: u128 = 0b11; const LEN_SHIFT: u32 = 64; diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index 1cd8f49a0ae2..adfead00a6c9 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -2133,10 +2133,7 @@ impl ShellExecEnv { Ok(()) } - /// Routes `label = value` depending on where the assignment appeared - /// (`FOO=1 cmd` → cmd-local, bare `FOO=1` → the variable table). - /// NOTE: both targets `.ref()` the value, so callers should deref - /// `value` after the call. + /// `FOO=1 cmd` goes to `cmd_local_env`, a bare `FOO=1` to `vars`. Both ref `value`: deref it after the call. pub(crate) fn assign_var( &mut self, label: crate::shell::env_str::EnvStr, @@ -2149,9 +2146,7 @@ impl ShellExecEnv { } } - /// Looks up `$HOME` (`$USERPROFILE` on Windows). Falls back to `""` (or - /// `/data/local/tmp` on Android) so `cd` with no args / `~` expansion - /// never sees a null. + /// `$HOME` (`$USERPROFILE` on Windows), or `""` (`/data/local/tmp` on Android) so `cd` and `~` never see a null. pub(crate) fn get_homedir(&self) -> crate::shell::env_str::EnvStr { use crate::shell::env_str::EnvStr; let key = if cfg!(windows) { diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 5f83d4d92612..8452ce6d70fe 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1448,8 +1448,7 @@ impl<'a> SpawnArgs<'a> { env: impl Iterator, ) { self.override_env = true; - // The exported variables are a filtered view with a lower bound of 0, - // so reserve for the upper bound. + // `VarTable::exported` is a filter, whose lower size bound is 0, so reserve for the upper bound. self.env_array.reserve_exact( env.size_hint() .1 diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index cf91b284e887..ce02ac9f9719 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1276,8 +1276,7 @@ booga" }); }); - // A variable has one value and an export attribute. `export NAME` keeps the - // value, and a plain assignment keeps the attribute. + // `export NAME` keeps the value of the name, and a plain assignment keeps its export attribute. describe("export attribute", () => { // Prints NAME=value for each NAME argument, as a child process sees it. const childEnv = "console.log(process.argv.slice(1).map(k => k + '=' + (process.env[k] ?? '')).join(' '))"; @@ -1345,7 +1344,7 @@ booga" .testMini() .runAsTest("a subshell, a pipeline and a command substitution copy values and attributes"); - TestBuilder.command`PWD=junk; OLDPWD=junk; cd .; echo $PWD; echo $OLDPWD; pwd` + TestBuilder.command`PWD=junk; OLDPWD=junk; cd .; cd .; echo $PWD; echo $OLDPWD; pwd` .stdout(out => { const [pwd, oldpwd, cwd] = out.split("\n"); expect({ pwd, oldpwd }).toEqual({ pwd: cwd, oldpwd: cwd }); From be4b33c5edbad8f8a5c55b0c8b87e97a7c17f0e7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 04:56:48 +0000 Subject: [PATCH 4/9] shell: keep the export state in the variable table's own entry, not in EnvStr tag bits --- src/runtime/shell/EnvMap.rs | 173 +++++++++++++++---------- src/runtime/shell/EnvStr.rs | 18 +-- src/runtime/shell/ParsedShellScript.rs | 12 +- src/runtime/shell/interpreter.rs | 12 +- 4 files changed, 115 insertions(+), 100 deletions(-) diff --git a/src/runtime/shell/EnvMap.rs b/src/runtime/shell/EnvMap.rs index 7880cda79b2c..849ba039c41e 100644 --- a/src/runtime/shell/EnvMap.rs +++ b/src/runtime/shell/EnvMap.rs @@ -63,12 +63,6 @@ impl EnvMap { size } - pub(crate) fn init_with_capacity(cap: usize) -> EnvMap { - EnvMap { - map: EnvMapInner::with_capacity(cap), - } - } - /// NOTE: This will `.ref()` value, so you should `defer value.deref()` it /// before handing it to this function!!! pub(crate) fn insert(&mut self, key: EnvStr, val: EnvStr) { @@ -86,10 +80,6 @@ impl EnvMap { self.map.keys().iter().zip(self.map.values()) } - pub(crate) fn ensure_total_capacity(&mut self, new_capacity: usize) { - self.map.ensure_total_capacity(new_capacity).expect("OOM"); - } - /// NOTE: Make sure you deref the string when done! pub(crate) fn get(&self, key: EnvStr) -> Option { let val = *self.map.get(&key)?; @@ -97,21 +87,6 @@ impl EnvMap { Some(val) } - pub(crate) fn clone(&self) -> EnvMap { - let new = EnvMap { - map: self.map.clone().expect("OOM"), - }; - new.ref_strings(); - new - } - - fn ref_strings(&self) { - for (key, value) in self.map.keys().iter().zip(self.map.values()) { - key.ref_(); - value.ref_(); - } - } - fn deref_strings(&self) { for (key, value) in self.map.keys().iter().zip(self.map.values()) { key.deref(); @@ -127,101 +102,157 @@ impl Drop for EnvMap { } } -/// Slot attribute: the name has a value and is not exported. -const LOCAL: u8 = 1 << 0; -/// Slot attribute: the name is exported and has no value yet. -const UNSET: u8 = 1 << 1; +/// One shell variable. +#[derive(Copy, Clone, Default)] +enum Var { + /// Exported, with a value: a child process inherits it. + Exported(EnvStr), + /// Has a value, not exported. + Local(EnvStr), + /// Exported, no value yet. + #[default] + ExportedUnset, +} + +impl Var { + fn value(self) -> Option { + match self { + Var::Exported(value) | Var::Local(value) => Some(value), + Var::ExportedUnset => None, + } + } +} -const _: () = assert!(core::mem::size_of::() == 16); +type VarMap = ArrayHashMap; -/// Every shell variable: one slot per name, with its value and its attribute bits. +/// Every shell variable, one entry per name. pub(crate) struct VarTable { - map: EnvMap, + map: VarMap, } impl VarTable { - /// Every entry of `env` becomes an exported variable: its attribute bits are already zero. - pub(crate) fn from_exported(env: EnvMap) -> VarTable { - VarTable { map: env } + pub(crate) fn init() -> VarTable { + VarTable { map: VarMap::new() } + } + + pub(crate) fn init_with_capacity(cap: usize) -> VarTable { + VarTable { + map: VarMap::with_capacity(cap), + } + } + + pub(crate) fn ensure_total_capacity(&mut self, new_capacity: usize) { + self.map.ensure_total_capacity(new_capacity).expect("OOM"); } pub(crate) fn memory_cost(&self) -> usize { - self.map.memory_cost() + let mut size: usize = core::mem::size_of::(); + size += core::mem::size_of_val(self.map.keys()); + size += core::mem::size_of_val(self.map.values()); + for (name, var) in self.map.iter() { + size += name.memory_cost(); + size += var.value().map_or(0, EnvStr::memory_cost); + } + size } pub(crate) fn clone(&self) -> VarTable { - VarTable { - map: self.map.clone(), + let new = VarTable { + map: self.map.clone().expect("OOM"), + }; + for (name, var) in new.map.iter() { + name.ref_(); + if let Some(value) = var.value() { + value.ref_(); + } } + new } /// `NAME=value`: the name keeps its export attribute. Refs `value`, like `EnvMap::insert`. pub(crate) fn set(&mut self, name: EnvStr, value: EnvStr) { - let slot = self.map.map.get_or_put(name).expect("OOM"); - let attrs = if slot.found_existing { - slot.value_ptr.deref(); - slot.value_ptr.var_attrs() & !UNSET - } else { + let slot = self.map.get_or_put(name).expect("OOM"); + value.ref_(); + if !slot.found_existing { name.ref_(); - LOCAL + *slot.value_ptr = Var::Local(value); + return; + } + *slot.value_ptr = match *slot.value_ptr { + Var::Local(old) => { + old.deref(); + Var::Local(value) + } + Var::Exported(old) => { + old.deref(); + Var::Exported(value) + } + Var::ExportedUnset => Var::Exported(value), }; - value.ref_(); - *slot.value_ptr = value.with_var_attrs(attrs); } /// `export NAME=value`, or `export NAME` for `None`, which keeps the value. Refs `value`. pub(crate) fn export(&mut self, name: EnvStr, value: Option) { - let slot = self.map.map.get_or_put(name).expect("OOM"); + let slot = self.map.get_or_put(name).expect("OOM"); if !slot.found_existing { name.ref_(); } - match value { - Some(value) => { - slot.value_ptr.deref(); + match (value, *slot.value_ptr) { + (Some(value), old) => { value.ref_(); - *slot.value_ptr = value.with_var_attrs(0); + if let Some(old) = old.value() { + old.deref(); + } + *slot.value_ptr = Var::Exported(value); } - None if slot.found_existing => { - let attrs = slot.value_ptr.var_attrs() & !LOCAL; - *slot.value_ptr = slot.value_ptr.with_var_attrs(attrs); - } - None => *slot.value_ptr = slot.value_ptr.with_var_attrs(UNSET), + (None, Var::Local(value)) => *slot.value_ptr = Var::Exported(value), + (None, Var::Exported(_) | Var::ExportedUnset) => {} } } /// The value of `$NAME`. Deref it when done, like `EnvMap::get`. pub(crate) fn get(&self, name: EnvStr) -> Option { - let slot = *self.map.map.get(&name)?; - if slot.var_attrs() & UNSET != 0 { - return None; - } - let value = slot.with_var_attrs(0); + let value = self.map.get(&name)?.value()?; value.ref_(); Some(value) } /// The value a child process inherits for `name`. Deref it when done. pub(crate) fn get_exported(&self, name: EnvStr) -> Option { - let slot = *self.map.map.get(&name)?; - if slot.var_attrs() != 0 { + let Var::Exported(value) = *self.map.get(&name)? else { return None; - } - slot.ref_(); - Some(slot) + }; + value.ref_(); + Some(value) } /// The environment of a child process: exported names that have a value. pub(crate) fn exported(&self) -> impl core::iter::Iterator { - self.map.iter().filter(|(_, slot)| slot.var_attrs() == 0) + self.map.iter().filter_map(|(name, var)| match var { + Var::Exported(value) => Some((name, value)), + Var::Local(_) | Var::ExportedUnset => None, + }) } /// Every exported name. The value is `None` until the name is assigned. pub(crate) fn exported_names( &self, ) -> impl core::iter::Iterator)> + '_ { - self.map - .iter() - .filter(|(_, slot)| slot.var_attrs() & LOCAL == 0) - .map(|(name, slot)| (*name, (slot.var_attrs() & UNSET == 0).then_some(*slot))) + self.map.iter().filter_map(|(name, var)| match *var { + Var::Exported(value) => Some((*name, Some(value))), + Var::ExportedUnset => Some((*name, None)), + Var::Local(_) => None, + }) + } +} + +impl Drop for VarTable { + fn drop(&mut self) { + for (name, var) in self.map.iter() { + name.deref(); + if let Some(value) = var.value() { + value.deref(); + } + } } } diff --git a/src/runtime/shell/EnvStr.rs b/src/runtime/shell/EnvStr.rs index 919151e7e37b..fc26e45cc520 100644 --- a/src/runtime/shell/EnvStr.rs +++ b/src/runtime/shell/EnvStr.rs @@ -34,10 +34,7 @@ pub(crate) enum Tag { const PTR_MASK: u128 = (1u128 << 48) - 1; const TAG_SHIFT: u32 = 48; -const TAG_MASK: u128 = 0x3FFF; -// The top two bits of the tag field are the attributes of a `VarTable` slot, zero anywhere else. -const VAR_ATTRS_SHIFT: u32 = 62; -const VAR_ATTRS_MASK: u128 = 0b11; +const TAG_MASK: u128 = 0xFFFF; const LEN_SHIFT: u32 = 64; impl EnvStr { @@ -73,19 +70,6 @@ impl EnvStr { (self.0 >> LEN_SHIFT) as u64 as usize } - #[inline] - pub(super) fn var_attrs(self) -> u8 { - ((self.0 >> VAR_ATTRS_SHIFT) & VAR_ATTRS_MASK) as u8 - } - - #[inline] - pub(super) fn with_var_attrs(self, attrs: u8) -> EnvStr { - EnvStr( - (self.0 & !(VAR_ATTRS_MASK << VAR_ATTRS_SHIFT)) - | ((attrs as u128 & VAR_ATTRS_MASK) << VAR_ATTRS_SHIFT), - ) - } - #[inline] pub(crate) fn init_slice(str: &[u8]) -> EnvStr { if str.is_empty() { diff --git a/src/runtime/shell/ParsedShellScript.rs b/src/runtime/shell/ParsedShellScript.rs index 14f6dbcdefda..b6e6cd04a854 100644 --- a/src/runtime/shell/ParsedShellScript.rs +++ b/src/runtime/shell/ParsedShellScript.rs @@ -8,7 +8,7 @@ use bun_jsc::{ JsRef, JsResult, MarkedArgumentBuffer, StringJsc as _, }; -use super::env_map::EnvMap; +use super::env_map::VarTable; use super::interpreter::ShellArgs; use super::shell_body::shell_cmd_from_js; use super::{EnvStr, Interpreter}; @@ -26,7 +26,7 @@ pub(crate) struct ParsedShellScript { // the extra alloc matters. JSValues here are GC-rooted via `toJSWithValues` codegen // (own: array on the C++ wrapper), so storing them on the Rust heap is sound. pub(crate) jsobjs: JsCell>, - pub(crate) export_env: JsCell>, + pub(crate) export_env: JsCell>, pub(crate) quiet: Cell, pub(crate) cwd: JsCell>, /// Self-wrapper backref. `.classes.ts` has `finalize: true`, so the weak arm is @@ -84,7 +84,7 @@ impl ParsedShellScript { Vec, bool, Option, - Option, + Option, ) { let args = self.args.replace(None).expect("args already taken"); let jsobjs = self.jsobjs.replace(Vec::new()); @@ -143,7 +143,7 @@ impl ParsedShellScript { )?; // `defer object_iter.deinit()` — handled by Drop. - let mut env = EnvMap::init(); + let mut env = VarTable::init(); // errdefer env.deinit() — Drop on early-return handles this. env.ensure_total_capacity(object_iter.len); @@ -161,11 +161,11 @@ impl ParsedShellScript { let keyref = EnvStr::init_ref_counted(keyslice.into_boxed_slice()); let valueref = EnvStr::init_ref_counted(slice.into_boxed_slice()); - env.insert(keyref, valueref); + env.export(keyref, Some(valueref)); keyref.deref(); valueref.deref(); } - // Dropping the previous Option deinits it. + // Dropping the previous Option deinits it. self.export_env.set(Some(env)); Ok(JSValue::UNDEFINED) } diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index adfead00a6c9..854b73887ccc 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -477,25 +477,25 @@ impl Interpreter { event_loop: EventLoopHandle, shargs: Box, jsobjs: Vec, - export_env_: Option, + export_env_: Option, cwd_: Option<&[u8]>, ) -> ShellResult> { // ── export_env ───────────────────────────────────────────────────── // On the `.js` event loop, take `export_env_` (or empty); on `.mini`, // populate from `event_loop.env()` (the loop's `DotEnv::Loader`). let export_env = if matches!(event_loop, EventLoopHandle::Js { .. }) { - export_env_.unwrap_or_else(EnvMap::init) + export_env_.unwrap_or_else(VarTable::init) } else { // SAFETY: `event_loop.env()` returns the `MiniEventLoop`'s // `DotEnv::Loader`, which is set by `init_global()` and outlives // the interpreter (thread-lifetime singleton). let env_loader = unsafe { &mut *event_loop.env() }; - let mut export_env = EnvMap::init_with_capacity(env_loader.map.map.count()); + let mut export_env = VarTable::init_with_capacity(env_loader.map.map.count()); let mut iter = env_loader.iterator(); while let Some(entry) = iter.next() { let key = crate::shell::EnvStr::init_slice(&entry.key_ptr[..]); let value = crate::shell::EnvStr::init_slice(&entry.value_ptr.value[..]); - export_env.insert(key, value); + export_env.export(key, Some(value)); } export_env }; @@ -562,7 +562,7 @@ impl Interpreter { _buffered_stdout: Bufio::default(), _buffered_stderr: Bufio::default(), cmd_local_env: EnvMap::init(), - vars: VarTable::from_exported(export_env), + vars: export_env, __prev_cwd: cwd_arr.clone(), __cwd: cwd_arr, cwd_fd, @@ -2000,7 +2000,7 @@ impl ShellExecEnv { // EnvMap has a Drop impl; replace with fresh to free now and leave // valid state for any later `Drop` of the outer `Interpreter` box. self.cmd_local_env = EnvMap::init(); - self.vars = VarTable::from_exported(EnvMap::init()); + self.vars = VarTable::init(); self.__cwd = Vec::new(); self.__prev_cwd = Vec::new(); closefd(self.cwd_fd); From 07bcb8a176ae76b153a044eda113e131f87f7380 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:55:23 +0000 Subject: [PATCH 5/9] shell: keep a command's NAME=value prefixes on the command The prefixes of `NAME=value cmd` were in `cmd_local_env`, a map on the shell environment that no command cleared. Every later command of the same shell got them, and the spawn code appended them after the exported variables, so a name in both reached the child twice. The prefixes now live on the `Cmd` node (`prefix_env`) and go away with it. `VarTable::child_env` yields each exported name once, and a prefix replaces the exported value. `fill_env` takes that one iterator and reserves the whole array, with the null that ends it, in one call. --- src/runtime/shell/EnvMap.rs | 42 ++++++++++++------------- src/runtime/shell/builtin/export.rs | 6 ++-- src/runtime/shell/interpreter.rs | 24 ++------------- src/runtime/shell/states/Assigns.rs | 22 +++++++++---- src/runtime/shell/states/Cmd.rs | 14 ++++++--- src/runtime/shell/subproc.rs | 12 +++----- test/js/bun/shell/bunshell.test.ts | 48 +++++++++++++++++++++++------ 7 files changed, 93 insertions(+), 75 deletions(-) diff --git a/src/runtime/shell/EnvMap.rs b/src/runtime/shell/EnvMap.rs index 849ba039c41e..3aa869e3c456 100644 --- a/src/runtime/shell/EnvMap.rs +++ b/src/runtime/shell/EnvMap.rs @@ -51,18 +51,6 @@ impl EnvMap { } } - pub(crate) fn memory_cost(&self) -> usize { - let mut size: usize = core::mem::size_of::(); - size += core::mem::size_of_val(self.map.keys()); - size += core::mem::size_of_val(self.map.values()); - debug_assert_eq!(self.map.keys().len(), self.map.values().len()); - for (key, value) in self.map.keys().iter().zip(self.map.values()) { - size += key.memory_cost(); - size += value.memory_cost(); - } - size - } - /// NOTE: This will `.ref()` value, so you should `defer value.deref()` it /// before handing it to this function!!! pub(crate) fn insert(&mut self, key: EnvStr, val: EnvStr) { @@ -76,10 +64,6 @@ impl EnvMap { *result.value_ptr = val; } - pub(crate) fn iter(&self) -> impl core::iter::Iterator { - self.map.keys().iter().zip(self.map.values()) - } - /// NOTE: Make sure you deref the string when done! pub(crate) fn get(&self, key: EnvStr) -> Option { let val = *self.map.get(&key)?; @@ -193,7 +177,10 @@ impl VarTable { /// `export NAME=value`, or `export NAME` for `None`, which keeps the value. Refs `value`. pub(crate) fn export(&mut self, name: EnvStr, value: Option) { - let slot = self.map.get_or_put(name).expect("OOM"); + let slot = self + .map + .get_or_put_value(name, Var::ExportedUnset) + .expect("OOM"); if !slot.found_existing { name.ref_(); } @@ -226,12 +213,21 @@ impl VarTable { Some(value) } - /// The environment of a child process: exported names that have a value. - pub(crate) fn exported(&self) -> impl core::iter::Iterator { - self.map.iter().filter_map(|(name, var)| match var { - Var::Exported(value) => Some((name, value)), - Var::Local(_) | Var::ExportedUnset => None, - }) + /// The environment of a child process, one entry per name: each exported name that has a value, then `prefixes`, which replace an exported name. + pub(crate) fn child_env<'a>( + &'a self, + prefixes: Option<&'a EnvMap>, + ) -> impl core::iter::Iterator { + let replaced = move |name: &EnvStr| prefixes.is_some_and(|p| p.map.contains(name)); + let (names, values): (&[EnvStr], &[EnvStr]) = + prefixes.map_or((&[], &[]), |p| (p.map.keys(), p.map.values())); + self.map + .iter() + .filter_map(move |(name, var)| match var { + Var::Exported(value) if !replaced(name) => Some((name, value)), + Var::Exported(_) | Var::Local(_) | Var::ExportedUnset => None, + }) + .chain(names.iter().zip(values)) } /// Every exported name. The value is `None` until the name is assigned. diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 1ded76196e64..bbadf7b82252 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -63,9 +63,9 @@ impl Export { continue; } let name = EnvStr::init_slice(assign.label); - let shell = interp.as_cmd_mut(cmd).base.shell_mut(); - if let Some(value) = shell.cmd_local_env.get(name) { - shell.vars.set(name, value); + let me = interp.as_cmd_mut(cmd); + if let Some(value) = me.prefix_env.as_ref().and_then(|p| p.get(name)) { + me.base.shell_mut().vars.set(name, value); value.deref(); } } diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index 854b73887ccc..2c480424a9fa 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -561,7 +561,6 @@ impl Interpreter { root_shell: JsCell::new(ShellExecEnv { _buffered_stdout: Bufio::default(), _buffered_stderr: Bufio::default(), - cmd_local_env: EnvMap::init(), vars: export_env, __prev_cwd: cwd_arr.clone(), __cwd: cwd_arr, @@ -1797,7 +1796,6 @@ pub(crate) fn throw_shell_err( pub(crate) struct ShellExecEnv { pub(crate) _buffered_stdout: Bufio, pub(crate) _buffered_stderr: Bufio, - pub(crate) cmd_local_env: EnvMap, pub(crate) vars: VarTable, pub(crate) __prev_cwd: Vec, pub(crate) __cwd: Vec, @@ -1838,7 +1836,6 @@ pub(crate) enum ShellExecEnvKind { impl ShellExecEnv { pub(crate) fn memory_cost(&self) -> usize { let mut size = core::mem::size_of::(); - size += self.cmd_local_env.memory_cost(); size += self.vars.memory_cost(); size += self.__cwd.capacity(); size += self.__prev_cwd.capacity(); @@ -1916,7 +1913,7 @@ impl ShellExecEnv { /// Heap-allocates a /// fresh env for a subshell/pipeline child: dups `cwd_fd`, clones - /// `vars`, gives it a fresh empty `cmd_local_env`, and + /// `vars`, and /// borrows or owns buffered stdout/stderr per `kind` (subshell/pipeline /// borrow the parent's buffers so output bubbles up; cmd-subst owns). /// @@ -1956,7 +1953,6 @@ impl ShellExecEnv { let duped = Box::new(ShellExecEnv { _buffered_stdout: stdout, _buffered_stderr: stderr, - cmd_local_env: EnvMap::init(), vars: self.vars.clone(), __prev_cwd: self.__prev_cwd.clone(), __cwd: self.__cwd.clone(), @@ -1977,7 +1973,7 @@ impl ShellExecEnv { // explicit close. let boxed = unsafe { bun_core::heap::take(this) }; closefd(boxed.cwd_fd); - // EnvMap/Vec/Vec drop impls free their storage; `Bufio::Borrowed` + // VarTable/Vec/Vec drop impls free their storage; `Bufio::Borrowed` // is a raw ptr so its drop is a no-op. drop(boxed); } @@ -1997,9 +1993,8 @@ impl ShellExecEnv { o.clear_and_free(); } } - // EnvMap has a Drop impl; replace with fresh to free now and leave + // VarTable has a Drop impl; replace with fresh to free now and leave // valid state for any later `Drop` of the outer `Interpreter` box. - self.cmd_local_env = EnvMap::init(); self.vars = VarTable::init(); self.__cwd = Vec::new(); self.__prev_cwd = Vec::new(); @@ -2133,19 +2128,6 @@ impl ShellExecEnv { Ok(()) } - /// `FOO=1 cmd` goes to `cmd_local_env`, a bare `FOO=1` to `vars`. Both ref `value`: deref it after the call. - pub(crate) fn assign_var( - &mut self, - label: crate::shell::env_str::EnvStr, - value: crate::shell::env_str::EnvStr, - assign_ctx: AssignCtx, - ) { - match assign_ctx { - AssignCtx::Cmd => self.cmd_local_env.insert(label, value), - AssignCtx::Shell => self.vars.set(label, value), - } - } - /// `$HOME` (`$USERPROFILE` on Windows), or `""` (`/data/local/tmp` on Android) so `cd` and `~` never see a null. pub(crate) fn get_homedir(&self) -> crate::shell::env_str::EnvStr { use crate::shell::env_str::EnvStr; diff --git a/src/runtime/shell/states/Assigns.rs b/src/runtime/shell/states/Assigns.rs index 6103ea4a3341..f5083a7b4703 100644 --- a/src/runtime/shell/states/Assigns.rs +++ b/src/runtime/shell/states/Assigns.rs @@ -2,7 +2,7 @@ //! no effect on the environment of the shell, so we can skip them. use crate::shell::ast; -use crate::shell::interpreter::{Interpreter, Node, NodeId, ShellExecEnv, log}; +use crate::shell::interpreter::{EnvMap, Interpreter, Node, NodeId, ShellExecEnv, log}; use crate::shell::states::base::Base; use crate::shell::states::expansion::Expansion; use crate::shell::yield_::Yield; @@ -128,12 +128,22 @@ impl Assigns { merged }; + let name = EnvStr::init_slice(label); let value_ref = EnvStr::init_ref_counted(value.into_boxed_slice()); - interp.as_assigns_mut(this).base.shell_mut().assign_var( - EnvStr::init_slice(label), - value_ref, - ctx, - ); + // Both arms ref `value_ref`. + match ctx { + AssignCtx::Cmd => { + let cmd = interp.as_assigns(this).base.parent; + let prefixes = &mut interp.as_cmd_mut(cmd).prefix_env; + prefixes + .get_or_insert_with(|| Box::new(EnvMap::init())) + .insert(name, value_ref); + } + AssignCtx::Shell => { + let shell = interp.as_assigns_mut(this).base.shell_mut(); + shell.vars.set(name, value_ref); + } + } value_ref.deref(); Yield::Next(this) diff --git a/src/runtime/shell/states/Cmd.rs b/src/runtime/shell/states/Cmd.rs index eb7c86a02e24..1e03ec902776 100644 --- a/src/runtime/shell/states/Cmd.rs +++ b/src/runtime/shell/states/Cmd.rs @@ -6,7 +6,9 @@ use crate::shell::ExitCode; use crate::shell::ast; use crate::shell::builtin::{Builtin, Kind as BuiltinKind}; -use crate::shell::interpreter::{CowFd, Interpreter, Node, NodeId, ShellExecEnv, StateKind, log}; +use crate::shell::interpreter::{ + CowFd, EnvMap, Interpreter, Node, NodeId, ShellExecEnv, StateKind, log, +}; use crate::shell::io::{IO, OutKind as IoOutKind}; use crate::shell::shell_body::subproc::{Readable, ShellSubprocess, StdioKind}; use crate::shell::states::assigns::{AssignCtx, Assigns}; @@ -23,6 +25,8 @@ pub(crate) struct Cmd { pub(crate) io: IO, pub(crate) state: CmdState, pub args: Vec>, + /// The `NAME=value` words before the command name. They reach this command only. + pub(crate) prefix_env: Option>, pub(crate) redirection_file: Vec, pub(crate) redirection_fd: Option<*mut CowFd>, pub(crate) exec: Exec, @@ -212,6 +216,7 @@ impl Cmd { io, state: CmdState::Idle, args: Vec::new(), + prefix_env: None, redirection_file: Vec::new(), redirection_fd: None, exec: Exec::None, @@ -478,11 +483,10 @@ impl Cmd { // SAFETY: `shell_ptr` is the live env owned by this Cmd's scope chain. spawn_args.cwd = unsafe { &*shell_ptr }.cwd(); - // Fill env from the exported variables + cmd_local_env. { - let env = interp.as_cmd(this).base.shell(); - spawn_args.fill_env::(env.vars.exported()); - spawn_args.fill_env::(env.cmd_local_env.iter()); + let cmd = interp.as_cmd(this); + let prefixes = cmd.prefix_env.as_deref(); + spawn_args.fill_env::(cmd.base.shell().vars.child_env(prefixes)); } // Resolve argv[0] via PATH (`bun_which::which`). diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 8452ce6d70fe..83dad26ffb87 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1442,19 +1442,15 @@ impl<'a> SpawnArgs<'a> { out } - /// Appends one `key=value` entry per pair of `env` to the child's environment. + /// Sets the child's environment to one `key=value` entry per pair of `env`. pub(crate) fn fill_env<'e, const DISABLE_PATH_LOOKUP_FOR_ARV0: bool>( &mut self, env: impl Iterator, ) { self.override_env = true; - // `VarTable::exported` is a filter, whose lower size bound is 0, so reserve for the upper bound. - self.env_array.reserve_exact( - env.size_hint() - .1 - .unwrap_or(0) - .saturating_sub(self.env_array.len()), - ); + // The upper bound, because `VarTable::child_env` filters. One more for the null that ends the array. + self.env_array + .reserve_exact(env.size_hint().1.unwrap_or(0) + 1); if DISABLE_PATH_LOOKUP_FOR_ARV0 { // If the env object does not include a $PATH, it must disable path lookup for argv[0] diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index ce02ac9f9719..fef5f6addc87 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1256,12 +1256,7 @@ booga" let procEnv = JSON.parse(str1); expect(procEnv).toEqual({ ...bunEnv, BAZ: "1", FOO: "bar" }); procEnv = JSON.parse(str2); - expect(procEnv).toEqual({ - ...bunEnv, - BAZ: "1", - FOO: "bar", - BUN_TEST_VAR: "1", - }); + expect(procEnv).toEqual({ ...bunEnv, FOO: "bar", BUN_TEST_VAR: "1" }); }); test("syntax edgecase", async () => { @@ -1276,8 +1271,8 @@ booga" }); }); - // `export NAME` keeps the value of the name, and a plain assignment keeps its export attribute. - describe("export attribute", () => { + // `export NAME` keeps the value, a plain assignment keeps the export attribute, a `NAME=value` prefix reaches one command. + describe("exported variables", () => { // Prints NAME=value for each NAME argument, as a child process sees it. const childEnv = "console.log(process.argv.slice(1).map(k => k + '=' + (process.env[k] ?? '')).join(' '))"; const env = { ...bunEnv, OUTER: "fromenv", APPEND: "base", CLEARED: "full" }; @@ -1366,11 +1361,46 @@ booga" A=3 export A; echo $A B=1 export B=2 B; echo $B C=4 true; export C; echo "[$C]" + ${BUN} -e ${childEnv} A B C ` - .stdout("3\n2\n[]\n") + .stdout("3\n2\n[]\nA=3 B=2 C=\n") .testMini() .runAsTest("NAME=value export NAME assigns the value to the name"); + TestBuilder.command` + OUTER=pre NEW=1 ${BUN} -e ${childEnv} OUTER NEW + ${BUN} -e ${childEnv} OUTER NEW + OUTER=changed + ${BUN} -e ${childEnv} OUTER + echo "$OUTER [$NEW]" + ` + .env(env) + .stdout("OUTER=pre NEW=1\nOUTER=fromenv NEW=\nOUTER=changed\nchanged []\n") + .testMini() + .runAsTest("a NAME=value prefix reaches one command only"); + + if (isPosix) { + TestBuilder.command` + OUTER=pre env + export MADE=first; MADE=second env + TWICE=1 TWICE=2 env + ` + .env(env) + .stdout(out => { + const entries = out.split("\n").filter(line => /^(OUTER|MADE|TWICE)=/.test(line)); + expect(entries).toEqual([ + "OUTER=pre", + "OUTER=fromenv", + "MADE=second", + "OUTER=fromenv", + "MADE=first", + "TWICE=2", + ]); + }) + .testMini() + .runAsTest("a NAME=value prefix over an exported name gives the child one entry for the name"); + } + test.skipIf(isWindows)("a plain assignment to PATH is used by command lookup and by which", async () => { using dir = tempDir("shell-path-assign", { "onlyintool": "#!/bin/sh\necho from-onlyintool\n", From f51d531fbb7c3edc2671db3b509b8c454fa20812 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:42:43 +0000 Subject: [PATCH 6/9] shell: find a replaced exported name by name compare, look up export prefixes by operand `VarTable::child_env` hashed every exported name to ask the prefix map whether a prefix replaces it. A command has few prefixes, so it now compares each exported name with the prefix names. `export` looked for its own prefixes through the labels of the AST and compared bytes. It now asks the prefix map for each operand name, which compares names the way the variable table does. --- src/runtime/shell/EnvMap.rs | 2 +- src/runtime/shell/builtin/export.rs | 28 +++++++++++++--------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/runtime/shell/EnvMap.rs b/src/runtime/shell/EnvMap.rs index 3aa869e3c456..629d044da742 100644 --- a/src/runtime/shell/EnvMap.rs +++ b/src/runtime/shell/EnvMap.rs @@ -218,9 +218,9 @@ impl VarTable { &'a self, prefixes: Option<&'a EnvMap>, ) -> impl core::iter::Iterator { - let replaced = move |name: &EnvStr| prefixes.is_some_and(|p| p.map.contains(name)); let (names, values): (&[EnvStr], &[EnvStr]) = prefixes.map_or((&[], &[]), |p| (p.map.keys(), p.map.values())); + let replaced = move |name: &EnvStr| names.iter().any(|p| EnvMapContext.eql(p, name, 0)); self.map .iter() .filter_map(move |(name, var)| match var { diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index bbadf7b82252..0309e99195a5 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -52,22 +52,20 @@ impl Export { /// `NAME=v export NAME`: the prefix on this command assigns the operand. fn assign_prefixes(interp: &Interpreter, cmd: NodeId, argc: usize) { - let assigns = interp.as_cmd(cmd).ast_node().assigns; - for assign in assigns { - let is_operand = (0..argc).any(|i| { - let s = Builtin::of(interp, cmd).arg_bytes(i); - let eq = bun_core::strings::index_of_char_usize(s, b'=').unwrap_or(s.len()); - &s[..eq] == assign.label - }); - if !is_operand { + for i in 0..argc { + let Some(prefixes) = interp.as_cmd(cmd).prefix_env.as_deref() else { + return; + }; + let s = Builtin::of(interp, cmd).arg_bytes(i); + let eq = bun_core::strings::index_of_char_usize(s, b'=').unwrap_or(s.len()); + let Some(value) = prefixes.get(EnvStr::init_slice(&s[..eq])) else { continue; - } - let name = EnvStr::init_slice(assign.label); - let me = interp.as_cmd_mut(cmd); - if let Some(value) = me.prefix_env.as_ref().and_then(|p| p.get(name)) { - me.base.shell_mut().vars.set(name, value); - value.deref(); - } + }; + let label = EnvStr::dupe_ref_counted(&s[..eq]); + let shell = interp.as_cmd_mut(cmd).base.shell_mut(); + shell.vars.set(label, value); + label.deref(); + value.deref(); } } From 08e204f1a617a0d9bbbf9542414c9eb99d22f13c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 22:30:54 +0000 Subject: [PATCH 7/9] shell: test the child view of cd, the Windows name case, later lookups and the release paths - `cd`: the test reads PWD and OLDPWD in a child, with neither inherited. - Two tests record a choice between shells: a prefix on `export` assigns only an operand (bash), and OLDPWD is exported from the first `cd` (dash). - Windows: a name in another case is the same variable, and `cmd /c set` shows one entry for a name that a prefix replaces. - The PATH test is two tests. `which` runs on every platform. The lookup is a todo on Windows, where it matches the key `PATH` byte for byte. - A `PATH=` prefix does not decide where a later command is found. - 168 sequences of assignments, exports and prefixes per name are compared with the system `sh`. - A LeakSanitizer case in shell-worker-terminate-leak.test.ts takes each release path of the variable table and of a command's prefix map. - docs: variable names are ASCII-case-insensitive on Windows. --- docs/runtime/shell.mdx | 2 + test/js/bun/shell/bunshell.test.ts | 131 ++++++++++++++++-- .../shell/shell-worker-terminate-leak.test.ts | 27 ++++ 3 files changed, 148 insertions(+), 12 deletions(-) diff --git a/docs/runtime/shell.mdx b/docs/runtime/shell.mdx index 540814c28814..dbe5e35c47b1 100644 --- a/docs/runtime/shell.mdx +++ b/docs/runtime/shell.mdx @@ -345,6 +345,8 @@ await $`NODE_ENV=production; bun -e 'console.log(process.env.NODE_ENV)'`.env({ await $`BAR=bar; bun -e 'console.log(process.env.BAR)'`; // undefined\n ``` +On Windows, variable names are ASCII-case-insensitive, as in `process.env`: `PATH`, `Path` and `path` are one variable, so an assignment under any spelling changes what child processes receive. + ### Changing the environment variables By default, all commands use `process.env` as their environment variables. diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index fef5f6addc87..e9b618ec3dbf 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1339,13 +1339,30 @@ booga" .testMini() .runAsTest("a subshell, a pipeline and a command substitution copy values and attributes"); - TestBuilder.command`PWD=junk; OLDPWD=junk; cd .; cd .; echo $PWD; echo $OLDPWD; pwd` + TestBuilder.command`PWD=junk; OLDPWD=junk; cd .; cd .; echo $PWD; echo $OLDPWD; pwd; ${BUN} -e ${childEnv} PWD OLDPWD` + .env({ ...bunEnv, PWD: undefined, OLDPWD: undefined }) .stdout(out => { - const [pwd, oldpwd, cwd] = out.split("\n"); - expect({ pwd, oldpwd }).toEqual({ pwd: cwd, oldpwd: cwd }); + const [pwd, oldpwd, cwd, child] = out.split("\n"); + expect({ pwd, oldpwd, child }).toEqual({ pwd: cwd, oldpwd: cwd, child: `PWD=${cwd} OLDPWD=${cwd}` }); }) .testMini() - .runAsTest("cd sets PWD and OLDPWD over a plain assignment"); + .runAsTest("cd sets and exports PWD and OLDPWD over a plain assignment"); + + // OLDPWD gets the export attribute from the first `cd`, as in dash. bash exports it from the start. + TestBuilder.command` + OLDPWD=junk + ${BUN} -e ${childEnv} OLDPWD + cd .; cd . + pwd + ${BUN} -e ${childEnv} OLDPWD + ` + .env({ ...bunEnv, OLDPWD: undefined }) + .stdout(out => { + const [before, cwd, after] = out.split("\n"); + expect({ before, after }).toEqual({ before: "OLDPWD=", after: `OLDPWD=${cwd}` }); + }) + .testMini() + .runAsTest("OLDPWD is not exported before the first cd"); TestBuilder.command`OUTER=changed; export NOVALUE; export` .env(env) @@ -1367,6 +1384,18 @@ booga" .testMini() .runAsTest("NAME=value export NAME assigns the value to the name"); + // A prefix on `export` assigns only a name that is an operand, as in bash. dash assigns every prefix. + TestBuilder.command` + D=1 export E; echo "[$D]" + OUTER=x export F; echo $OUTER + G=1 H=2 export G; echo "[$G][$H]" + ${BUN} -e ${childEnv} D OUTER G H + ` + .env(env) + .stdout("[]\nfromenv\n[1][]\nD= OUTER=fromenv G=1 H=\n") + .testMini() + .runAsTest("a prefix on export does not assign a name that is not an operand"); + TestBuilder.command` OUTER=pre NEW=1 ${BUN} -e ${childEnv} OUTER NEW ${BUN} -e ${childEnv} OUTER NEW @@ -1379,6 +1408,14 @@ booga" .testMini() .runAsTest("a NAME=value prefix reaches one command only"); + // Windows: names are ASCII-case-insensitive, as in process.env, so `outer` is the inherited `OUTER`. + TestBuilder.command`outer=changed; echo $OUTER; ${BUN} -e ${childEnv} OUTER` + .env(env) + .stdout(isWindows ? "changed\nOUTER=changed\n" : "fromenv\nOUTER=fromenv\n") + .testMini() + .runAsTest("a name in another case is the same variable on Windows only"); + + // `env` is a POSIX tool. It prints every entry of the environment it got, duplicates included. if (isPosix) { TestBuilder.command` OUTER=pre env @@ -1401,16 +1438,68 @@ booga" .runAsTest("a NAME=value prefix over an exported name gives the child one entry for the name"); } - test.skipIf(isWindows)("a plain assignment to PATH is used by command lookup and by which", async () => { - using dir = tempDir("shell-path-assign", { - "onlyintool": "#!/bin/sh\necho from-onlyintool\n", - }); - chmodSync(join(String(dir), "onlyintool"), 0o755); - const toolDir = String(dir); + // `set NAME` of cmd.exe prints every entry of the environment it got whose name starts with NAME. + if (isWindows) { + TestBuilder.command` + OUTER=pre cmd /d /c set OUTER + outer=low cmd /d /c set OUTER + export MADE=first; made=second cmd /d /c set MADE + ` + .env(env) + .stdout(out => { + expect(out.split(/\r?\n/).filter(Boolean)).toEqual(["OUTER=pre", "outer=low", "made=second"]); + }) + .testMini() + .runAsTest("a NAME=value prefix over an exported name gives the child one entry for the name"); + } - const { stdout, stderr, exitCode } = await $`PATH=${toolDir}; onlyintool; which onlyintool`.quiet().nothrow(); + // Each sequence runs in its own subshell, for a name that is unset at the start and for an inherited name. + test.skipIf(isWindows)("sequences of assignments, exports and prefixes give the result of sh", async () => { + const ops = ["V=1", "export V", "export V=3", "V=4 printenv V"]; + const sequences = ops.flatMap(a => [[a], ...ops.flatMap(b => [[a, b], ...ops.map(c => [a, b, c])])]); + const script = ["UNSET_AT_START", "OUTER"] + .flatMap(name => + sequences.map(sequence => { + const statements = sequence.join("; ").replaceAll("V", name); + return `(echo '# ${statements}'; ${statements}; printenv ${name} || echo not-in-env; echo "[$${name}]")`; + }), + ) + .join("\n"); + + const sh = Bun.spawnSync({ cmd: ["sh", "-c", script], env, stderr: "inherit" }); + const bun = await $`${{ raw: script }}`.env(env).quiet().nothrow(); + expect(bun.stdout.toString().split("\n")).toEqual(sh.stdout.toString().split("\n")); + }); + + TestBuilder.command`A=1 $EMPTY; echo "[$A]"` + .stdout("[1]\n") + .todo("a command with no command word does not keep its assignments yet") + .runAsTest("NAME=value before an empty expansion assigns the name"); + + test("a plain assignment to PATH is used by which", async () => { + const exe = isWindows ? "onlyintool.exe" : "onlyintool"; + using dir = tempDir("shell-path-assign-which", { [exe]: "" }); + if (!isWindows) chmodSync(join(String(dir), exe), 0o755); + + const { stdout, stderr, exitCode } = await $`PATH=${String(dir)}; which onlyintool`.quiet().nothrow(); expect(stderr.toString()).toBe(""); - expect(stdout.toString()).toBe(`from-onlyintool\n${toolDir}/onlyintool\n`); + expect(stdout.toString()).toBe(join(String(dir), exe) + "\n"); + expect(exitCode).toBe(0); + }); + + // Windows: the command lookup matches the key PATH byte for byte and the inherited key is spelled Path (#42049). + test.todoIf(isWindows)("a plain assignment to PATH is used by command lookup", async () => { + using dir = tempDir( + "shell-path-assign", + isWindows + ? { "onlyintool.cmd": "@echo from-onlyintool\r\n" } + : { "onlyintool": "#!/bin/sh\necho from-onlyintool\n" }, + ); + if (!isWindows) chmodSync(join(String(dir), "onlyintool"), 0o755); + + const { stdout, stderr, exitCode } = await $`PATH=${String(dir)}; onlyintool`.quiet().nothrow(); + expect(stderr.toString()).toBe(""); + expect(stdout.toString().trim()).toBe("from-onlyintool"); expect(exitCode).toBe(0); }); }); @@ -3723,4 +3812,22 @@ test.skipIf(isWindows)("external command resolution uses the PATH from the shell expect(stdout.toString()).toBe("from-onlyintool\n"); expect(exitCode).toBe(0); } + + // A `PATH=` prefix is the PATH of its own command only: a later command is looked up on the shell's PATH. + { + const { stdout, stderr, exitCode } = await $`PATH=${toolDir}/empty true; onlyintool` + .env({ ...bunEnv, PATH: toolDir }) + .quiet() + .nothrow(); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toBe("from-onlyintool\n"); + expect(exitCode).toBe(0); + } + + { + const { stdout, stderr, exitCode } = await $`PATH=${toolDir} true; onlyintool`.quiet().nothrow(); + expect(stderr.toString()).toBe("bun: command not found: onlyintool\n"); + expect(stdout.toString()).toBe(""); + expect(exitCode).toBe(1); + } }); diff --git a/test/js/bun/shell/shell-worker-terminate-leak.test.ts b/test/js/bun/shell/shell-worker-terminate-leak.test.ts index 834015b1cf95..104d4c2b39b6 100644 --- a/test/js/bun/shell/shell-worker-terminate-leak.test.ts +++ b/test/js/bun/shell/shell-worker-terminate-leak.test.ts @@ -103,6 +103,33 @@ test.skipIf(!isASAN || isWindows)( LEAK_TEST_TIMEOUT, ); +test.skipIf(!isASAN || isWindows)( + "worker.terminate() with a live prefixed Bun.$ command does not leak variable values or the prefix map", + async () => { + // Each statement takes one release path of the variable table. `sh` checks what it got, then stays alive with its prefix map. + await runTerminateScenario( + ` + const running = Bun.$\` + L=1; L=2 + OUTER=changed + export E=1; E=2; export E=3 + P=1 export P + R=1 R=2 sh -c 'test "$L$OUTER$E$P$R" = changed312 && echo ok > flag.txt && exec sleep 100' + \`.nothrow(); + const exited = running.then(() => { + throw new Error("sh did not get OUTER, E, P and R as the script set them"); + }); + while (!(await Bun.file("flag.txt").exists())) { + await Promise.race([Bun.sleep(5), exited]); + } + postMessage("ok"); + `, + { OUTER: "fromenv" }, + ); + }, + LEAK_TEST_TIMEOUT, +); + test.skipIf(!isASAN || isWindows)( "worker.terminate() with a live Bun.$ file redirect does not leak the redirection fd", async () => { From b7013adc68f15f6624dcaf05f31acb66f6025525 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 8 Nov 2026 03:34:49 +0000 Subject: [PATCH 8/9] shell: test that export PATH with no value keeps the PATH for which and for the command lookup --- test/js/bun/shell/bunshell.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index bf561ce2f0f7..91c98801e1ff 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1523,9 +1523,12 @@ booga" using dir = tempDir("shell-path-assign-which", { [exe]: "" }); if (!isWindows) chmodSync(join(String(dir), exe), 0o755); - const { stdout, stderr, exitCode } = await $`PATH=${String(dir)}; which onlyintool`.quiet().nothrow(); + const found = join(String(dir), exe) + "\n"; + const { stdout, stderr, exitCode } = await $`PATH=${String(dir)}; which onlyintool; export PATH; which onlyintool` + .quiet() + .nothrow(); expect(stderr.toString()).toBe(""); - expect(stdout.toString()).toBe(join(String(dir), exe) + "\n"); + expect(stdout.toString()).toBe(found + found); expect(exitCode).toBe(0); }); @@ -3990,6 +3993,17 @@ test.skipIf(isWindows)("external command resolution uses the PATH from the shell expect(exitCode).toBe(0); } + // `export PATH` with no value keeps the PATH. + { + const { stdout, stderr, exitCode } = await $`export PATH; onlyintool` + .env({ ...bunEnv, PATH: toolDir }) + .quiet() + .nothrow(); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toBe("from-onlyintool\n"); + expect(exitCode).toBe(0); + } + // A `PATH=` prefix is the PATH of its own command only: a later command is looked up on the shell's PATH. { const { stdout, stderr, exitCode } = await $`PATH=${toolDir}/empty true; onlyintool` From 3ad58071df746b7077244ecec0a9abf25e26bfd1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 8 Nov 2026 06:40:00 +0000 Subject: [PATCH 9/9] shell: match the PATH key for the command lookup by the name rule of the variable table `fill_env` took the lookup path from the entry whose key is `PATH` byte for byte. On Windows the inherited key is `Path`, and the variable table keeps that spelling, so after `PATH=dir` the child got the new value and the lookup still used the PATH of the process. `name_eql` is now the one rule for variable names (ASCII-case-insensitive on Windows), used by the table, by `child_env` and by the lookup. The lookup test runs on Windows. The sequence test runs the reference `sh` with `Bun.spawn`, beside the Bun Shell run, and checks the exit code and stderr of both. --- src/runtime/shell/EnvMap.rs | 28 +++++++++++++++------------- src/runtime/shell/subproc.rs | 2 +- test/js/bun/shell/bunshell.test.ts | 20 ++++++++++++++------ 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/runtime/shell/EnvMap.rs b/src/runtime/shell/EnvMap.rs index 629d044da742..26f3584f965b 100644 --- a/src/runtime/shell/EnvMap.rs +++ b/src/runtime/shell/EnvMap.rs @@ -29,18 +29,20 @@ impl ArrayHashContext for EnvMapContext { } fn eql(&self, a: &EnvStr, b: &EnvStr, _b_index: usize) -> bool { - #[cfg(windows)] - { - // Must be length-checked: "PATH" must NOT match "PATHEXT". - return bun_core::strings::eql_case_insensitive_asciii_check_length( - a.slice(), - b.slice(), - ); - } - #[cfg(not(windows))] - { - a.slice() == b.slice() - } + name_eql(a.slice(), b.slice()) + } +} + +/// Whether `a` and `b` name the same variable. Names are ASCII-case-insensitive on Windows. +pub(crate) fn name_eql(a: &[u8], b: &[u8]) -> bool { + #[cfg(windows)] + { + // Must be length-checked: "PATH" must NOT match "PATHEXT". + return bun_core::strings::eql_case_insensitive_asciii_check_length(a, b); + } + #[cfg(not(windows))] + { + a == b } } @@ -220,7 +222,7 @@ impl VarTable { ) -> impl core::iter::Iterator { let (names, values): (&[EnvStr], &[EnvStr]) = prefixes.map_or((&[], &[]), |p| (p.map.keys(), p.map.values())); - let replaced = move |name: &EnvStr| names.iter().any(|p| EnvMapContext.eql(p, name, 0)); + let replaced = move |name: &EnvStr| names.iter().any(|p| name_eql(p.slice(), name.slice())); self.map .iter() .filter_map(move |(name, var)| match var { diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index 89451867caba..086038c344af 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1409,7 +1409,7 @@ impl<'a> SpawnArgs<'a> { line[len] = 0; let line: &'a [u8] = line; - if key == b"PATH" { + if sh::env_map::name_eql(key, b"PATH") { self.path = &line[b"PATH=".len()..len]; } diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 91c98801e1ff..f4746ac36803 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -1508,9 +1508,16 @@ booga" ) .join("\n"); - const sh = Bun.spawnSync({ cmd: ["sh", "-c", script], env, stderr: "inherit" }); - const bun = await $`${{ raw: script }}`.env(env).quiet().nothrow(); - expect(bun.stdout.toString().split("\n")).toEqual(sh.stdout.toString().split("\n")); + await using sh = Bun.spawn({ cmd: ["sh", "-c", script], env, stdout: "pipe", stderr: "pipe" }); + const [expected, shStderr, shExitCode, bun] = await Promise.all([ + sh.stdout.text(), + sh.stderr.text(), + sh.exited, + $`${{ raw: script }}`.env(env).quiet().nothrow(), + ]); + expect({ shStderr, shExitCode }).toEqual({ shStderr: "", shExitCode: 0 }); + expect(bun.stdout.toString().split("\n")).toEqual(expected.split("\n")); + expect({ stderr: bun.stderr.toString(), exitCode: bun.exitCode }).toEqual({ stderr: "", exitCode: 0 }); }); TestBuilder.command`A=1 $EMPTY; echo "[$A]"` @@ -1532,8 +1539,7 @@ booga" expect(exitCode).toBe(0); }); - // Windows: the command lookup matches the key PATH byte for byte and the inherited key is spelled Path (#42049). - test.todoIf(isWindows)("a plain assignment to PATH is used by command lookup", async () => { + test("a plain assignment to PATH is used by command lookup", async () => { using dir = tempDir( "shell-path-assign", isWindows @@ -1541,8 +1547,10 @@ booga" : { "onlyintool": "#!/bin/sh\necho from-onlyintool\n" }, ); if (!isWindows) chmodSync(join(String(dir), "onlyintool"), 0o755); + // cmd.exe runs a .cmd file, so Windows keeps the rest of the PATH. + const path = isWindows ? `${dir};${process.env.PATH}` : String(dir); - const { stdout, stderr, exitCode } = await $`PATH=${String(dir)}; onlyintool`.quiet().nothrow(); + const { stdout, stderr, exitCode } = await $`PATH=${path}; onlyintool`.quiet().nothrow(); expect(stderr.toString()).toBe(""); expect(stdout.toString().trim()).toBe("from-onlyintool"); expect(exitCode).toBe(0);