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

Fix onnxruntime static library link when linking onto C/C++ #215

Merged
merged 2 commits into from
Jun 21, 2024

Conversation

cagnolone
Copy link
Contributor

Issue

When building into a static library which then is linked to a C/C++ project, if you build by using
cargo build --release
the onnx runtime lib is not merged with the newly created library, and link errors arise, forcing you to link again the onnx runtime.

However, if you build by specifying the path to the libraries, such as
ORT_LIB_LOCATION=/path/to/lib cargo build --release
no issues are present.

Solution

Change the linking instructions for the default case where the downloaded onnx libraries are used, adding static instruction.

Test

Tested on Linux x86_64, Ubuntu 22.04.4 LTS on WSL2

@decahedron1
Copy link
Member

This fixes #214, right? Does this affect dynamic linking at all? e.g. if there was only libonnxruntime.so in the ORT_LIB_LOCATION, it shouldn't fail to link.

@cagnolone
Copy link
Contributor Author

How can I force the dynamic linking? I have tried by using the load-dynamic feature, and if I build with verbose on I don't see the the static linking within the build script anymore.

In my .cache/ort, I only find the libonnxruntime.a file, and the project is only pre-linking everything without a problem.
When I launch the C executable, you must additionally pass the libonnxruntime.so file, otherwise it would crash on start.

I think that this is the standard behaviour, same as before. If you follow the logic path to get to the changed line, you would only get to it if the load-dynamic feature is OFF. I struggle to understand without more information if this change might introduce problems in other configurations.

@decahedron1
Copy link
Member

This change does break compile-time dynamic library linking.

error: could not find native static library `onnxruntime`, perhaps an -L flag is missing?

How can I force the dynamic linking? I have tried by using the load-dynamic feature, and if I build with verbose on I don't see the the static linking within the build script anymore.

load-dynamic is runtime dynamic linking, I'm talking about compile-time dynamic linking (i.e. load-dynamic disabled and ORT_LIB_LOCATION containing libonnxruntime.so); sorry, I should have been more clear about that.

I think the best solution here would be to check for the presence of libonnxruntime.a or onnxruntime.lib in lib_dir and add the static kind in that case, otherwise emitting the old unspecified line.

@cagnolone
Copy link
Contributor Author

Ok, I have managed to reproduce the linking error you specified in the case of the compile-time dynamic link.
I have updated the build file and now it works in both cases.

Thanks for the help!

@decahedron1 decahedron1 merged commit 882f657 into pykeio:main Jun 21, 2024
7 checks passed
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.

2 participants