Skip to content

Commit

Permalink
Make use of pathlib.Path when dealing with paths within emscripten. N…
Browse files Browse the repository at this point in the history
…FC (emscripten-core#14916)

This change is very much like that large change I made to the test
suite. See emscripten-core#14191 and emscripten-core#14175.

It enables is to use unix style paths rather than arrays of path
components which has several advantages.  As well as beeing more
readable and writable it also allows for things like `git grep
"musl/src"` to work as expected.  For an example of how much more
readable it makes the code see `tools/system_libs.py`.

This change was made almost exclusively with a bunch of `sed`
commands.

Because there is likely some minor performance overhead I did
some measurements.

Running `./tests/runner wasm2 --skip-slow`:

before:
real    1m10.403s
user    49m40.135s
sys     6m14.466s
after:
real    1m10.397s
user    49m43.849s
sys     6m10.698s

Running `./embuilder build libc --force`:

before:
real    0m5.597s
user    3m2.721s
sys     0m50.843s
after:
real    0m5.609s
user    3m0.998s
sys     0m49.796s
  • Loading branch information
sbc100 authored Aug 20, 2021
1 parent 60da014 commit d8ec08a
Show file tree
Hide file tree
Showing 24 changed files with 153 additions and 149 deletions.
48 changes: 24 additions & 24 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@


import emscripten
from tools import shared, system_libs
from tools import shared, system_libs, utils
from tools import colored_logger, diagnostics, building
from tools.shared import unsuffixed, unsuffixed_basename, WINDOWS, safe_copy
from tools.shared import run_process, read_and_preprocess, exit_with_error, DEBUG
Expand Down Expand Up @@ -240,7 +240,7 @@ def __init__(self):
self.embed_files = []
self.exclude_files = []
self.ignore_dynamic_linking = False
self.shell_path = shared.path_from_root('src', 'shell.html')
self.shell_path = utils.path_from_root('src/shell.html')
self.source_map_base = ''
self.emrun = False
self.cpu_profiler = False
Expand Down Expand Up @@ -979,7 +979,7 @@ def run(args):
# site/build/text/docs/tools_reference/emcc.txt
# This then needs to be copied to its final home in docs/emcc.txt from where
# we read it here. We have CI rules that ensure its always up-to-date.
with open(shared.path_from_root('docs', 'emcc.txt'), 'r') as f:
with open(utils.path_from_root('docs/emcc.txt'), 'r') as f:
print(f.read())

print('''
Expand Down Expand Up @@ -1023,7 +1023,7 @@ def run(args):
args = [x for x in args if x != '--cflags']
with misc_temp_files.get_file(suffix='.o') as temp_target:
input_file = 'hello_world.c'
cmd = [shared.PYTHON, sys.argv[0], shared.path_from_root('tests', input_file), '-v', '-c', '-o', temp_target] + args
cmd = [shared.PYTHON, sys.argv[0], utils.path_from_root('tests', input_file), '-v', '-c', '-o', temp_target] + args
proc = run_process(cmd, stderr=PIPE, check=False)
if proc.returncode != 0:
print(proc.stderr)
Expand Down Expand Up @@ -1367,19 +1367,19 @@ def phase_linker_setup(options, state, newargs, settings_map):
add_link_flag(state, sys.maxsize, f)

if options.emrun:
options.pre_js += read_file(shared.path_from_root('src', 'emrun_prejs.js')) + '\n'
options.post_js += read_file(shared.path_from_root('src', 'emrun_postjs.js')) + '\n'
options.pre_js += read_file(utils.path_from_root('src/emrun_prejs.js')) + '\n'
options.post_js += read_file(utils.path_from_root('src/emrun_postjs.js')) + '\n'
# emrun mode waits on program exit
settings.EXIT_RUNTIME = 1

if options.cpu_profiler:
options.post_js += read_file(shared.path_from_root('src', 'cpuprofiler.js')) + '\n'
options.post_js += read_file(utils.path_from_root('src/cpuprofiler.js')) + '\n'

if options.memory_profiler:
settings.MEMORYPROFILER = 1

if options.thread_profiler:
options.post_js += read_file(shared.path_from_root('src', 'threadprofiler.js')) + '\n'
options.post_js += read_file(utils.path_from_root('src/threadprofiler.js')) + '\n'

if options.memory_init_file is None:
options.memory_init_file = settings.OPT_LEVEL >= 2
Expand Down Expand Up @@ -2030,8 +2030,8 @@ def check_memory_setting(setting):

if settings.MINIMAL_RUNTIME:
# Minimal runtime uses a different default shell file
if options.shell_path == shared.path_from_root('src', 'shell.html'):
options.shell_path = shared.path_from_root('src', 'shell_minimal_runtime.html')
if options.shell_path == utils.path_from_root('src/shell.html'):
options.shell_path = utils.path_from_root('src/shell_minimal_runtime.html')

if settings.EXIT_RUNTIME:
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['proc_exit']
Expand All @@ -2045,7 +2045,7 @@ def check_memory_setting(setting):

if settings.MODULARIZE and not (settings.EXPORT_ES6 and not settings.SINGLE_FILE) and \
settings.EXPORT_NAME == 'Module' and options.oformat == OFormat.HTML and \
(options.shell_path == shared.path_from_root('src', 'shell.html') or options.shell_path == shared.path_from_root('src', 'shell_minimal.html')):
(options.shell_path == utils.path_from_root('src/shell.html') or options.shell_path == utils.path_from_root('src/shell_minimal.html')):
exit_with_error(f'Due to collision in variable name "Module", the shell file "{options.shell_path}" is not compatible with build options "-s MODULARIZE=1 -s EXPORT_NAME=Module". Either provide your own shell file, change the name of the export to something else to avoid the name collision. (see https://github.com/emscripten-core/emscripten/issues/7950 for details)')

if settings.STANDALONE_WASM:
Expand Down Expand Up @@ -2638,7 +2638,7 @@ def phase_final_emitting(options, state, target, wasm_target, memfile):
target_dir = os.path.dirname(os.path.abspath(target))
worker_output = os.path.join(target_dir, settings.PTHREAD_WORKER_FILE)
with open(worker_output, 'w') as f:
f.write(shared.read_and_preprocess(shared.path_from_root('src', 'worker.js'), expand_macros=True))
f.write(shared.read_and_preprocess(utils.path_from_root('src/worker.js'), expand_macros=True))

# Minify the worker.js file in optimized builds
if (settings.OPT_LEVEL >= 1 or settings.SHRINK_LEVEL >= 1) and not settings.DEBUG_LEVEL:
Expand All @@ -2659,7 +2659,7 @@ def phase_final_emitting(options, state, target, wasm_target, memfile):
# Process .js runtime file. Note that we need to handle the license text
# here, so that it will not confuse the hacky script.
shared.JS.handle_license(final_js)
shared.run_process([shared.PYTHON, shared.path_from_root('tools', 'hacky_postprocess_around_closure_limitations.py'), final_js])
shared.run_process([shared.PYTHON, utils.path_from_root('tools/hacky_postprocess_around_closure_limitations.py'), final_js])

# Unmangle previously mangled `import.meta` references in both main code and libraries.
# See also: `preprocess` in parseTools.js.
Expand Down Expand Up @@ -2717,13 +2717,13 @@ def version_string():
# if the emscripten folder is not a git repo, don't run git show - that can
# look up and find the revision in a parent directory that is a git repo
revision_suffix = ''
if os.path.exists(shared.path_from_root('.git')):
if os.path.exists(utils.path_from_root('.git')):
git_rev = run_process(
['git', 'rev-parse', 'HEAD'],
stdout=PIPE, stderr=PIPE, cwd=shared.path_from_root()).stdout.strip()
stdout=PIPE, stderr=PIPE, cwd=utils.path_from_root()).stdout.strip()
revision_suffix = '-git (%s)' % git_rev
elif os.path.exists(shared.path_from_root('emscripten-revision.txt')):
with open(shared.path_from_root('emscripten-revision.txt')) as f:
elif os.path.exists(utils.path_from_root('emscripten-revision.txt')):
with open(utils.path_from_root('emscripten-revision.txt')) as f:
git_rev = f.read().strip()
revision_suffix = ' (%s)' % git_rev
return f'emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) {shared.EMSCRIPTEN_VERSION}{revision_suffix}'
Expand Down Expand Up @@ -3163,7 +3163,7 @@ def phase_binaryen(target, options, wasm_target):
building.asyncify_lazy_load_code(wasm_target, debug=intermediate_debug_info)

def preprocess_wasm2js_script():
return read_and_preprocess(shared.path_from_root('src', 'wasm2js.js'), expand_macros=True)
return read_and_preprocess(utils.path_from_root('src/wasm2js.js'), expand_macros=True)

def run_closure_compiler():
global final_js
Expand Down Expand Up @@ -3441,7 +3441,7 @@ def generate_traditional_runtime_html(target, options, js_target, target_basenam
# when script.inline isn't empty, add required helper functions such as tryParseAsDataURI
if script.inline:
for filename in ('arrayUtils.js', 'base64Utils.js', 'URIUtils.js'):
content = read_and_preprocess(shared.path_from_root('src', filename))
content = read_and_preprocess(utils.path_from_root('src', filename))
script.inline = content + script.inline

script.inline = 'var ASSERTIONS = %s;\n%s' % (settings.ASSERTIONS, script.inline)
Expand Down Expand Up @@ -3516,7 +3516,7 @@ def generate_html(target, options, js_target, target_basename,

if settings.EXPORT_NAME != 'Module' and \
not settings.MINIMAL_RUNTIME and \
options.shell_path == shared.path_from_root('src', 'shell.html'):
options.shell_path == utils.path_from_root('src/shell.html'):
# the minimal runtime shell HTML is designed to support changing the export
# name, but the normal one does not support that currently
exit_with_error('Customizing EXPORT_NAME requires that the HTML be customized to use that name (see https://github.com/emscripten-core/emscripten/issues/10086)')
Expand Down Expand Up @@ -3547,9 +3547,9 @@ def generate_worker_js(target, js_target, target_basename):


def worker_js_script(proxy_worker_filename):
web_gl_client_src = read_file(shared.path_from_root('src', 'webGLClient.js'))
idb_store_src = read_file(shared.path_from_root('src', 'IDBStore.js'))
proxy_client_src = read_file(shared.path_from_root('src', 'proxyClient.js'))
web_gl_client_src = read_file(utils.path_from_root('src/webGLClient.js'))
idb_store_src = read_file(utils.path_from_root('src/IDBStore.js'))
proxy_client_src = read_file(utils.path_from_root('src/proxyClient.js'))
proxy_client_src = do_replace(proxy_client_src, '{{{ filename }}}', proxy_worker_filename)
proxy_client_src = do_replace(proxy_client_src, '{{{ IDBStore.js }}}', idb_store_src)
return web_gl_client_src + '\n' + proxy_client_src
Expand Down Expand Up @@ -3662,7 +3662,7 @@ def replacement(self):

def is_valid_abspath(options, path_name):
# Any path that is underneath the emscripten repository root must be ok.
if shared.path_from_root().replace('\\', '/') in path_name.replace('\\', '/'):
if utils.path_from_root().replace('\\', '/') in path_name.replace('\\', '/'):
return True

def in_directory(root, child):
Expand Down
2 changes: 1 addition & 1 deletion emcmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def has_substr(args, substr):

# Append the Emscripten toolchain file if the user didn't specify one.
if not has_substr(args, '-DCMAKE_TOOLCHAIN_FILE'):
args.append('-DCMAKE_TOOLCHAIN_FILE=' + utils.path_from_root('cmake', 'Modules', 'Platform', 'Emscripten.cmake'))
args.append('-DCMAKE_TOOLCHAIN_FILE=' + utils.path_from_root('cmake/Modules/Platform/Emscripten.cmake'))

if not has_substr(args, '-DCMAKE_CROSSCOMPILING_EMULATOR'):
node_js = config.NODE_JS[0]
Expand Down
4 changes: 2 additions & 2 deletions emscons.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
import os
import subprocess
import sys
from tools import shared
from tools import utils

tool_path = os.path.join(shared.path_from_root('tools'), 'scons', 'site_scons', 'site_tools', 'emscripten')
tool_path = utils.path_from_root('tools/scons/site_scons/site_tools/emscripten')

env = os.environ.copy()
env['EMSCRIPTEN_TOOL_PATH'] = tool_path
Expand Down
2 changes: 1 addition & 1 deletion emscripten.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def compile_settings():
# Call js compiler
env = os.environ.copy()
env['EMCC_BUILD_DIR'] = os.getcwd()
out = shared.run_js_tool(path_from_root('src', 'compiler.js'),
out = shared.run_js_tool(path_from_root('src/compiler.js'),
[settings_file], stdout=subprocess.PIPE, stderr=stderr_file,
cwd=path_from_root('src'), env=env)
assert '//FORWARDED_DATA:' in out, 'Did not receive forwarded data in pre output - process failed?'
Expand Down
2 changes: 1 addition & 1 deletion tests/clang_native.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# These extra args need to be passed to Clang when targeting a native host system executable
def get_clang_native_args():
if MACOS:
return ['-isystem', path_from_root('system', 'include', 'libcxx')]
return ['-isystem', path_from_root('system/include/libcxx')]
elif os.name == 'nt':
# TODO: If Windows.h et al. are needed, will need to add something like '-isystemC:/Program
# Files (x86)/Microsoft SDKs/Windows/v7.1A/Include'.
Expand Down
3 changes: 2 additions & 1 deletion tests/fuzz/csmith_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from tools import shared
from tools import config
from tools import utils

# can add flags like --no-threads --ion-offthread-compile=off
engine = eval('config.' + sys.argv[1]) if len(sys.argv) > 1 else config.JS_ENGINES[0]
Expand Down Expand Up @@ -84,7 +85,7 @@
continue

shared.run_process([COMP, '-m32', opts, '-emit-llvm', '-c', fullname, '-o', filename + '.bc'] + CSMITH_CFLAGS + shared.get_cflags() + ['-w'])
shared.run_process([shared.path_from_root('tools', 'nativize_llvm.py'), filename + '.bc'], stderr=PIPE)
shared.run_process([utils.path_from_root('tools/nativize_llvm.py'), filename + '.bc'], stderr=PIPE)
shutil.move(filename + '.bc.run', filename + '2')
shared.run_process([COMP, fullname, '-o', filename + '3'] + CSMITH_CFLAGS + ['-w'])
print('3) Run natively')
Expand Down
4 changes: 2 additions & 2 deletions tests/jsrun.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import common
from subprocess import Popen, PIPE, CalledProcessError

from tools import shared
from tools import shared, utils

WORKING_ENGINES = {} # Holds all configured engines and whether they work: maps path -> True/False

Expand Down Expand Up @@ -59,7 +59,7 @@ def check_engine(engine):
if engine_path not in WORKING_ENGINES:
logging.debug('Checking JS engine %s' % engine)
try:
output = run_js(shared.path_from_root('tests/hello_world.js'), engine, skip_check=True)
output = run_js(utils.path_from_root('tests/hello_world.js'), engine, skip_check=True)
if 'hello, world!' in output:
WORKING_ENGINES[engine_path] = True
else:
Expand Down
4 changes: 2 additions & 2 deletions tests/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
import jsrun
import parallel_testsuite
import common
from tools import shared, config
from tools import shared, config, utils


sys.path.append(shared.path_from_root('third_party/websockify'))
sys.path.append(utils.path_from_root('third_party/websockify'))

logger = logging.getLogger("runner")

Expand Down
16 changes: 8 additions & 8 deletions tests/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def test_zzz_html_source_map(self):

def test_emscripten_log(self):
self.btest_exit(test_file('emscripten_log/emscripten_log.cpp'),
args=['--pre-js', path_from_root('src', 'emscripten-source-map.min.js'), '-gsource-map'])
args=['--pre-js', path_from_root('src/emscripten-source-map.min.js'), '-gsource-map'])

def test_preload_file(self):
create_file('somefile.txt', 'load me right before running the code please')
Expand Down Expand Up @@ -574,7 +574,7 @@ def test_custom_file_package_url(self):
# change the file package base dir to look in a "cdn". note that normally
# you would add this in your own custom html file etc., and not by
# modifying the existing shell in this manner
default_shell = read_file(path_from_root('src', 'shell.html'))
default_shell = read_file(path_from_root('src/shell.html'))
create_file('shell.html', default_shell.replace('var Module = {', '''
var Module = {
locateFile: function(path, prefix) {
Expand Down Expand Up @@ -668,7 +668,7 @@ def test():
test()

# TODO: CORS, test using a full url for locateFile
# create_file('shell.html', read_file(path_from_root('src', 'shell.html')).replace('var Module = {', 'var Module = { locateFile: function (path) {return "http:/localhost:8888/cdn/" + path;}, '))
# create_file('shell.html', read_file(path_from_root('src/shell.html')).replace('var Module = {', 'var Module = { locateFile: function (path) {return "http:/localhost:8888/cdn/" + path;}, '))
# test()

def test_dev_random(self):
Expand Down Expand Up @@ -3683,7 +3683,7 @@ def test_memory_growth_during_startup(self):
# pthreads tests

def prep_no_SAB(self):
create_file('html.html', read_file(path_from_root('src', 'shell_minimal.html')).replace('''<body>''', '''<body>
create_file('html.html', read_file(path_from_root('src/shell_minimal.html')).replace('''<body>''', '''<body>
<script>
SharedArrayBuffer = undefined;
Atomics = undefined;
Expand Down Expand Up @@ -3992,15 +3992,15 @@ def test_pthread_custom_pthread_main_url(self):
''')

# Test that it is possible to define "Module.locateFile" string to locate where worker.js will be loaded from.
create_file('shell.html', read_file(path_from_root('src', 'shell.html')).replace('var Module = {', 'var Module = { locateFile: function (path, prefix) {if (path.endsWith(".wasm")) {return prefix + path;} else {return "cdn/" + path;}}, '))
create_file('shell.html', read_file(path_from_root('src/shell.html')).replace('var Module = {', 'var Module = { locateFile: function (path, prefix) {if (path.endsWith(".wasm")) {return prefix + path;} else {return "cdn/" + path;}}, '))
self.compile_btest(['main.cpp', '--shell-file', 'shell.html', '-s', 'WASM=0', '-s', 'IN_TEST_HARNESS', '-s', 'USE_PTHREADS', '-s', 'PTHREAD_POOL_SIZE', '-o', 'test.html'], reporting=Reporting.JS_ONLY)
shutil.move('test.worker.js', Path('cdn/test.worker.js'))
if os.path.exists('test.html.mem'):
shutil.copyfile('test.html.mem', Path('cdn/test.html.mem'))
self.run_browser('test.html', '', '/report_result?exit:0')

# Test that it is possible to define "Module.locateFile(foo)" function to locate where worker.js will be loaded from.
create_file('shell2.html', read_file(path_from_root('src', 'shell.html')).replace('var Module = {', 'var Module = { locateFile: function(filename) { if (filename == "test.worker.js") return "cdn/test.worker.js"; else return filename; }, '))
create_file('shell2.html', read_file(path_from_root('src/shell.html')).replace('var Module = {', 'var Module = { locateFile: function(filename) { if (filename == "test.worker.js") return "cdn/test.worker.js"; else return filename; }, '))
self.compile_btest(['main.cpp', '--shell-file', 'shell2.html', '-s', 'WASM=0', '-s', 'IN_TEST_HARNESS', '-s', 'USE_PTHREADS', '-s', 'PTHREAD_POOL_SIZE', '-o', 'test2.html'], reporting=Reporting.JS_ONLY)
try_delete('test.worker.js')
self.run_browser('test2.html', '', '/report_result?exit:0')
Expand Down Expand Up @@ -4268,7 +4268,7 @@ def test_manual_wasm_instantiate(self, args=[]):
def test_wasm_locate_file(self):
# Test that it is possible to define "Module.locateFile(foo)" function to locate where worker.js will be loaded from.
ensure_dir('cdn')
create_file('shell2.html', read_file(path_from_root('src', 'shell.html')).replace('var Module = {', 'var Module = { locateFile: function(filename) { if (filename == "test.wasm") return "cdn/test.wasm"; else return filename; }, '))
create_file('shell2.html', read_file(path_from_root('src/shell.html')).replace('var Module = {', 'var Module = { locateFile: function(filename) { if (filename == "test.wasm") return "cdn/test.wasm"; else return filename; }, '))
self.compile_btest([test_file('browser_test_hello_world.c'), '--shell-file', 'shell2.html', '-o', 'test.html'])
shutil.move('test.wasm', Path('cdn/test.wasm'))
self.run_browser('test.html', '', '/report_result?0')
Expand Down Expand Up @@ -4863,7 +4863,7 @@ def test_unicode_html_shell(self):
return 0;
}
''')
create_file('shell.html', read_file(path_from_root('src', 'shell.html')).replace('Emscripten-Generated Code', 'Emscripten-Generated Emoji 😅'))
create_file('shell.html', read_file(path_from_root('src/shell.html')).replace('Emscripten-Generated Code', 'Emscripten-Generated Emoji 😅'))
self.btest_exit('main.cpp', args=['--shell-file', 'shell.html'])

# Tests the functionality of the emscripten_thread_sleep() function.
Expand Down
2 changes: 1 addition & 1 deletion tests/test_sockets.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ def test_websocket_send(self):
# Test that native POSIX sockets API can be used by proxying calls to an intermediate WebSockets -> POSIX sockets bridge server
def test_posix_proxy_sockets(self):
# Build the websocket bridge server
self.run_process(['cmake', path_from_root('tools', 'websocket_to_posix_proxy')])
self.run_process(['cmake', path_from_root('tools/websocket_to_posix_proxy')])
self.run_process(['cmake', '--build', '.'])
if os.name == 'nt': # This is not quite exact, instead of "isWindows()" this should be "If CMake defaults to building with Visual Studio", but there is no good check for that, so assume Windows==VS.
proxy_server = os.path.join(self.get_dir(), 'Debug', 'websocket_to_posix_proxy.exe')
Expand Down
1 change: 0 additions & 1 deletion third_party/websockify/websockify.py

This file was deleted.

5 changes: 5 additions & 0 deletions third_party/websockify/websockify.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env python

import websockify

websockify.websocketproxy.websockify_init()
Loading

0 comments on commit d8ec08a

Please sign in to comment.