Skip to content

Commit bcf911e

Browse files
authored
Remove the need for jscompiler to run with CWD=emscripten/src. NFC (#23519)
The only place that CWD was relied upon internally was the use of the `read()` function in some JS library code that was expecting to be able to read files relative to the `src` directory. To deal with this use case I created a new `readFile` function for compiler usage and modified the exported `read` function to explicitly support `src/` relative files. This change makes the existing `EMCC_BUILD_DIR` environment variable obsolete since it was only needed because we were modifying CWD when running the JS compiler.
1 parent 509d469 commit bcf911e

File tree

8 files changed

+44
-30
lines changed

8 files changed

+44
-30
lines changed

src/compiler.mjs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@
77

88
// JavaScript compiler, main entry point
99

10-
import {Benchmarker, applySettings, loadDefaultSettings, printErr, read} from './utility.mjs';
11-
1210
import assert from 'node:assert';
1311
import {parseArgs} from 'node:util';
12+
import {Benchmarker, applySettings, loadDefaultSettings, printErr, readFile} from './utility.mjs';
1413

1514
loadDefaultSettings();
1615

@@ -31,11 +30,15 @@ Usage: compiler.mjs <settings.json> [-o out.js] [--symbols-only]`);
3130
// Load settings from JSON passed on the command line
3231
const settingsFile = positionals[0];
3332
assert(settingsFile, 'settings file not specified');
34-
const user_settings = JSON.parse(read(settingsFile));
33+
const user_settings = JSON.parse(readFile(settingsFile));
3534
applySettings(user_settings);
3635

3736
export const symbolsOnly = values['symbols-only'];
3837

38+
// TODO(sbc): Remove EMCC_BUILD_DIR at some point. It used to be required
39+
// back when ran the JS compiler with overridden CWD.
40+
process.env['EMCC_BUILD_DIR'] = process.cwd()
41+
3942
// In case compiler.mjs is run directly (as in gen_sig_info)
4043
// ALL_INCOMING_MODULE_JS_API might not be populated yet.
4144
if (!ALL_INCOMING_MODULE_JS_API.length) {

src/jsifier.mjs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// before this stage, which just does the final conversion to JavaScript.
99

1010
import assert from 'node:assert';
11+
import * as path from 'node:path';
1112
import {
1213
ATEXITS,
1314
ATINITS,
@@ -29,10 +30,11 @@ import {
2930
compileTimeContext,
3031
print,
3132
printErr,
32-
read,
33+
readFile,
3334
warn,
3435
warnOnce,
3536
warningOccured,
37+
localFile,
3638
} from './utility.mjs';
3739
import {LibraryManager, librarySymbols} from './modules.mjs';
3840

@@ -137,16 +139,18 @@ function getTransitiveDeps(symbol) {
137139
}
138140

139141
function shouldPreprocess(fileName) {
140-
var content = read(fileName).trim();
142+
var content = readFile(fileName).trim();
141143
return content.startsWith('#preprocess\n') || content.startsWith('#preprocess\r\n');
142144
}
143145

144-
function getIncludeFile(fileName, needsPreprocess) {
146+
function getIncludeFile(fileName, alwaysPreprocess) {
145147
let result = `// include: ${fileName}\n`;
146-
if (needsPreprocess) {
147-
result += processMacros(preprocess(fileName), fileName);
148+
const absFile = path.isAbsolute(fileName) ? fileName : localFile(fileName);
149+
const doPreprocess = alwaysPreprocess || shouldPreprocess(absFile);
150+
if (doPreprocess) {
151+
result += processMacros(preprocess(absFile), fileName);
148152
} else {
149-
result += read(fileName);
153+
result += readFile(absFile);
150154
}
151155
result += `// end include: ${fileName}\n`;
152156
return result;
@@ -155,7 +159,7 @@ function getIncludeFile(fileName, needsPreprocess) {
155159
function preJS() {
156160
let result = '';
157161
for (const fileName of PRE_JS_FILES) {
158-
result += getIncludeFile(fileName, shouldPreprocess(fileName));
162+
result += getIncludeFile(fileName, /*alwaysPreprocess=*/ false);
159163
}
160164
return result;
161165
}
@@ -697,8 +701,8 @@ function(${args}) {
697701
libraryItems.push(JS);
698702
}
699703

700-
function includeFile(fileName, needsPreprocess = true) {
701-
print(getIncludeFile(fileName, needsPreprocess));
704+
function includeFile(fileName, alwaysPreprocess = true) {
705+
print(getIncludeFile(fileName, alwaysPreprocess));
702706
}
703707

704708
function finalCombiner() {
@@ -755,7 +759,7 @@ var proxiedFunctionTable = [
755759
includeFile(postFile);
756760

757761
for (const fileName of POST_JS_FILES) {
758-
includeFile(fileName, shouldPreprocess(fileName));
762+
includeFile(fileName, /*alwaysPreprocess=*/ false);
759763
}
760764

761765
if (MODULARIZE) {

src/modules.mjs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
isDecorator,
1414
isJsOnlySymbol,
1515
error,
16-
read,
16+
readFile,
1717
warn,
1818
setCurrentFile,
1919
printErr,
@@ -273,7 +273,7 @@ export const LibraryManager = {
273273
} catch (e) {
274274
error(`failure to execute js library "${filename}":`);
275275
if (VERBOSE) {
276-
const orig = read(filename);
276+
const orig = readFile(filename);
277277
if (processed) {
278278
error(
279279
`preprocessed source (you can run a js engine on this to get a clearer error message sometimes):\n=============\n${processed}\n=============`,
@@ -317,7 +317,7 @@ let defines = {};
317317
* that can then be used in JavaScript via macros.
318318
*/
319319
function loadStructInfo(filename) {
320-
const temp = JSON.parse(read(filename));
320+
const temp = JSON.parse(readFile(filename));
321321
Object.assign(structs, temp.structs);
322322
Object.assign(defines, temp.defines);
323323
}

src/parseTools.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
addToCompileTimeContext,
1717
error,
1818
printErr,
19-
read,
19+
readFile,
2020
runInMacroContext,
2121
setCurrentFile,
2222
warn,
@@ -64,7 +64,7 @@ function findIncludeFile(filename, currentDir) {
6464
// ident checked is true in our global.
6565
// Also handles #include x.js (similar to C #include <file>)
6666
export function preprocess(filename) {
67-
let text = read(filename);
67+
let text = readFile(filename);
6868
if (EXPORT_ES6 && USE_ES6_IMPORT_META) {
6969
// `eval`, Terser and Closure don't support module syntax; to allow it,
7070
// we need to temporarily replace `import.meta` and `await import` usages

src/utility.mjs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ export function isDecorator(ident) {
224224
return suffixes.some((suffix) => ident.endsWith(suffix));
225225
}
226226

227-
export function read(filename) {
227+
export function readFile(filename) {
228228
return fs.readFileSync(filename, 'utf8');
229229
}
230230

@@ -240,6 +240,15 @@ export function localFile(filename) {
240240
return path.join(srcDir, filename);
241241
}
242242

243+
// Helper function for JS library files that can be used to read files
244+
// relative to the src/ directory.
245+
function read(filename) {
246+
if (!path.isAbsolute(filename)) {
247+
filename = localFile(filename);
248+
}
249+
return readFile(filename);
250+
}
251+
243252
// Anything needed by the script that we load below must be added to the
244253
// global object. These, for example, are all needed by parseTools.js.
245254
export function print(x) {
@@ -311,7 +320,7 @@ export function applySettings(obj) {
311320

312321
export function loadSettingsFile(f) {
313322
const settings = {};
314-
vm.runInNewContext(read(f), settings, {filename: f});
323+
vm.runInNewContext(readFile(f), settings, {filename: f});
315324
applySettings(settings);
316325
return settings;
317326
}

test/test_other.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4961,14 +4961,15 @@ def test_jslib_has_library(self):
49614961
self.do_runf(test_file('hello_world.c'), emcc_args=['-L', '-lfoo.js'])
49624962

49634963
def test_EMCC_BUILD_DIR(self):
4964-
# EMCC_BUILD_DIR env var contains the dir we were building in, when running the js compiler (e.g. when
4965-
# running a js library). We force the cwd to be src/ for technical reasons, so this lets you find out
4966-
# where you were.
4964+
# EMCC_BUILD_DIR was necessary in the past since we used to force the cwd to be src/ for
4965+
# technical reasons.
49674966
create_file('lib.js', r'''
4968-
printErr('dir was ' + process.env.EMCC_BUILD_DIR);
4967+
printErr('EMCC_BUILD_DIR: ' + process.env.EMCC_BUILD_DIR);
4968+
printErr('CWD: ' + process.cwd());
49694969
''')
49704970
err = self.run_process([EMXX, test_file('hello_world.c'), '--js-library', 'lib.js'], stderr=PIPE).stderr
4971-
self.assertContained('dir was ' + os.path.realpath(os.path.normpath(self.get_dir())), err)
4971+
self.assertContained('EMCC_BUILD_DIR: ' + os.path.realpath(os.path.normpath(self.get_dir())), err)
4972+
self.assertContained('CWD: ' + os.path.realpath(os.path.normpath(self.get_dir())), err)
49724973

49734974
def test_float_h(self):
49744975
process = self.run_process([EMCC, test_file('float+.c')], stdout=PIPE, stderr=PIPE)

tools/emscripten.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,14 +183,11 @@ def compile_javascript(symbols_only=False):
183183
json.dump(settings.external_dict(), s, sort_keys=True, indent=2)
184184

185185
# Call js compiler
186-
env = os.environ.copy()
187-
env['EMCC_BUILD_DIR'] = os.getcwd()
188186
args = [settings_file]
189187
if symbols_only:
190188
args += ['--symbols-only']
191189
out = shared.run_js_tool(path_from_root('src/compiler.mjs'),
192-
args, stdout=subprocess.PIPE, stderr=stderr_file,
193-
cwd=path_from_root('src'), env=env, encoding='utf-8')
190+
args, stdout=subprocess.PIPE, stderr=stderr_file, encoding='utf-8')
194191
if symbols_only:
195192
glue = None
196193
forwarded_data = out

tools/maint/gen_sig_info.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ def extract_sig_info(sig_info, extra_settings=None, extra_cflags=None, cxx=False
322322
utils.write_file(settings_json, json.dumps(settings))
323323
output = shared.run_js_tool(utils.path_from_root('src/compiler.mjs'),
324324
['--symbols-only', settings_json],
325-
stdout=subprocess.PIPE, cwd=utils.path_from_root())
325+
stdout=subprocess.PIPE)
326326
symbols = json.loads(output)['deps'].keys()
327327
symbols = [s for s in symbols if not ignore_symbol(s, cxx)]
328328
if cxx:

0 commit comments

Comments
 (0)