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

gh-127111: Emscripten Move link flags from LDFLAGS_NODIST to LINKFORSHARED #127666

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Dec 6, 2024

The -sEXPORTED_FUNCTIONS setting will break when linking a shared library because the library doesn't main or Py_Version symbols. Only that one needs to be moved, but most of these other flags are ignored when linking a shared library so it's better not to pass them. The only one that needs to be passed when linking shared libraries is -sWASM_BIGINT which changes the ABI.

…RED`

The `-sEXPORTED_FUNCTIONS` setting will break when linking a shared library
because the library doesn't `main` or `Py_Version` symbols. Only that one
_needs_ to be moved, but most of these other flags are ignored when
linking a shared library so it's better not to pass them. The only one that
needs to be passed when linking shared libraries is `-sWASM_BIGINT` which
changes the ABI.
@hoodmane hoodmane changed the title gh-127111: Move link flags from LDFLAGS_NODIST to LINKFORSHARED gh-127111: Emscripten Move link flags from LDFLAGS_NODIST to LINKFORSHARED Dec 6, 2024
Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

👍 Seems to address the problem I was having - thanks!

@freakboy3742 freakboy3742 merged commit be07edf into python:main Dec 9, 2024
45 checks passed
@hoodmane hoodmane deleted the emscripten-link-for-shared branch December 9, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants