-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[dylink] Fix rpath calculation in nested dependencies #24234
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
742a3e6
f1f85ff
65f39d4
8bc8fff
3fb1ddd
62dc4a4
81b1dbd
f4e3b25
8d92565
d9289c2
3396b43
af36541
f4fecbb
b86e1f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -904,7 +904,9 @@ var LibraryDylink = { | |
// We need to set rpath in flags based on the current library's rpath. | ||
// We can't mutate flags or else if a depends on b and c and b depends on d, | ||
// then c will be loaded with b's rpath instead of a's. | ||
flags = {...flags, rpath: { parentLibPath: libName, paths: metadata.runtimePaths }} | ||
var dso = LDSO.loadedLibsByName[libName]; | ||
var libPath = dso?.path ?? libName; | ||
flags = {...flags, rpath: { parentLibPath: libPath, paths: metadata.runtimePaths }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is possible to calculate the dirname here. For now, the dirname is calculated inside replaceORIGIN function, and the reason I did that was to isolate the code that depends on If we calculate the dirname here, I think it can be something like:
|
||
// now load needed libraries and the module itself. | ||
if (flags.loadAsync) { | ||
return metadata.neededDynlibs | ||
|
@@ -937,6 +939,7 @@ var LibraryDylink = { | |
var dso = { | ||
refcount: Infinity, | ||
name, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So does that mean that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, only the dependencies of the libraries will have only the so_name in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
So, I think the alternative approach would be ensuring |
||
path: name, // full path to the library, updated when the library is resolved in the filesystem. | ||
exports: syms, | ||
global: true, | ||
}; | ||
|
@@ -1105,6 +1108,7 @@ var LibraryDylink = { | |
#endif | ||
if (f) { | ||
var libData = FS.readFile(f, {encoding: 'binary'}); | ||
dso.path = f; | ||
return flags.loadAsync ? Promise.resolve(libData) : libData; | ||
} | ||
#endif | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
11735 | ||
11758 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
27774 | ||
27799 |
Uh oh!
There was an error while loading. Please reload this page.