link-search-path doesn't have a way to specify priority / doesn't auto deprioritize OS link paths

[vlovich]

Problem

I modified the llama.cpp repo that llama-cpp-2 has as a submodule to expose a new symbol & then added a method to call that new symbol in llama-cpp-2 via a Rust pub API. I then called the new Rust pub API in my code and added cpal as a dependency and the path to my local llama-cpp-2 as a dependency with the cuda feature on (cuda doesn't matter but right now that forces llama.cpp to be built as a shared library). I then tried to build my crate. I also installed libllama.so in my OS.

I expected to see this happen: Crate builds

Instead, this happened: Crate fails to link saying the new symbol I added to llama.cpp is missing.

The root cause is that cpal depends indirectly on alsa-sys which uses pkg-config which adds the location of the found library in /usr/lib to the search path. This happens before llama-cpp-2 runs and thus /usr/lib ends up ahead of the path that llama-cpp-2 prints causing it to try to link against the OS library that doesn't have the symbol.

While there's no way to specify priority and while valuable, that feels like it would be a much larger discussion. I was wondering if a more targeted fix would be to ensure that all paths under OUT_DIR are moved to the front (in an order-preserving way) while the remaining directories are moved later (in an order-preserving way). That way the linker ALWAYS looks for paths within OUT_DIR before looking in other locations.

I'm not sure if this bug is in rust itself or in Cargo, so I also filed to rust: rust-lang/rust#137404

Steps

1. Install alsa and another dynamic library that conflicts with a crate that tries to build the same dynamic library (tested on Linux with libllama.so and llama-cpp-sys-2)
2. Add alsa-sys as a dependency & use some symbols from it so that /usr/lib ends up getting added to the search path by pkg-config
3. Modify the crate to build a new version of the conflicting dynamic library with new symbols & try to use those symbols

Expected: build should still work because the dynamic library should be used out of OUT_DIR not /usr/lib.

Possible Solution(s)

Bubble up link search paths under OUT_DIR to the front and bubble down search paths outside of it in a relative order preserving way.

Notes

No response

Version

cargo 1.84.0 (66221abde 2024-11-19)
release: 1.84.0
commit-hash: 66221abdeca2002d318fde6efff516aab091df0e
commit-date: 2024-11-19
host: x86_64-unknown-linux-gnu
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Arch Linux Rolling Release [64-bit]

[weihanglo]

Thanks for the issue report!

Note that the documentation says:

These paths are also added to the dynamic library search path environment variable if they are within the OUT_DIR. Depending on this behavior is discouraged since this makes it difficult to use the resulting binary. In general, it is best to avoid creating dynamic libraries in a build script (using existing system libraries is fine).

The order of link search paths in Cargo is fair stable for a long while IIRC. Your proposal, prioritizing OUT_DIR in lib search path, might have unexpected consequences. For example a dependency might start picking up a wrong native lib from other transitive dependencies' OUT_DIR, while it is intended to use it from the system. However, it is less likely to happen since one OUT_DIR usually has only the native libs the crate is interested in.

The short term solution for you is either removing llama from your system library, or using something like nix to have a completely sandboxed build environment.

[vlovich]

While I appreciate that this behavior has been warned about and has been unchanged, I don't think the reasoning holds up to scrutiny. If I'm building libfoo.so I'm likely trying to build a -sys crate where I wrap a native library with Rust bindings. It's not quite as common to build a dynamic library and the consume it whole from the crate which is the only scenario where the current behavior is safe.

For example, llama-cpp-2 is a wrapper around the bindgen that llama-cpp-sys-2 utilizes to link against the built library. This sys crate pattern is quite ubiquitous which means that the library that gets picked up is inconsistent and dependent on the host configuration, not the build as expressed and expected via Cargo.toml.

While I respect the position of "uninstall the conflicting host library or use nix" it feels like a user hostile position as it puts the burden of figuring surprising behavior on developers of varying skill levels.

I agree that this has the potential to change which dylib you link against today, I would present that this actually is more likely to match user expectations of linking against the dylib within your target folder consistently across any machine that clones the repo (reduces cognitive load and collaboration issues). Would you be more OK with this approach if this required a command line flag to opt in and then it could becomes the default in a future version / edition?

[weihanglo]

Just to clarify, I don't object to the proposal, as opposite it looks reasonable to me. However scrutiny is always needed since the behavior is stable and there are so many hidden workflows I might not be aware of. Already put this in the agenda of Cargo team meeting next week.

[vlovich]

Sounds good. Thanks for driving the discussion!

[weihanglo]

Hi, @vlovich.
We've discussed this during the meeting today. We think the risk is relatively low, and the change is quite reasonable. It is unlikely that your package depends on the same native dependency twice, one from system and the other from vendored source. If it were really that case, linker should have got confused, or symbol conflicts should have happened already.

Also, there might be an overlapped interest in artifact dependencies for building dylibs, making this issue more interesting. For example, you might want to emit a link search path to the cdylib artifact dependencies.

Anyway, thanks for the proposal! I'll review the PR later in this week.