Skip to content

Commit

Permalink
Support specifying multiple possible ENVIRONMENTs. (#7809)
Browse files Browse the repository at this point in the history
Until this PR we supported either all of them or just 1. With this, ENVIRONMENT=web,worker means it can be web or worker, but not node or anything else.

Improves the testing of ENVIRONMENT, which caught a bug where scriptDirectory = __dirname + '/'; needs to be after an assertion (so that the assertion is hit first and shows a clear error message, instead of a weird one later).

See #7801.
  • Loading branch information
kripken committed Jan 11, 2019
1 parent dec86bd commit e9d8b5c
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 29 deletions.
11 changes: 5 additions & 6 deletions src/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,11 @@ Runtime.QUANTUM_SIZE = QUANTUM_SIZE;

// State computations

ENVIRONMENT_MAY_BE_WEB = !ENVIRONMENT || ENVIRONMENT === 'web';
ENVIRONMENT_MAY_BE_WORKER = !ENVIRONMENT || ENVIRONMENT === 'worker';
ENVIRONMENT_MAY_BE_NODE = !ENVIRONMENT || ENVIRONMENT === 'node';
ENVIRONMENT_MAY_BE_SHELL = !ENVIRONMENT || ENVIRONMENT === 'shell';

ENVIRONMENT_MAY_BE_WEB_OR_WORKER = ENVIRONMENT_MAY_BE_WEB || ENVIRONMENT_MAY_BE_WORKER;
var ENVIRONMENTS = ENVIRONMENT.split(',');
ENVIRONMENT_MAY_BE_WEB = !ENVIRONMENT || ENVIRONMENTS.indexOf('web') >= 0;
ENVIRONMENT_MAY_BE_WORKER = !ENVIRONMENT || ENVIRONMENTS.indexOf('worker') >= 0;
ENVIRONMENT_MAY_BE_NODE = !ENVIRONMENT || ENVIRONMENTS.indexOf('node') >= 0;
ENVIRONMENT_MAY_BE_SHELL = !ENVIRONMENT || ENVIRONMENTS.indexOf('shell') >= 0;

if (ENVIRONMENT && !(ENVIRONMENT_MAY_BE_WEB || ENVIRONMENT_MAY_BE_WORKER || ENVIRONMENT_MAY_BE_NODE || ENVIRONMENT_MAY_BE_SHELL)) {
throw 'Invalid environment specified in "ENVIRONMENT": ' + ENVIRONMENT + '. Should be one of: web, worker, node, shell.';
Expand Down
3 changes: 2 additions & 1 deletion src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,8 @@ var LEGACY_VM_SUPPORT = 0;
// 'worker' - a web worker environment.
// 'node' - Node.js.
// 'shell' - a JS shell like d8, js, or jsc.
// Or it can be a comma-separated list of them, e.g., "web,worker". If this is
// the empty string, then all runtime environments are supported.
// (There is also a 'pthread' environment, see shell.js, but it cannot be specified
// manually yet TODO)
var ENVIRONMENT = '';
Expand Down Expand Up @@ -1305,7 +1307,6 @@ var ENVIRONMENT_MAY_BE_WEB = 1;
var ENVIRONMENT_MAY_BE_WORKER = 1;
var ENVIRONMENT_MAY_BE_NODE = 1;
var ENVIRONMENT_MAY_BE_SHELL = 1;
var ENVIRONMENT_MAY_BE_WEB_OR_WORKER = 1;

// Internal: passes information to emscripten.py about whether to minify
// JS -> asm.js import names. Controlled by optimization level, enabled
Expand Down
8 changes: 4 additions & 4 deletions src/shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Module['postRun'] = [];
// Determine the runtime environment we are in. You can customize this by
// setting the ENVIRONMENT setting at compile time (see settings.js).

#if ENVIRONMENT
#if ENVIRONMENT && ENVIRONMENT.indexOf(',') < 0
var ENVIRONMENT_IS_WEB = {{{ ENVIRONMENT === 'web' }}};
var ENVIRONMENT_IS_WORKER = {{{ ENVIRONMENT === 'worker' }}};
var ENVIRONMENT_IS_NODE = {{{ ENVIRONMENT === 'node' }}};
Expand Down Expand Up @@ -100,12 +100,12 @@ function locateFile(path) {

#if ENVIRONMENT_MAY_BE_NODE
if (ENVIRONMENT_IS_NODE) {
scriptDirectory = __dirname + '/';
#if ENVIRONMENT
#if ASSERTIONS
if (!(typeof process === 'object' && typeof require === 'function')) throw new Error('not compiled for this environment (did you build to HTML and try to run it not on the web, or set ENVIRONMENT to something - like node - and run it someplace else - like on the web?)');
#endif
#endif
scriptDirectory = __dirname + '/';

// Expose functionality in the same simple way that the shells work
// Note that we pollute the global namespace here, otherwise we break in node
Expand Down Expand Up @@ -220,7 +220,7 @@ if (ENVIRONMENT_IS_SHELL) {
}
} else
#endif // ENVIRONMENT_MAY_BE_SHELL
#if ENVIRONMENT_MAY_BE_WEB_OR_WORKER
#if ENVIRONMENT_MAY_BE_WEB || ENVIRONMENT_MAY_BE_WORKER
if (ENVIRONMENT_IS_WEB || ENVIRONMENT_IS_WORKER) {
if (ENVIRONMENT_IS_WORKER) { // Check worker, not web, since window could be polyfilled
scriptDirectory = self.location.href;
Expand Down Expand Up @@ -317,7 +317,7 @@ if (ENVIRONMENT_IS_WEB || ENVIRONMENT_IS_WORKER) {

Module['setWindowTitle'] = function(title) { document.title = title };
} else
#endif // ENVIRONMENT_MAY_BE_WEB_OR_WORKER
#endif // ENVIRONMENT_MAY_BE_WEB || ENVIRONMENT_MAY_BE_WORKER
{
#if ASSERTIONS
throw new Error('environment detection error');
Expand Down
50 changes: 32 additions & 18 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -7897,25 +7897,39 @@ def test_wrap_malloc(self):

def test_environment(self):
self.set_setting('ASSERTIONS', 1)

def test():
self.do_run_in_out_file_test('tests', 'core', 'test_hello_world')
js = open('src.cpp.o.js').read()
assert ('require(' in js) == ('node' in self.get_setting('ENVIRONMENT')), 'we should have require() calls only if node js specified'

for engine in JS_ENGINES:
for work in (1, 0):
# set us to test in just this engine
self.banned_js_engines = [e for e in JS_ENGINES if e != engine]
# tell the compiler to build with just that engine
if engine == NODE_JS and work:
self.set_setting('ENVIRONMENT', 'node')
else:
self.set_setting('ENVIRONMENT', 'shell')
print(engine, work, self.get_setting('ENVIRONMENT'))
try:
self.do_run_in_out_file_test('tests', 'core', 'test_hello_world')
except Exception as e:
if not work:
self.assertContained('not compiled for this environment', str(e))
else:
raise
js = open('src.cpp.o.js').read()
assert ('require(' in js) == (self.get_setting('ENVIRONMENT') == 'node'), 'we should have require() calls only if node js specified'
print(engine)
# set us to test in just this engine
self.banned_js_engines = [e for e in JS_ENGINES if e != engine]
# tell the compiler to build with just that engine
if engine == NODE_JS:
right = 'node'
wrong = 'shell'
else:
right = 'shell'
wrong = 'node'
# test with the right env
self.set_setting('ENVIRONMENT', right)
print(' ', self.get_setting('ENVIRONMENT'))
test()
# test with the wrong env
self.set_setting('ENVIRONMENT', wrong)
print(' ', self.get_setting('ENVIRONMENT'))
try:
test()
raise Exception('unexpected success')
except Exception as e:
self.assertContained('not compiled for this environment', str(e))
# test with a combined env
self.set_setting('ENVIRONMENT', right + ',' + wrong)
print(' ', self.get_setting('ENVIRONMENT'))
test()

def test_dfe(self):
if not self.supports_js_dfe():
Expand Down

0 comments on commit e9d8b5c

Please sign in to comment.