Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Apr 25, 2021

Its better to simply list dynamic library dependencies directly on the
command line.

Handle the deprecated option by simply added the list of libraries to
the command line. This means we no longer need to modify the dylink
section after linking since the linker will inject all the needed
dynamic libraries that it sees on the command line.

The emcc.py change here involves moving the settings parsing code
to before Find input files so that we can handle the legacy use of
RUNTIME_LINKED_LIBS by simply adding to newargs before input
files are searched for.

@sbc100 sbc100 requested a review from dschuff April 25, 2021 19:05
Its better to simply list dynamic library dependencies directly on the
command line.

Handle the deprecated option by simply added the list of libraries to
the command line.  This means we no longer need to modify the dylink
section after linking since the linker will inject all the needed
dynamic libraries that it sees on the command line.
@sbc100 sbc100 force-pushed the deprecate_runtime_linked_libs branch from 61c194c to c25daa4 Compare April 25, 2021 19:05

Current Trunk
-------------
- The `RUNTIME_LINKED_LIBS` setting is now deprecated. It's better to simply
Copy link
Member

Choose a reason for hiding this comment

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

Do we have any examples or documentation of the new CLI UI?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have the same amount of documentation for the new method as the old method AFAICT :) .. which is basically none.

Joking aside we should add some yes.

key, value = s.split('=', 1)
settings_map[key] = value

# Libraries are searched before settings_changes are applied, so apply the
Copy link
Member

Choose a reason for hiding this comment

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

Is this still true? it looks like there's some library-related code in between the old and new location of this code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm .. this comment seem very old yes. Goes back to 9b727e8. I will investigate, but land this as-is if thats ok?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the code doesn't look to me like it should actually depend on what's in between.

@sbc100 sbc100 merged commit 9197f4b into main Apr 26, 2021
@sbc100 sbc100 deleted the deprecate_runtime_linked_libs branch April 26, 2021 16:20
sbc100 added a commit to sbc100/emscripten that referenced this pull request Oct 28, 2025
We have had this marked as deprecated since emscripten-core#14004 (more than 4 years).
sbc100 added a commit to sbc100/emscripten that referenced this pull request Oct 28, 2025
We have had this marked as deprecated since emscripten-core#14004 (more than 4 years).
sbc100 added a commit to sbc100/emscripten that referenced this pull request Oct 28, 2025
We have had this marked as deprecated since emscripten-core#14004 (more than 4 years).
sbc100 added a commit to sbc100/emscripten that referenced this pull request Oct 28, 2025
We have had this marked as deprecated since emscripten-core#14004 (more than 4 years).
sbc100 added a commit to sbc100/emscripten that referenced this pull request Oct 28, 2025
We have had this marked as deprecated since emscripten-core#14004 (more than 4 years).
sbc100 added a commit to sbc100/emscripten that referenced this pull request Oct 28, 2025
We have had this marked as deprecated since emscripten-core#14004 (more than 4 years).
sbc100 added a commit to sbc100/emscripten that referenced this pull request Oct 29, 2025
We have had this marked as deprecated since emscripten-core#14004 (more than 4 years).
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.

3 participants