Skip to content

Commit 2420d3e

Browse files
authored
Support script arguments for environmentSetupScript in form of "script path" [arg...] (#4764)
1 parent 30a6acf commit 2420d3e

3 files changed

Lines changed: 37 additions & 5 deletions

File tree

docs/kits.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,8 @@ The following additional options may be specified:
157157
158158
`environmentSetupScript`
159159

160-
> The absolute path to a script that modifies/adds environment variables for the kit. Uses `call` on Windows and `source` in `bash` otherwise.
160+
> The absolute path to a script or a string in form of `"script path" [arg ...]`that modifies/adds environment variables for the kit.
161+
Uses `call` on Windows and `source` in `bash` otherwise.
161162

162163
`description`
163164

src/kits/kit.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -695,18 +695,24 @@ export async function getShellScriptEnvironment(kit: Kit, opts?: expand.Expansio
695695
let script = '';
696696
let run_command = '';
697697

698-
let environmentSetupScript = kit.environmentSetupScript?.trim();
698+
let environmentSetupScript = kit.environmentSetupScript!.trim();
699699
if (opts) {
700-
environmentSetupScript = await expand.expandString(environmentSetupScript!, opts);
700+
environmentSetupScript = await expand.expandString(environmentSetupScript, opts);
701+
}
702+
703+
// If the string doesn't start with a quote, assume it is a path to a script file, so we quote it in case there are spaces in the path.
704+
// Otherwise, assume it is in form of `"script path" [arg ...]`, we will not add extra quotes, to avoid breaking the command.
705+
if (!environmentSetupScript.startsWith('"')) {
706+
environmentSetupScript = `"${environmentSetupScript}"`;
701707
}
702708

703709
if (process.platform === 'win32') { // windows
704-
script += `call "${environmentSetupScript}"\r\n`; // call the user batch script
710+
script += `call ${environmentSetupScript}\r\n`; // call the user batch script
705711
script += `set >> "${environment_path}"`; // write env vars to temp file
706712
// Quote the script file path before running it, in case there are spaces.
707713
run_command = `call "${script_path}"`;
708714
} else { // non-windows
709-
script += `source "${environmentSetupScript}"\n`; // run the user shell script
715+
script += `source ${environmentSetupScript}\n`; // run the user shell script
710716
script += `printenv >> ${environment_path}`; // write env vars to temp file
711717
run_command = `/bin/bash -c "source ${script_path}"`; // run script in bash to enable bash-builtin commands like 'source'
712718
}

test/unit-tests/kitmanager.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,29 @@ suite('Kits test', () => {
6868
expect(env_vars_arr).to.deep.include(['TESTVAR13', 'cde']);
6969
}
7070
});
71+
72+
test('Test load env vars from shell script with arguments', async () => {
73+
const fname_extension = process.platform === 'win32' ? 'bat' : 'sh';
74+
const fname = `cmake-kit-test-${Math.random().toString()}.${fname_extension}`;
75+
const script_path = path.join(paths.tmpDir, fname);
76+
// generate a file with test batch / shell script that sets two env vars
77+
if (process.platform === 'win32') {
78+
await fs.writeFile(script_path, `set "TESTVAR12=%~1"\r\nset "TESTVAR13=%~2"`);
79+
} else {
80+
await fs.writeFile(script_path, `export "TESTVAR12=$1"\nexport "TESTVAR13=$2"`);
81+
}
82+
83+
const kit = { name: "Test Kit 2", environmentSetupScript: `"${script_path}" abc cde`, isTrusted: true };
84+
const env_vars = await getShellScriptEnvironment(kit);
85+
await fs.unlink(script_path);
86+
expect(env_vars).to.not.be.undefined;
87+
88+
if (env_vars) {
89+
const env_vars_arr = Array.from(Object.entries(env_vars));
90+
// must contain all env vars, not only the ones we defined!
91+
expect(env_vars_arr.length).to.be.greaterThan(2);
92+
expect(env_vars_arr).to.deep.include(['TESTVAR12', 'abc']);
93+
expect(env_vars_arr).to.deep.include(['TESTVAR13', 'cde']);
94+
}
95+
});
7196
});

0 commit comments

Comments
 (0)