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.

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

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jan 8, 2025

By delaying the processing of --js-library arguments we avoid having to jump through hoops and store them alongside their ordinal number for half the time.

By delaying the processing of `--js-library` arguments we avoid having
jump through hoops and store them alongside their ordinal number for
half the time.
@sbc100 sbc100 requested review from dschuff and kripken January 8, 2025 01:13
@sbc100 sbc100 enabled auto-merge (squash) January 8, 2025 02:13
@@ -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

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

@sbc100 sbc100 merged commit 9f83cfa into emscripten-core:main Jan 8, 2025
29 checks passed
@sbc100 sbc100 deleted the js_library branch January 8, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants