pykeio / ort

Fast ML inference & training for Rust with ONNX Runtime
https://ort.pyke.io/
Apache License 2.0
786 stars 91 forks source link

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

Closed cagnolone closed 2 months ago

cagnolone commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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!