Skip to content

Commit 6c8a8d0

Browse files
richardo2016xicilion
richardo2016
authored and
xicilion
committed
process, fix: make requried envs' kv fallback to the ones of parent process. (#516)
1 parent efa57be commit 6c8a8d0

File tree

13 files changed

+336
-31
lines changed

13 files changed

+336
-31
lines changed

‎fibjs/src/process/SubProcess_posix.cpp

+27-6
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "config.h"
1111
#include "object.h"
1212
#include "ifs/process.h"
13+
#include "ifs/util.h"
1314
#include "SubProcess.h"
1415
#include <spawn.h>
1516
#include <vector>
@@ -19,6 +20,11 @@
1920

2021
namespace fibjs {
2122

23+
static const char* DEFT_ENV_KEYS[] = {
24+
"HOME",
25+
"TMPDIR"
26+
};
27+
2228
static exlib::spinlock s_lock;
2329
static std::map<pid_t, obj_ptr<SubProcess>> s_ids;
2430
void init_signal();
@@ -151,20 +157,35 @@ result_t SubProcess::create(exlib::string command, v8::Local<v8::Array> args, v8
151157
std::vector<exlib::string> envstr;
152158
std::vector<char*> envp;
153159

154-
v8::Local<v8::Object> envs;
160+
v8::Local<v8::Object> cur_envs;
161+
hr = process_base::get_env(cur_envs);
162+
if (hr < 0)
163+
return hr;
155164

156-
hr = GetConfigValue(isolate->m_isolate, opts, "env", envs, true);
165+
v8::Local<v8::Value> opt_envs_v;
166+
hr = GetConfigValue(isolate->m_isolate, opts, "env", opt_envs_v, true);
157167
if (hr == CALL_E_PARAMNOTOPTIONAL)
158-
hr = process_base::get_env(envs);
159-
if (hr < 0)
168+
util_base::clone(cur_envs, opt_envs_v);
169+
else if (hr < 0)
160170
return hr;
171+
172+
v8::Local<v8::Object> opt_envs = opt_envs_v->ToObject();
173+
v8::Local<v8::Value> dflt_k;
174+
bool has_k;
175+
for (int32_t i = 0; i < (int32_t)ARRAYSIZE(DEFT_ENV_KEYS); i++) {
176+
util_base::has(opt_envs, DEFT_ENV_KEYS[i], has_k);
177+
if (!has_k) {
178+
dflt_k = isolate->NewString(DEFT_ENV_KEYS[i]);
179+
opt_envs->Set(dflt_k, cur_envs->Get(dflt_k));
180+
}
181+
}
161182

162-
v8::Local<v8::Array> keys = envs->GetPropertyNames();
183+
v8::Local<v8::Array> keys = opt_envs->GetPropertyNames();
163184
len = (int32_t)keys->Length();
164185

165186
for (i = 0; i < len; i++) {
166187
v8::Local<v8::Value> k = keys->Get(i);
167-
v8::Local<v8::Value> v = envs->Get(k);
188+
v8::Local<v8::Value> v = opt_envs->Get(k);
168189
exlib::string ks, vs;
169190

170191
hr = GetArgumentValue(k, ks);

‎fibjs/src/process/SubProcess_win.cpp

+36-6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "object.h"
1111
#include "ifs/process.h"
12+
#include "ifs/util.h"
1213
#include "SubProcess.h"
1314
#include <vector>
1415
#include <psapi.h>
@@ -17,6 +18,20 @@
1718

1819
namespace fibjs {
1920

21+
static const char* DEFT_ENV_KEYS[] = {
22+
"SystemRoot",
23+
"TEMP",
24+
"TMP",
25+
// for registry :start
26+
"CommonProgramFiles",
27+
"CommonProgramFiles(x86)",
28+
"CommonProgramW6432",
29+
"ProgramFiles",
30+
"ProgramFiles(x86)",
31+
"ProgramW6432"
32+
// for registry :end
33+
};
34+
2035
void init_signal();
2136

2237
void init_process()
@@ -134,20 +149,35 @@ result_t SubProcess::create(exlib::string command, v8::Local<v8::Array> args, v8
134149

135150
exlib::wstring envstr;
136151

137-
v8::Local<v8::Object> envs;
152+
v8::Local<v8::Object> cur_envs;
153+
hr = process_base::get_env(cur_envs);
154+
if (hr < 0)
155+
return hr;
138156

139-
hr = GetConfigValue(isolate->m_isolate, opts, "env", envs, true);
157+
v8::Local<v8::Value> opt_envs_v;
158+
hr = GetConfigValue(isolate->m_isolate, opts, "env", opt_envs_v, true);
140159
if (hr == CALL_E_PARAMNOTOPTIONAL)
141-
hr = process_base::get_env(envs);
142-
if (hr < 0)
160+
util_base::clone(cur_envs, opt_envs_v);
161+
else if (hr < 0)
143162
return hr;
163+
164+
v8::Local<v8::Object> opt_envs = opt_envs_v->ToObject();
165+
v8::Local<v8::Value> dflt_k;
166+
bool has_k;
167+
for (int32_t i = 0; i < (int32_t)ARRAYSIZE(DEFT_ENV_KEYS); i++) {
168+
util_base::has(opt_envs, DEFT_ENV_KEYS[i], has_k);
169+
if (!has_k) {
170+
dflt_k = isolate->NewString(DEFT_ENV_KEYS[i]);
171+
opt_envs->Set(dflt_k, cur_envs->Get(dflt_k));
172+
}
173+
}
144174

145-
v8::Local<v8::Array> keys = envs->GetPropertyNames();
175+
v8::Local<v8::Array> keys = opt_envs->GetPropertyNames();
146176
len = (int32_t)keys->Length();
147177

148178
for (i = 0; i < len; i++) {
149179
v8::Local<v8::Value> k = keys->Get(i);
150-
v8::Local<v8::Value> v = envs->Get(k);
180+
v8::Local<v8::Value> v = opt_envs->Get(k);
151181
exlib::string ks, vs;
152182

153183
hr = GetArgumentValue(k, ks);

‎idl/us-en/process.idl

+6-6
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ module process
9696
@code
9797
{
9898
"timeout": 100, // unit ms
99-
"envs": [] // Enviroment variable of child process.
99+
"env": {} // Enviroment variable of child process.
100100
}
101101
@endcode
102102
@return Child process object containing result of command.
@@ -109,7 +109,7 @@ module process
109109
@code
110110
{
111111
"timeout": 100, // unit ms
112-
"envs": [] // Environment variable of child process.
112+
"env": {} // Environment variable of child process.
113113
}
114114
@endcode
115115
@return Child process object containing result of command.
@@ -123,7 +123,7 @@ module process
123123
@code
124124
{
125125
"timeout": 100, // unit ms
126-
"envs": [] // Environment variable of child process.
126+
"env": {} // Environment variable of child process.
127127
}
128128
@endcode
129129
@return Child process object containing result of the command.
@@ -136,7 +136,7 @@ module process
136136
@code
137137
{
138138
"timeout": 100, // unit ms
139-
"envs": [] // Environment variable of child process.
139+
"env": {} // Environment variable of child process.
140140
}
141141
@endcode
142142
@return Child process object containing result of the command.
@@ -150,7 +150,7 @@ module process
150150
@code
151151
{
152152
"timeout": 100, // unit ms
153-
"envs": [] // Environment variable of child process.
153+
"env": {} // Environment variable of child process.
154154
}
155155
@endcode
156156
@return Result code of bash command.
@@ -163,7 +163,7 @@ module process
163163
@code
164164
{
165165
"timeout": 100, // unit ms
166-
"envs": [] // Environment variable of child process.
166+
"env": {} // Environment variable of child process.
167167
}
168168
@endcode
169169
@return Result code of bash command.

‎idl/zh-cn/process.idl

+6-6
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ module process : EventEmitter
157157
```JavaScript
158158
{
159159
"timeout": 100, // 单位为 ms
160-
"envs": [] // 进程环境变量
160+
"env": {} // 进程环境变量
161161
}
162162
```
163163
@param command 指定运行的命令行
@@ -173,7 +173,7 @@ module process : EventEmitter
173173
```JavaScript
174174
{
175175
"timeout": 100, // 单位为 ms
176-
"envs": [] // 进程环境变量
176+
"env": {} // 进程环境变量
177177
}
178178
```
179179
@param command 指定运行的命令行
@@ -188,7 +188,7 @@ module process : EventEmitter
188188
```JavaScript
189189
{
190190
"timeout": 100, // 单位为 ms
191-
"envs": [] // 进程环境变量
191+
"env": {} // 进程环境变量
192192
}
193193
```
194194
@param command 指定运行的命令行
@@ -204,7 +204,7 @@ module process : EventEmitter
204204
```JavaScript
205205
{
206206
"timeout": 100, // 单位为 ms
207-
"envs": [] // 进程环境变量
207+
"env": {} // 进程环境变量
208208
}
209209
```
210210
@param command 指定运行的命令行
@@ -219,7 +219,7 @@ module process : EventEmitter
219219
```JavaScript
220220
{
221221
"timeout": 100, // 单位为 ms
222-
"envs": [] // 进程环境变量
222+
"env": {} // 进程环境变量
223223
}
224224
```
225225
@param command 指定运行的命令行
@@ -235,7 +235,7 @@ module process : EventEmitter
235235
```JavaScript
236236
{
237237
"timeout": 100, // 单位为 ms
238-
"envs": [] // 进程环境变量
238+
"env": {} // 进程环境变量
239239
}
240240
```
241241
@param command 指定运行的命令行

‎test/_subprocess_check/.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
process_env_keys.*.json

‎test/_subprocess_check/generator.process_env_keys.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* @description run this script to generate env keys for your device.
3+
*/
4+
const fs = require('fs')
5+
const path = require('path')
6+
7+
const keys = Object.keys(process.env)
8+
9+
fs.writeTextFile(
10+
path.resolve(__dirname, `process_env_keys.${process.platform}.json`),
11+
JSON.stringify(keys, null, '\t')
12+
)

‎test/_subprocess_check/index.js

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/**
2+
* @description run this script to check if any module's test case invalid in SubProcess
3+
*/
4+
5+
const test = require("test");
6+
test.setup();
7+
8+
const path = require("path");
9+
const util = require("util");
10+
const cmd = process.execPath;
11+
12+
describe("Run other Test Case In SubProcess", () => {
13+
it(`run: ${process.platform}`, () => {
14+
if (process.platform === 'win32') {
15+
const retcode = process.run(cmd, [path.join(__dirname, '../main.js')], {
16+
env1: {
17+
...util.pick(process.env, [
18+
"SystemRoot",
19+
"TEMP",
20+
"TMP",
21+
"CommonProgramFiles",
22+
"CommonProgramFiles(x86)",
23+
"CommonProgramW6432",
24+
"ProgramFiles",
25+
"ProgramFiles(x86)",
26+
"ProgramW6432",
27+
]),
28+
CI_SUBPROCESS_CHECK: 1
29+
}
30+
});
31+
32+
assert.equal(retcode, 0);
33+
}
34+
35+
if (process.platform === 'darwin') {
36+
const retcode = process.run(cmd, [path.join(__dirname, '../main.js')], {
37+
env: {
38+
HOME: process.env.HOME,
39+
TMPDIR: process.env.TMPDIR,
40+
CI_SUBPROCESS_CHECK: 1
41+
}
42+
});
43+
44+
assert.equal(retcode, 0);
45+
}
46+
47+
if (process.platform === 'linux') {
48+
const retcode = process.run(cmd, [path.join(__dirname, '../main.js')], {
49+
env: {
50+
HOME: process.env.HOME,
51+
TMPDIR: process.env.TMPDIR,
52+
CI_SUBPROCESS_CHECK: 1
53+
}
54+
});
55+
56+
assert.equal(retcode, 0);
57+
}
58+
});
59+
});
60+
61+
require.main === module && test.run(console.DEBUG);

‎test/gui_test.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ test.setup();
33

44
var test_util = require('./test_util');
55

6-
var os = require("os");
76
var path = require("path");
87

9-
var win = process.platform == 'win32';
8+
var win = process.platform === 'win32';
109

1110
var html =
1211
`<html>

‎test/main.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
var test = require("test");
44
test.setup();
55

6-
var coroutine = require('coroutine');
7-
86
global.full_test = process.argv.indexOf('full') >= 0;
97

8+
const CI_SUBPROCESS_CHECK = !!process.env.CI_SUBPROCESS_CHECK;
9+
1010
run("./assert_test.js");
1111
run("./test_test.js");
1212
run("./console_test.js");
@@ -29,7 +29,7 @@ run("./fs_test.js");
2929
run("./ms_test.js");
3030
run("./io_test.js");
3131
run("./os_test.js");
32-
run("./process_test.js");
32+
!CI_SUBPROCESS_CHECK && run("./process_test.js");
3333
run("./encoding_test.js");
3434
run("./json_test.js");
3535
run("./module_test.js");
@@ -71,6 +71,6 @@ run("./v8_test.js");
7171
run("./getter_throw.js")
7272

7373
run("./internal_test/helpers.js")
74-
run("./opt_tools_test/index.js")
74+
!CI_SUBPROCESS_CHECK && run("./opt_tools_test/index.js")
7575

7676
process.exitCode = test.run();

‎test/opt_tools_test/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ chdirAndDo(
1818

1919
const processRunOptions = {
2020
env: {
21-
...process.env,
21+
// all required environment would fallback to the parent's one
2222
FIBJS_SILENT_INSALL: 1
2323
}
2424
}

‎test/process/exec.dns.js

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
const assert = require('assert')
2+
const dns = require('dns')
3+
4+
dns.resolve('fibjs.org');
5+
console.log('resolve domain success!');
6+
7+
dns.lookup('fibjs.org');
8+
console.log('lookup domain success!');

0 commit comments

Comments
 (0)