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

target/debug/deps folder not propagated in LD_LIBRARY_PATH #1297

Closed
Larswad opened this issue May 31, 2023 · 15 comments · Fixed by #1652
Closed

target/debug/deps folder not propagated in LD_LIBRARY_PATH #1297

Larswad opened this issue May 31, 2023 · 15 comments · Fixed by #1652
Assignees

Comments

@Larswad
Copy link

Larswad commented May 31, 2023

Using tarpauling 0.25.1.
I have a crate building a shared library (.so) with c - code (build.rs).
I have another rust-only crate exposing FFI code to that library (calls made in both directions, rust calls into C code, rust expose C FFI functions to be called from the library).
The rust crate has in it's Cargo.toml set a dependency to the dynamic C library:

[dev-dependencies]
my-library.workspace = true

In the rust crate I have a test in the tests folder, loading the C library dynamically using libloading (e.g. dlopen).
Running this with cargo test works fine, with tarpaulin the library is not found.
I inspected the LD_LIBRARY_PATH in both cases, when running tarpaulin the:

/target/debug/deps

folder was missing (but not when running cargo test) and is the reason for the failure.
I am not sure if I have missed anything that I should have given to tarpaulin or if it's a problem not specific to tarpaulin but the inconsistency here bring me the think it's an issue.

To Reproduce
Build a c library using build.rs, it's enough with one C function in there.
Build a rust crate using the function in the C library.
Create a test using libloading that loads up the C library, calling get to resolve the C library's function.
Call it.

Expected behavior
The C library should have been found by the test code when running tarpaulin, just like when using cargo test.
The /target//deps folder should be included in the LD_LIBRARY_PATH during test execution.

@Larswad
Copy link
Author

Larswad commented May 31, 2023

If useful, the:
/target/debug
folder was also missing in the tarpaulin build in LD_LIBRARY_PATH.

@xd009642
Copy link
Owner

Out of curiosity if you do:

cargo test --no-run
./target/debug/$TEST

Where $TEST is the compiled test, so something like deps/$MODULE_$HASH does it also fail to run with a linker issue? If so that's not really something I can/will fix in tarpaulin - as I can't identify temporary changes to LD_LIBRARY_PATH when building tests that aren't available before running it or after running it.

A bunch of crates using native code will work if you do cargo test but then if you run the tests separately afterwards will require you to set the LD_LIBRARY_PATH yourself.

@Larswad
Copy link
Author

Larswad commented May 31, 2023

