Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay processing of --js-library flags. NFC #23332

Merged
merged 1 commit into from
Jan 8, 2025
Merged

Delay processing of --js-library flags. NFC #23332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions emcc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,9 +705,11 @@ def phase_parse_arguments(state):
settings.WARN_DEPRECATED = 0

for i in range(len(newargs)):
if newargs[i] in ('-l', '-L', '-I', '-z'):
if newargs[i] in ('-l', '-L', '-I', '-z', '--js-library'):
# Scan for flags that can be written as either one or two arguments
# and normalize them to the single argument form.
if newargs[i] == '--js-library':
newargs[i] += '='
newargs[i] += newargs[i + 1]
newargs[i + 1] = ''

Expand Down Expand Up @@ -792,6 +794,8 @@ def phase_setup(options, state, newargs):
state.add_link_flag(i + 1, newargs[i + 1])
elif arg.startswith('-z'):
state.add_link_flag(i, newargs[i])
elif arg.startswith('--js-library='):
state.add_link_flag(i, newargs[i])
elif arg.startswith('-Wl,'):
# Multiple comma separated link flags can be specified. Create fake
# fractional indices for these: -Wl,a,b,c,d at index 4 becomes:
Expand Down Expand Up @@ -939,7 +943,7 @@ def filter_out_link_flags(args):
def is_link_flag(flag):
if flag in ('-nostdlib', '-nostartfiles', '-nolibc', '-nodefaultlibs', '-s'):
return True
return flag.startswith(('-l', '-L', '-Wl,', '-z'))
return flag.startswith(('-l', '-L', '-Wl,', '-z', '--js-library'))

skip = False
for arg in args:
Expand Down Expand Up @@ -1311,7 +1315,7 @@ def consume_arg_file():
options.memory_profiler = True
newargs[i] = ''
settings_changes.append('EMSCRIPTEN_TRACING=1')
settings.JS_LIBRARIES.append((0, 'library_trace.js'))
settings.JS_LIBRARIES.append('library_trace.js')
elif check_flag('--emit-symbol-map'):
options.emit_symbol_map = True
settings.EMIT_SYMBOL_MAP = 1
Expand Down Expand Up @@ -1346,8 +1350,6 @@ def consume_arg_file():
options.emit_tsd = consume_arg()
elif check_flag('--no-entry'):
options.no_entry = True
elif check_arg('--js-library'):
settings.JS_LIBRARIES.append((i + 1, os.path.abspath(consume_arg_file())))
elif check_flag('--remove-duplicates'):
diagnostics.warning('legacy-settings', '--remove-duplicates is deprecated as it is no longer needed. If you cannot link without it, file a bug with a testcase')
elif check_flag('--jcache'):
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ lint.ignore = [
"PLW2901",
]
lint.per-file-ignores."emrun.py" = [ "PLE0704" ]
lint.mccabe.max-complexity = 48 # Recommended: 10
lint.mccabe.max-complexity = 49 # Recommended: 10
lint.pylint.allow-magic-value-types = [
"bytes",
"float",
"int",
"str",
]
lint.pylint.max-args = 15 # Recommended: 5
lint.pylint.max-branches = 49 # Recommended: 12
lint.pylint.max-branches = 50 # Recommended: 12
lint.pylint.max-returns = 16 # Recommended: 6
lint.pylint.max-statements = 142 # Recommended: 50

