Skip to content

Remove some magic handling of native shared libraries#20886

Merged
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:remove_library_rerouteing
Dec 12, 2023
Merged

Remove some magic handling of native shared libraries#20886
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:remove_library_rerouteing

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented Dec 8, 2023

We had this rather strange (I imagine completely unused) feature where is emscripten is passed a concrete path to a shared library file (e.g. /path/to/libfoo.so) and it could determine that the shared library was not indented for emscripten then it would instead search the library for other things called libfoo that might work.

This seems too magical at best and harmful/confusing a worst.

The presence of this feature is blocking the way for some internal optimizations and refactorings (e.g. #20884)

I tracked the addition of this feature back to 03033b2 in 2012.

@sbc100 sbc100 requested a review from kripken December 8, 2023 23:11
@sbc100 sbc100 force-pushed the remove_library_rerouteing branch 2 times, most recently from fd51a94 to f8e1771 Compare December 8, 2023 23:12
@sbc100 sbc100 requested a review from dschuff December 9, 2023 00:11
Comment thread ChangeLog.md Outdated
- Emscripten is now more strict about handling unsupported shared library
inputs. For example if you passed `/usr/lib/libz.so` emscripten used to
magically redirect that to its own internal libz if it could find one.
(#20886)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe this could say what a user should do if they run into this? I guess they should provide a absolute path to an emscripten dylib, or emscripten will search its own library paths if the path isn't absolute?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You shouldn't be passing system libraries to emscripten at all. So I guess the advice would "don't use host libraries with emscripten, you need to use emscripten-specific libraries".

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Dec 11, 2023

Updated ChangeLog entry to be even more explicit about that changed here. PTAL

We had this rather strange (I imagine completely unused) feature where
is emscripten is passed a concrete path to a shared library file (e.g.
/path/to/libfoo.so) and it could determine that the shared library was
not indented for emscripten then it would instead search the library
for other things called `libfoo` that might work.

This seems too magical at best and harmful/confusing a worst.

The presence of this feature is blocking the way for some internal
optimizations and refactorings (e.g. emscripten-core#20884)
@sbc100 sbc100 force-pushed the remove_library_rerouteing branch 2 times, most recently from 001af32 to c8a43e7 Compare December 11, 2023 21:53
Comment thread ChangeLog.md
Comment thread ChangeLog.md
Comment thread emcc.py
diagnostics.warning('map-unrecognized-libraries', f'unrecognized file type: `{arg}`. Mapping to `{flag}` and hoping for the best')
state.add_link_flag(i, flag)
else:
input_files.append((i, arg))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps we could add an error here that suggests the proper form, to help people update? That is, if /abs/path/libfoo.so does not exist, but libfoo.a exists in the sysroot, we could say Absolute path .. provided, but the contents there are not valid for wasm. Did you mean -lfoo ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'd rather not for the following reasons:

  1. I think its unlikely anyone is actually relying on this/
  2. I think the unknown file format is pretty reasonable and actionable
  3. Do a search of all the library paths at this point is tricky.. especially given the we sometime build libraries on demand and they might not exist yet.
  4. Its hard to recommend -lfoo just because somebody passed somepath/libfoo.so. In most cases the solution to "unknown file format somepath/libfoo.so" is to rebuild somepath/libfoo.so with emcc not to replace that with -lfoo

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fair enough, lgtm.

@sbc100 sbc100 enabled auto-merge (squash) December 11, 2023 22:45
@sbc100 sbc100 disabled auto-merge December 12, 2023 02:09
@sbc100 sbc100 merged commit 1850b75 into emscripten-core:main Dec 12, 2023
@sbc100 sbc100 deleted the remove_library_rerouteing branch December 12, 2023 02:09
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