Thanks. I am not completely sure what you intend there to check. But I specified the path like:
cargo test --no-run file://target/debug/deps/-hash
(Had to specify as file:// apparantly).
That just doesn't seem to do any meaningful though although nothing fails, hard to describe what I mean, but it doesn't seem to care about the binary I give to it, regardless of --no-run or not.

Just to be clear, it is the environment's LD_LIBRARY_PATH that is not the same for running cargo test as for tarpaulin while running the test (not during building of the test). Since this library is loaded dynamically in runtime (not as a "statically bound" dynamic library if you know what I mean).
So regardless of reason i'm curious why the environment clearly is different, even though i'm not clear on how tarpauling works procedureally in the build/execution process. Does tarpaulin have access to the
target/<buildconfig>/
and
target/<buildconfig>/deps/
folders? If so, I mean, I guess tarpauling could add those path to LD_LIBRARY_PATH by itself, before executing the test.
The .so binary is both present in the target/debug and target/debug/deps folders.

@xd009642
Copy link
Owner

So you run cargo test --no-run just to compile the tests. And then run one of the compiled tests.

Some native library bindings will actually add to the linker path dynamically via the build script and download the shared objects to a folder in the target dir which may not be the same place. They may also choose not to add that folder to LD_LIBRARY_PATH if a user specifies they want to use a system installed one and not a downloaded one - but still have a downloaded one in their target dir from a previous run.

of course if cargo test always modifies the LD_LIBRARY_PATH in a predictable way I can emulate that - but I have a feeling it doesn't and it's partially on the crate authors doing things in their build.rs files to enable cargos run environment to discover them

@Larswad
Copy link
Author

Larswad commented May 31, 2023

So I misunderstood you, it's too separate things. Sorry.
I first cargo test --no-run
(needed to specify features in my case since we have some mutually exclusive custom features depending on architecture).
Then launch the test by simply executing it manually giving that path target/debug/deps/...testname. Yes, that fails like you suspected.
Only if I prefix with:
LD_LIBRARY_PATH=target/debug it is fine.
If you feel this is kind of stretches things a bit for tarpaulin in a wrong way I'm fine with closing it.
Just don't know how to get out of this situation now. We run tarpaulin in our CI.
We had excluded cfg directives using --avoid-cfg-tarpaulin (found out the reason later, it was because another crate, coap-lite uses an old tag, "skip", that we have no control over, the one that worked for me was #[cfg_attr(tarpaulin, ignore)] (because I'm even fine with ignoring this specific test for tarpaulin.
So, if I ignore it (and remove --avoid-cfg-tarpaulin) I get problems with coap-lite instead.
Bummer.

@Larswad
Copy link
Author

Larswad commented May 31, 2023

I found out that env!() macro in my test gives me the LD_LIBRARY_PATH as it were during build-time (in which the deps folder is actually there). I could in the test loop over the paths, concat with the library name and try loading until one of the paths worked. That solves my issue in the best way.
If you'd ever consider this as an issue, the env!() macro there could be useful to populate the runtime LD_LIBRARY_PATH for tarpaulin. Otherwise, thanks for responding so kindly!

@xd009642
Copy link
Owner

I don't think the env macro would work for tarpaulin in that case as you're not rebuilding tarpaulin every time you run it. But I will look into what cargo test is doing during run and see if there's something I can add to smooth the workflow.

Personally in $DAY_JOB I set the LD_LIBRARY_PATH for tensorflow and ffmpeg libraries which we install in CI and link to. I prefer that over relying on whatever crate I'm using for bindings downloading one as it has in the past broken things - or resulted in CI testing a different set of dependencies than we're using in prod

@Larswad
Copy link
Author

Larswad commented May 31, 2023

Thanks for the advice. Will discuss it in the team.

@Luthaf
Copy link
Contributor

Luthaf commented Nov 22, 2024

In general,cargo test will add such directories to the library loading path; and rustdoc recently started to do the same (rust-lang/cargo#8531, fixed in 1.78 by rust-lang/cargo#13490).

It would be nice if cargo-tarpaulin could do the same, so it does not require a separate setup from the other cargo tools!

EDIT: a bit more details: cargo seems to add folders like target/<debug>/build/<crate>-<hash>/out/ to the library search path, NOT target/debug/deps. The first one is where a build script would place it's output when compiling a shared library. Cargo might also just be taking the output of cargo:rustc-link-search=native=<path> from the build script outputs, and not treating target/<debug>/build/<crate>-<hash>/out/ specially. If the information is available, the latter seems like the best way to handle this for tarpaulin.

@xd009642
Copy link
Owner

xd009642 commented Nov 24, 2024

@Luthaf so with tarpaulin's torch test https://github.com/xd009642/tarpaulin/tree/develop/tests/data/torch_test torch is downloaded into a target/<debug>/build/torch-sys-<hash>/out and is picked up automatically by tarpaulin, probably because I do use cargo metadata to look for this info and add things to the linker path... I'll have a look to see if there's any linker path hints I'm missing - maybe something changed or I'm skipping build script output I could be using.

EDIT: Okay so right here I check the build script for linker paths and libraries so I can add things to the Linker path https://github.com/xd009642/tarpaulin/blob/develop/src/cargo.rs#L295-L313

@Luthaf
Copy link
Contributor

Luthaf commented Nov 25, 2024

Thanks for checking. I think it was failing for me, but I'll check again and try to reproduce the issue I was having in a small project.

@Luthaf
Copy link
Contributor

Luthaf commented Nov 25, 2024

Ok, so with this Cargo.toml:

[package]
name = "tarpaulin-metatensor"
version = "0.1.0"
edition = "2021"

[dependencies]
metatensor = "0.2"

And this lib.rs:

#[cfg(test)]
mod tests {

    #[test]
    fn test_labels() {
        let labels = metatensor::Labels::new(["a"], &[[0]]);
        assert_eq!(labels.names(), ["a"]);
    }
}

I get the following output for cargo test:

   Compiling rawpointer v0.2.1
   Compiling smallvec v1.13.2
   Compiling once_cell v1.20.2
   Compiling num-traits v0.2.19
   Compiling metatensor-sys v0.1.10
   Compiling matrixmultiply v0.3.9
   Compiling num-complex v0.4.6
   Compiling num-integer v0.1.46
   Compiling ndarray v0.16.1
   Compiling metatensor v0.2.0
   Compiling tarpaulin-metatensor v0.1.0 (...)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 2.83s
     Running unittests src/lib.rs (target/debug/deps/tarpaulin_metatensor-d69f48c88f0a5a85)

running 1 test
test tests::test_labels ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests tarpaulin_metatensor

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

But this with cargo tarpaulin (version 0.31.2):

2024-11-25T11:39:51.293396Z  INFO cargo_tarpaulin::config: Creating config
2024-11-25T11:39:51.348345Z  INFO cargo_tarpaulin: Running Tarpaulin
2024-11-25T11:39:51.348357Z  INFO cargo_tarpaulin: Building project
2024-11-25T11:39:51.348361Z  INFO cargo_tarpaulin::cargo: Cleaning project
   Compiling autocfg v1.4.0
   Compiling libc v0.2.164
   Compiling rustix v0.38.41
   Compiling bitflags v2.6.0
   Compiling cc v1.0.105
   Compiling home v0.5.5
   Compiling either v1.13.0
   Compiling rawpointer v0.2.1
   Compiling smallvec v1.13.2
   Compiling once_cell v1.20.2
   Compiling num-traits v0.2.19
   Compiling matrixmultiply v0.3.9
   Compiling cmake v0.1.51
   Compiling num-complex v0.4.6
   Compiling num-integer v0.1.46
   Compiling ndarray v0.16.1
   Compiling errno v0.3.9
   Compiling which v5.0.0
   Compiling metatensor-sys v0.1.10
   Compiling metatensor v0.2.0
   Compiling tarpaulin-metatensor v0.1.0 (...)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 6.31s
2024-11-25T11:39:57.781512Z  INFO cargo_tarpaulin::process_handling: running .../tarpaulin-metatensor/target/debug/deps/tarpaulin_metatensor-d69f48c88f0a5a85
2024-11-25T11:39:57.781582Z  INFO cargo_tarpaulin::process_handling: Setting LLVM_PROFILE_FILE
dyld[98923]: Library not loaded: @rpath/libmetatensor.dylib
  Referenced from: <946ECB99-A829-315D-8879-CA602D522EEB> .../tarpaulin-metatensor/target/debug/deps/tarpaulin_metatensor-d69f48c88f0a5a85
  Reason: tried: '~/.rustup/toolchains/stable-aarch64-apple-darwin/lib/libmetatensor.dylib' (no such file), '~/lib/libmetatensor.dylib' (no such file), '/usr/local/lib/libmetatensor.dylib' (no such file), '/usr/lib/libmetatensor.dylib' (no such file, not in dyld cache)
2024-11-25T11:39:57.782508Z ERROR cargo_tarpaulin: Test failed during run
Error: "Test failed during run"

The file is in target/debug/build/metatensor-sys-2149f4330271b8f4/out/lib/libmetatensor.dylib, and this path should be sent to rustc with cargo:rustc-link-search=native here: https://github.com/metatensor/metatensor/blob/0fbc23d0588eae65ff54f4e1a6158d4eb18d67ff/rust/metatensor-sys/build.rs#L57

This is not urgent for me, I can work around this by statically linking metatensor with my actual code.

@xd009642
Copy link
Owner

aha that's helpful, okay I should be able to solve this my end now I have an MRE!

@xd009642
Copy link
Owner

xd009642 commented Nov 26, 2024

Okay I potentially have a fix in this PR branch #1652 - doesn't yet work on windows and mac but I'll look into them and figure out the right behaviour

@xd009642
Copy link
Owner

This is fixed and in the develop branch, release should be out before the end of the week

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 a pull request may close this issue.

3 participants