Expand Down
25 changes: 11 additions & 14 deletions tools/link.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1240,7 +1240,7 @@ def phase_linker_setup(options, state): # noqa: C901, PLR0912, PLR0915
state.append_link_flag('--no-whole-archive')
settings.FILESYSTEM = 1
settings.SYSCALLS_REQUIRE_FILESYSTEM = 0
settings.JS_LIBRARIES.append((0, 'library_wasmfs.js'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this 0 not ensure that system libs were before user ones?

Copy link
Collaborator Author

@sbc100 sbc100 Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, and that is still that case. All these JS_LIBRARIES.append happen before process_libraries which runs before process_libraries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First process libraries should be phase_linker_setup in the comment? If so then yes, that looks right, thanks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sorry, fixed commend

settings.JS_LIBRARIES.append('library_wasmfs.js')
if settings.ASSERTIONS:
# used in assertion checks for unflushed content
settings.REQUIRED_EXPORTS += ['wasmfs_flush']
Expand Down Expand Up @@ -1362,14 +1362,14 @@ def phase_linker_setup(options, state): # noqa: C901, PLR0912, PLR0915

if settings.PTHREADS:
setup_pthreads()
settings.JS_LIBRARIES.append((0, 'library_pthread.js'))
settings.JS_LIBRARIES.append('library_pthread.js')
if settings.PROXY_TO_PTHREAD:
settings.PTHREAD_POOL_SIZE_STRICT = 0
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$runtimeKeepalivePush']
else:
if settings.PROXY_TO_PTHREAD:
exit_with_error('-sPROXY_TO_PTHREAD requires -pthread to work!')
settings.JS_LIBRARIES.append((0, 'library_pthread_stub.js'))
settings.JS_LIBRARIES.append('library_pthread_stub.js')

if settings.MEMORY64:
# Any "pointers" passed to JS will now be i64's, in both modes.
Expand All @@ -1383,7 +1383,7 @@ def phase_linker_setup(options, state): # noqa: C901, PLR0912, PLR0915
# set location of Wasm Worker bootstrap JS file
if settings.WASM_WORKERS == 1:
settings.WASM_WORKER_FILE = unsuffixed(os.path.basename(target)) + '.ww.js'
settings.JS_LIBRARIES.append((0, 'library_wasm_worker.js'))
settings.JS_LIBRARIES.append('library_wasm_worker.js')

# Set min browser versions based on certain settings such as WASM_BIGINT,
# PTHREADS, AUDIO_WORKLET
Expand All @@ -1401,7 +1401,7 @@ def phase_linker_setup(options, state): # noqa: C901, PLR0912, PLR0915
if settings.AUDIO_WORKLET:
if settings.AUDIO_WORKLET == 1:
settings.AUDIO_WORKLET_FILE = unsuffixed(os.path.basename(target)) + '.aw.js'
settings.JS_LIBRARIES.append((0, shared.path_from_root('src', 'library_webaudio.js')))
settings.JS_LIBRARIES.append(shared.path_from_root('src', 'library_webaudio.js'))
if not settings.MINIMAL_RUNTIME:
# If we are in the audio worklet environment, we can only access the Module object
# and not the global scope of the main JS script. Therefore we need to export
Expand Down Expand Up @@ -2807,12 +2807,15 @@ def map_to_js_libs(library_name):

def process_libraries(state):
new_flags = []
libraries = []
suffixes = STATICLIB_ENDINGS + DYNAMICLIB_ENDINGS
system_libs_map = system_libs.Library.get_usable_variations()

# Find library files
# Process `-l` and `--js-library` flags
for i, flag in state.link_flags:
if flag.startswith('--js-library='):
js_lib = os.path.abspath(flag.split('=', 1)[1])
settings.JS_LIBRARIES.append(js_lib)
continue
if not flag.startswith('-l'):
new_flags.append((i, flag))
continue
Expand All @@ -2822,7 +2825,7 @@ def process_libraries(state):

js_libs = map_to_js_libs(lib)
if js_libs is not None:
libraries += [(i, js_lib) for js_lib in js_libs]
settings.JS_LIBRARIES += js_libs

# We don't need to resolve system libraries to absolute paths here, we can just
# let wasm-ld handle that. However, we do want to map to the correct variant.
Expand All @@ -2848,12 +2851,6 @@ def process_libraries(state):

new_flags.append((i, flag))

settings.JS_LIBRARIES += libraries

# At this point processing JS_LIBRARIES is finished, no more items will be added to it.
# Sort the input list from (order, lib_name) pairs to a flat array in the right order.
settings.JS_LIBRARIES.sort(key=lambda lib: lib[0])
settings.JS_LIBRARIES = [lib[1] for lib in settings.JS_LIBRARIES]
state.link_flags = new_flags


Expand Down
Loading