Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions src/runtime/shell/EnvMap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ impl EnvMap {
Some(val)
}

/// Removes `key` if present, dereffing the stored key and value it owned.
pub fn remove(&mut self, key: EnvStr) {
if let Some((k, v)) = self.map.fetch_swap_remove(&key) {
k.deref();
v.deref();
}
}

pub fn clone(&self) -> EnvMap {
let new = EnvMap {
map: self.map.clone().expect("OOM"),
Expand Down
39 changes: 33 additions & 6 deletions src/runtime/shell/builtin/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,47 @@ impl Export {
if s.is_empty() {
continue;
}
let (name, value) = match s.iter().position(|&b| b == b'=') {
Some(eq) => (&s[..eq], &s[eq + 1..]),
None => (s, &b""[..]),
let eq = s.iter().position(|&b| b == b'=');
let name = match eq {
Some(eq) => &s[..eq],
None => s,
};
// 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`.
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) };
unsafe {
match eq {
Some(eq) => {
let val = EnvStr::dupe_ref_counted(&s[eq + 1..]);
// A `NAME=value` shell-local entry must not shadow the
// exported binding: `$VAR` expansion checks `shell_env`
// before `export_env`.
(*shell).shell_env.remove(label);
(*shell).export_env.insert(label, val);
val.deref();
}
// `export NAME` gives NAME the export attribute while keeping
// its current value: promote a shell-local value rather than
// blanking it, and leave an already-exported value untouched.
None => {
if let Some(existing) = (*shell).shell_env.get(label) {
(*shell).shell_env.remove(label);
(*shell).export_env.insert(label, existing);
existing.deref();
} else if let Some(existing) = (*shell).export_env.get(label) {
existing.deref();
} else {
let val = EnvStr::dupe_ref_counted(b"");
(*shell).export_env.insert(label, val);
val.deref();
}
}
}
}
label.deref();
val.deref();
}
Builtin::done(interp, cmd, 0)
}
Expand Down
17 changes: 16 additions & 1 deletion src/runtime/shell/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2141,7 +2141,22 @@ impl ShellExecEnv {
) {
match assign_ctx {
AssignCtx::Cmd => self.cmd_local_env.insert(label, value),
AssignCtx::Shell => self.shell_env.insert(label, value),
AssignCtx::Shell => {
// POSIX: a plain assignment to a name that already carries the
// export attribute keeps it exported, so children must see the
// new value. Update the exported binding in place rather than
// shadowing it with a shell-local entry.
if let Some(existing) = self.export_env.get(label) {
existing.deref();
// Drop any shell-local entry so it can't shadow the exported
// value: `$VAR` expansion consults `shell_env` before
// `export_env`.
self.shell_env.remove(label);
Comment thread
claude[bot] marked this conversation as resolved.
self.export_env.insert(label, value);
} else {
self.shell_env.insert(label, value);
}
Comment thread
claude[bot] marked this conversation as resolved.
}
AssignCtx::Exported => self.export_env.insert(label, value),
}
}
Expand Down
50 changes: 50 additions & 0 deletions test/js/bun/shell/bunshell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,56 @@ booga"

expect(procEnv).toEqual({ ...bunEnv, BUN_TEST_VAR: "1", FOO: "bar" });
});

// POSIX: a plain assignment to a name that already carries the export
// attribute keeps it exported, so the new value must reach child processes,
// not just the shell's own `$VAR` view.
test.concurrent("reassigning an exported var updates both the shell and the child", async () => {
const env = { ...bunEnv, OUTER: "fromenv" };
const code = "process.stdout.write('child=' + (process.env.OUTER ?? '<unset>'))";
const { stdout } = await $`OUTER=changed; echo shell=$OUTER; ${BUN} -e ${code}`.env(env);
expect(stdout.toString()).toBe("shell=changed\nchild=changed");
});

test.concurrent("append idiom on an exported var reaches the child", async () => {
const env = { ...bunEnv, OUTER: "fromenv" };
const code = "process.stdout.write(process.env.OUTER ?? '<unset>')";
const { stdout } = await $`OUTER="$OUTER+more"; ${BUN} -e ${code}`.env(env);
expect(stdout.toString()).toBe("fromenv+more");
});

test.concurrent("bare assignment to a non-exported name stays shell-local", async () => {
const code = "process.stdout.write(process.env.NEWV ?? '<unset>')";
const { stdout } = await $`NEWV=nv; echo shell=$NEWV; ${BUN} -e ${code}`.env(bunEnv);
expect(stdout.toString()).toBe("shell=nv\n<unset>");
});

// A shell-local var that is later exported and then reassigned must not keep
// a stale shell-local entry shadowing the exported value: `$VAR` expansion
// checks shell_env before export_env.
test.concurrent("bare reassignment after export clears the shell-local shadow", async () => {
const code = "process.stdout.write('child=' + (process.env.FOO ?? '<unset>'))";
const { stdout } = await $`FOO=a; export FOO=b; FOO=c; echo shell=$FOO; ${BUN} -e ${code}`.env(bunEnv);
expect(stdout.toString()).toBe("shell=c\nchild=c");
});

test.concurrent("export NAME=value overrides a prior shell-local value", async () => {
const code = "process.stdout.write('child=' + (process.env.FOO ?? '<unset>'))";
const { stdout } = await $`FOO=a; export FOO=b; echo shell=$FOO; ${BUN} -e ${code}`.env(bunEnv);
expect(stdout.toString()).toBe("shell=b\nchild=b");
});

test.concurrent("export NAME promotes an existing shell-local value", async () => {
const code = "process.stdout.write('child=' + (process.env.FOO ?? '<unset>'))";
const { stdout } = await $`FOO=a; export FOO; echo shell=$FOO; ${BUN} -e ${code}`.env(bunEnv);
expect(stdout.toString()).toBe("shell=a\nchild=a");
});

test.concurrent("export NAME leaves an already-exported value intact", async () => {
const code = "process.stdout.write('child=' + (process.env.FOO ?? '<unset>'))";
const { stdout } = await $`export FOO=b; export FOO; echo shell=$FOO; ${BUN} -e ${code}`.env(bunEnv);
expect(stdout.toString()).toBe("shell=b\nchild=b");
});
});

describe("cd & pwd", () => {
Expand Down
Loading