Skip to content

Commit 9ca511f

Browse files
gagglegaggle
committed
Refactor: thread env explicitly through shellcode/datadir
shellcode() and datadir() read PATH and XDG_DATA_HOME from Deno.env implicitly at codegen time, which made the integration test for issue #51 noisy: it had to mutate process env, save originals, and restore them so the generated shellcode would point at the test's temp dirs. This change makes env an explicit parameter (defaulting to Deno.env.toObject()) so production callers in app.ts stay unchanged, but tests can pass a clean env per-invocation. Test drops from 130 to 68 lines, without all the save/restore ceremony.
1 parent aba9421 commit 9ca511f

2 files changed

Lines changed: 43 additions & 108 deletions

File tree

‎src/shellcode().test.ts‎

Lines changed: 28 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -2,133 +2,64 @@ import { assert, assertEquals } from "jsr:@std/assert";
22
import { Path } from "libpkgx";
33
import shellcode, { datadir } from "./shellcode().ts";
44

5-
// Exercises `_pkgx_chpwd_hook` end-to-end via a zsh subprocess with a fake
6-
// `dev` binary on PATH. Reproduces the bug from issue #51 where cd-ing
7-
// directly into a subdir of an activated devenv fails to activate and emits
8-
// `permission denied`.
9-
10-
async function run_hook_in_subdir(): Promise<
11-
{ stdout: string; stderr: string; recorded_args: string[] }
12-
> {
5+
// Issue #51: cd-ing directly into a subdir of an already-activated devenv
6+
// must activate the devenv (and not emit `permission denied`). This drives
7+
// the generated shellcode through a real zsh subprocess with a fake `dev`
8+
// on PATH so we can assert how the chpwd hook invokes it.
9+
Deno.test("chpwd hook activates when cd-ing into subdir of devenv (#51)", async () => {
1310
const tmp = Path.mktemp();
1411
const proj = tmp.join("proj").mkdir();
1512
const sub = proj.join("sub").mkdir();
16-
17-
// Override XDG_DATA_HOME so datadir() lives in the temp tree.
1813
const xdg = tmp.join("xdg").mkdir();
14+
const bin = tmp.join("bin").mkdir();
15+
const log = tmp.join("dev-args.log");
1916

20-
// Pre-create the activation marker for `proj`.
21-
const marker_dir = xdg.join(
22-
"pkgx",
23-
"dev",
24-
proj.string.slice(1),
25-
).mkdir("p");
26-
marker_dir.join("dev.pkgx.activated").touch();
17+
// pre-activate `proj` (not `sub`) — that's the case from the bug report
18+
xdg.join("pkgx", "dev").join(proj.string.slice(1)).mkdir("p")
19+
.join("dev.pkgx.activated").touch();
2720

28-
// Fake `dev` that records its argv and emits a sentinel shell command we
29-
// can detect in stdout. Crucially: the real `dev` (with no args) would
30-
// sniff $PWD, find nothing, and exit 1 — the bug we're testing.
31-
const fake_bin = tmp.join("bin").mkdir();
32-
const fake_dev = fake_bin.join("dev");
33-
const log = tmp.join("dev-args.log");
21+
// fake `dev` records its argv and emits a sentinel for eval to run
22+
const fake_dev = bin.join("dev");
3423
Deno.writeTextFileSync(
3524
fake_dev.string,
36-
`#!/bin/sh\nprintf '%s\\n' "$@" >> "${log.string}"\necho "echo HOOK_OK"\n`,
25+
`#!/bin/sh\nprintf '%s\\n' "$@" >> "${log}"\necho 'echo HOOK_OK'\n`,
3726
);
3827
Deno.chmodSync(fake_dev.string, 0o755);
3928

4029
const env = {
4130
...Deno.env.toObject(),
31+
PATH: `${bin}:${Deno.env.get("PATH") ?? ""}`,
4232
XDG_DATA_HOME: xdg.string,
43-
PATH: `${fake_bin.string}:${Deno.env.get("PATH") ?? ""}`,
4433
};
4534

46-
// Generate shellcode using a PATH that resolves `dev` to our fake and an
47-
// XDG_DATA_HOME that points at our temp datadir (both are baked in at
48-
// codegen time).
49-
const original_path = Deno.env.get("PATH");
50-
const original_xdg = Deno.env.get("XDG_DATA_HOME");
51-
Deno.env.set("PATH", env.PATH);
52-
Deno.env.set("XDG_DATA_HOME", xdg.string);
53-
let code: string;
54-
try {
55-
code = shellcode();
56-
} finally {
57-
if (original_path !== undefined) Deno.env.set("PATH", original_path);
58-
if (original_xdg === undefined) Deno.env.delete("XDG_DATA_HOME");
59-
else Deno.env.set("XDG_DATA_HOME", original_xdg);
60-
}
61-
62-
// Simulate the user: shell starts in HOME, then cd's directly into the
63-
// subdirectory of the already-activated project.
64-
const script = `
65-
${code}
66-
cd "${sub.string}"
67-
`;
68-
69-
const script_path = tmp.join("script.zsh");
70-
Deno.writeTextFileSync(script_path.string, script);
71-
7235
const proc = await new Deno.Command("zsh", {
73-
args: [script_path.string],
36+
args: ["-c", `${shellcode(env)}\ncd "${sub}"`],
7437
env,
7538
stdout: "piped",
7639
stderr: "piped",
7740
}).output();
7841

79-
const recorded_args = log.isFile()
80-
? Deno.readTextFileSync(log.string).split("\n").filter((x) => x.length > 0)
42+
const stdout = new TextDecoder().decode(proc.stdout);
43+
const stderr = new TextDecoder().decode(proc.stderr);
44+
const dev_args = log.isFile()
45+
? Deno.readTextFileSync(log.string).split("\n").filter(Boolean)
8146
: [];
8247

83-
return {
84-
stdout: new TextDecoder().decode(proc.stdout),
85-
stderr: new TextDecoder().decode(proc.stderr),
86-
recorded_args,
87-
};
88-
}
89-
90-
Deno.test("chpwd hook activates when cd-ing directly into subdir of devenv", async () => {
91-
const { stdout, stderr, recorded_args } = await run_hook_in_subdir();
92-
93-
// The hook must invoke our fake dev and run its emitted shellcode.
9448
assert(
9549
stdout.includes("HOOK_OK"),
96-
`expected hook to eval dev's stdout (HOOK_OK), got stdout=${
97-
JSON.stringify(stdout)
98-
} stderr=${JSON.stringify(stderr)}`,
50+
`hook should eval dev's stdout. stdout=${stdout} stderr=${stderr}`,
9951
);
100-
101-
// Crucially: no "permission denied" from the shell trying to execute a
102-
// directory path as a command (the bug from issue #51).
103-
assert(
104-
!/permission denied/i.test(stderr),
105-
`unexpected 'permission denied' in stderr: ${stderr}`,
106-
);
107-
108-
// dev must have been invoked with the activated dir so it sniffs the
109-
// right place — not invoked bare while $PWD points at the subdir.
52+
assertEquals(stderr, "", "hook should produce no stderr");
11053
assertEquals(
111-
recorded_args.length,
112-
1,
113-
`expected dev to be called with exactly one argument, got: ${
114-
JSON.stringify(recorded_args)
115-
}`,
116-
);
117-
assert(
118-
recorded_args[0].endsWith("/proj"),
119-
`expected dev to be called with the activated dir, got: ${
120-
recorded_args[0]
121-
}`,
54+
dev_args,
55+
[proj.string],
56+
"dev must be invoked with the activated dir, not bare",
12257
);
12358
});
12459

12560
Deno.test("datadir respects XDG_DATA_HOME", () => {
126-
const original = Deno.env.get("XDG_DATA_HOME");
127-
try {
128-
Deno.env.set("XDG_DATA_HOME", "/tmp/xdg-test");
129-
assertEquals(datadir().string, "/tmp/xdg-test/pkgx/dev");
130-
} finally {
131-
if (original === undefined) Deno.env.delete("XDG_DATA_HOME");
132-
else Deno.env.set("XDG_DATA_HOME", original);
133-
}
61+
assertEquals(
62+
datadir({ XDG_DATA_HOME: "/tmp/xdg-test" }).string,
63+
"/tmp/xdg-test/pkgx/dev",
64+
);
13465
});

‎src/shellcode().ts‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
import { Path } from "libpkgx";
22

3-
export default function shellcode() {
3+
type Env = Record<string, string>;
4+
5+
export default function shellcode(env: Env = Deno.env.toObject()) {
46
// find self
5-
const dev_cmd = Deno.env.get("PATH")?.split(":").map((path) =>
7+
const dev_cmd = env.PATH?.split(":").map((path) =>
68
Path.abs(path)?.join("dev")
79
)
810
.filter((x) => x?.isExecutableFile())[0];
911

1012
if (!dev_cmd) throw new Error("couldn’t find `dev`");
1113

14+
const dd = datadir(env);
15+
1216
return `
1317
_pkgx_chpwd_hook() {
1418
if ! type _pkgx_dev_try_bye >/dev/null 2>&1 || _pkgx_dev_try_bye; then
1519
dir="$PWD"
1620
while [ "$dir" != / -a "$dir" != . ]; do
17-
if [ -f "${datadir()}/$dir/dev.pkgx.activated" ]; then
21+
if [ -f "${dd}/$dir/dev.pkgx.activated" ]; then
1822
eval "$(${dev_cmd} "$dir")"
1923
break
2024
fi
@@ -29,8 +33,8 @@ dev() {
2933
if type -f _pkgx_dev_try_bye >/dev/null 2>&1; then
3034
dir="$PWD"
3135
while [ "$dir" != / -a "$dir" != . ]; do
32-
if [ -f "${datadir()}/$dir/dev.pkgx.activated" ]; then
33-
rm "${datadir()}/$dir/dev.pkgx.activated"
36+
if [ -f "${dd}/$dir/dev.pkgx.activated" ]; then
37+
rm "${dd}/$dir/dev.pkgx.activated"
3438
break
3539
fi
3640
dir="$(dirname "$dir")"
@@ -43,8 +47,8 @@ dev() {
4347
if [ "$2" ]; then
4448
"${dev_cmd}" "$@"
4549
elif ! type -f _pkgx_dev_try_bye >/dev/null 2>&1; then
46-
mkdir -p "${datadir()}$PWD"
47-
touch "${datadir()}$PWD/dev.pkgx.activated"
50+
mkdir -p "${dd}$PWD"
51+
touch "${dd}$PWD/dev.pkgx.activated"
4852
eval "$(${dev_cmd})"
4953
else
5054
echo "devenv already active" >&2
@@ -77,19 +81,19 @@ fi
7781
`.trim();
7882
}
7983

80-
export function datadir() {
84+
export function datadir(env: Env = Deno.env.toObject()) {
8185
return new Path(
82-
Deno.env.get("XDG_DATA_HOME")?.trim() || platform_data_home_default(),
86+
env.XDG_DATA_HOME?.trim() || platform_data_home_default(env),
8387
).join("pkgx", "dev");
8488
}
8589

86-
function platform_data_home_default() {
90+
function platform_data_home_default(env: Env) {
8791
const home = Path.home();
8892
switch (Deno.build.os) {
8993
case "darwin":
9094
return home.join("Library/Application Support");
9195
case "windows": {
92-
const LOCALAPPDATA = Deno.env.get("LOCALAPPDATA");
96+
const LOCALAPPDATA = env.LOCALAPPDATA;
9397
if (LOCALAPPDATA) {
9498
return new Path(LOCALAPPDATA);
9599
} else {

0 commit comments

Comments
 (0)