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.

Sign up for GitHub

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

Jump to bottom

cargo test --doc not load same shared libraries as cargo test --lib #8531

Closed
adamcrume opened this issue Jul 23, 2020 · 11 comments · Fixed by #13490
Closed

cargo test --doc not load same shared libraries as cargo test --lib #8531

adamcrume opened this issue Jul 23, 2020 · 11 comments · Fixed by #13490
Labels
A-doctests Area: rustdoc --test C-bug Category: bug E-medium Experience: Medium I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.

Comments

@adamcrume
Copy link

Problem
cargo test --doc doesn't appear to be loading the same shared libraries as cargo test --lib in 1.45

Steps

  1. Clone https://github.com/tensorflow/rust
  2. Set Rust to 1.44
  3. Note that both cargo test --doc and cargo test --lib pass
  4. Set Rust to 1.45
  5. Note that cargo test --lib passes but cargo test --doc fails with error while loading shared libraries: libtensorflow.so.1: cannot open shared object file: No such file or directory

Notes

Output of cargo version:
cargo 1.44.0 (05d080f 2020-05-06) or cargo 1.45.0 (744bd1f 2020-06-15)

Running on Ubuntu 20.04 LTS.
@adamcrume adamcrume added the C-bug Category: bug label Jul 23, 2020
@ehuss ehuss added the A-doctests Area: rustdoc --test label Jul 23, 2020
@thoren-d
Copy link

thoren-d commented Oct 3, 2020

