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
[draft] emcc forgetting library #21605
base: main
Are you sure you want to change the base?
Conversation
@@ -2764,6 +2764,8 @@ def process_dynamic_libs(dylibs, lib_dirs): | |||
exit_with_error(f'{os.path.normpath(dylib)}: shared library dependency not found: `{needed}`') | |||
to_process.append(path) | |||
|
|||
seen.add(os.path.basename(dylib)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seen
set here is only used to prevent processing a library more than once. It not used for anything else below. So I don't see how this could mean that libraries were not being included.
Can you explain the bug you are trying to fix a litte more? Do you have an example that fails perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I opened an issue regarding this with the reproducer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My motivation of adding the dylib to seen is:
If the dylib does not have any needed libraries,
for needed in dylink.needed:
there is no way for it to be added to the list of "seen".
Hence emcc does not know if a library, (eg. in the issue #21667
libbar.so) was visited already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"seen" will be reused in the while loop again. And since we are checking
if needed in seen
, libbar will be discovered and the compilation will
continue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue is a little complicated since we don't have have a real concept of SO_NAME
(which is what goes in the needed section) vs filename which is what we currently track.
Just because a file called libfoo.so
was processed on the commant line doesn't necessarily mean the its "visible" in the library path when other shared libraries depend on it. I'd rather not apply this change now, and point users to using -L
or -Wl,-rpath
as the only way to satisfy the needed
libraries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(at least for now)
A library is added to seen only when it is in the needed list with
its path to be found. Therefore, a library may be left unseen even if it
is passed as a parameter.