-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Implement PosixEnv::GetRuntimePath to enable AutoEP tests on Linux/MacOS #26424
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
Implement PosixEnv::GetRuntimePath to enable AutoEP tests on Linux/MacOS #26424
Conversation
We try to follow the implementation of WindowsEnv which resolves the runtime directory as the directory of the current DLL (onnxruntime) or executable for static linkage.
The same symbols are already exposed for Windows. This enables the CUDA EP to be selected in AutoEP tests.
769dd8d to
30b8639
Compare
| if (dladdr(symbol_from_this_library, &dl_info) && dl_info.dli_fname) { | ||
| // Converting to absolute path since running a program with and without gdb attached can make a difference whether | ||
| // dli_fname will be absolute or relative. Converting to absolute for consistent result | ||
| runtime_path = PathString(std::filesystem::absolute(std::filesystem::path(dl_info.dli_fname).parent_path().string())) + "/"; |
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.
I saw that returning an absolute vs an relative path makes a difference on whether ORT will attempt to call CreateEp which CUDA EP does not implement.
| global: | ||
| GetProvider; | ||
| CreateEpFactories; | ||
| ReleaseEpFactory; |
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 enables the autoep test cases. The exports are already present on Windows.
| #else | ||
| // TODO: MacOS could use _NSGetExecutablePath, but this needs to be tested! | ||
| runtime_path = PathString(); | ||
| #endif |
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.
I don't have a Mac to test. Can this be deferred to another PR? It is not required for the tests as the tests use libonnxruntime.dynlib
|
|
||
| // Return the path of the executable/shared library for the current running code. This is to make it | ||
| // possible to load other shared libraries installed next to our core runtime code. | ||
| PathString GetRuntimePath() const override { |
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.
You may directly use std::filesystem::path. The class PathString was created before we migrated the code to C++17.
|
FYI, I have a PR to enable |
Thanks! Wasn't aware of this. Will close this one then! |
Description
During #26210 we discovered that implementing
PosixEnv::GetRuntimePathis a requirement to make autoep test work on Linux.This PR implements PosixEnv::GetRuntimePath in a way that is close to Windows on Linux and (partially) on MacOS. The newly added method is tested by enabling autoep tests on Linux and MacOS.
Motivation and Context
@edgchen1