For what it's worth, I'm seeing the same issue running doctests against my library that uses SDL2 (https://github.com/Rust-SDL2/rust-sdl2). cargo +1.44.0 test --doc works.

@adamcrume adamcrume mentioned this issue Oct 15, 2020
Open
@raskr
Copy link

raskr commented Jan 16, 2021 •
edited
Loading

Same problem with any OS + rust1.48+, but not with rust1.42.
cargo test --doc with shared lib seems to be broken.

@adamcrume
Copy link
Author

Using git bisect, I narrowed down to 3fd2814 being the breaking commit.

@scott-wilson scott-wilson mentioned this issue May 15, 2021
Merged
22 tasks
@scott-wilson scott-wilson mentioned this issue Jun 16, 2021
Open
jackgerrits added a commit to jackgerrits/vowpalwabbit-rs that referenced this issue Aug 5, 2021
@jackgerrits
Turn off doctest due to bug in tooling rust-lang/cargo#8531
1c3c339
@pipehappy1
Copy link

This is still broken on 1.58

sgrif added a commit to sgrif/cargo that referenced this issue Mar 9, 2022
@sgrif
Properly set dylib search path for doctests
88b413e 
Fixes rust-lang#8531.

This issue was introduced by 3fd2814.
Prior to that commit, the `rustdoc_process` function took other branch
of the if statement being modified by this commit. The flag was
previously called `is_host`, and `rustdoc` was run reporting `false`. In
that commit, the flag was changed to `is_rustc_tool`, and rustdoc began
reporting `true`, which resulted in the native directories added by
build scripts no longer being appended to LD_LIBRARY_PATH when running
doctests.

This commit changes the behavior so that we are always appending the
same set of paths, and the only variance is if we are cross compiling or
not. An alternative would be to change rustdoc to always pass
`kind: CompileKind::Host, is_rustc_tool: false`, but as rustdoc is in
fact a rustc tool, that feels wrong. The commit which introduced the bug
did not include context on why the flag was changed to `is_rustc_tool`,
so I don't have a sense for if we should just change that flag to mean
something else.

While the test suite previously had an explicit test for the
library path behavior of `cargo run`, it did not test this for various
test forms. This commit modifies the test to ensure the behavior is
consistent across `run`, normal tests, and doctests.
@sgrif sgrif mentioned this issue Mar 9, 2022
Closed
@Trolldemorted
Copy link

Trolldemorted commented May 10, 2022 •
edited
Loading

This issue is known for almost 3 years, could you give us a timeline when a fix will come? Not knowing about this I just wasted hours trying to find out why my doc tests fail :(

@Trolldemorted Trolldemorted mentioned this issue May 10, 2022
Closed
@justsmth
Copy link
Contributor

justsmth commented Jun 15, 2022 •
edited
Loading

Is there a workaround for this issue?

Update:
I found that I can workaround this by prefixing the command with LD_LIBRARY_PATH indicating the location of the shared libraries: LD_LIBRARY_PATH=... cargo test --doc

@Trolldemorted
Copy link

@adamcrume do you have any clue what might have caused this bug? Is @alexcrichton still around? Iirc he was stepping down from several Rust projects, but not cargo.

@adamcrume
Copy link
Author

I don't know what specifically in 3fd2814 caused this. I'm not familiar with the cargo codebase.

@weihanglo
Copy link
Member

We appreciate Alex efforts on Cargo and Alex has already retired from the Cargo Team.1

There was a previous attempt #10469 and concerns were listed #10469 (comment). Feel free to pick it up and continue.

Footnotes

  1. https://blog.rust-lang.org/inside-rust/2022/03/31/cargo-team-changes.html ↩

@weihanglo weihanglo added S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. A-environment-variables Area: environment variables E-medium Experience: Medium and removed A-environment-variables Area: environment variables labels May 14, 2023
@Trolldemorted
Copy link

@weihanglo with all due respect, the concerns you linked do not point towards a, obvious, clear path forward how this issue should be remediated. I do understand you rely on others to help you fix bugs, but a community can do that with reasonable efficacy only if it is crystal clear how things should work. The concerns you listed are referenced and thought through, but they don't enable outsiders to grasp how you'd want things to be done.

This bug is a problem for the Rust ecosystem. I'd love to recommend Rust more strongly in industrial projects, but this is one of the reasons why I cannot. The cargo team has broken the test coverage for all crates that include native dylibs, and some crates have responded by disabling their doctests with no_run, other crates have stopped efforts to link against included native libraries with a guaranteed matching version, and instead rely on package managers installing a more or less matching version, thus causing undefined behavior. Cargo should not ignore bugs that incentivize maintainers to reduce test coverage or cause undefined behaviour for years.

@ehuss I hate to be nagging, but at which point in time will the cargo team fix this issue, if no volunteers are able to address it in a way you'd accept it? How do you select which bug fixes are scheduled for a new cargo release?

@justsmth justsmth mentioned this issue Jun 28, 2023
Merged
@noel2004 noel2004 mentioned this issue Aug 16, 2023
Open
yasuo-ozu added a commit to QunaSys/z3.rs that referenced this issue Aug 23, 2023
@yasuo-ozu
Implement 'download-z3' flag
57a2477 
In some system, `cargo test --doc` may fail due to the bug:
rust-lang/cargo#8531
@tompscanlan
Copy link
Contributor

Looks as though no one is looking at this. I'm new to the code, but this seems approachable to me. I'll take a wack if it's really open.

@tompscanlan tompscanlan mentioned this issue Sep 20, 2023
Closed
@justsmth justsmth mentioned this issue Feb 1, 2024
Merged
@weihanglo weihanglo added the I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting label Feb 8, 2024
@weihanglo weihanglo mentioned this issue Feb 8, 2024
Open
@weihanglo weihanglo mentioned this issue Feb 26, 2024
Merged
@ehuss ehuss mentioned this issue Jun 6, 2024
Open
@goatgoose goatgoose mentioned this issue Jun 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-doctests Area: rustdoc --test C-bug Category: bug E-medium Experience: Medium I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.
Projects
Cargo status tracker
Archived in project
testing-devex backlog
Status: Done
Milestone
No milestone
9 participants
@ehuss @adamcrume @tompscanlan @thoren-d @raskr @pipehappy1 @Trolldemorted @weihanglo @justsmth