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

cargo test doesn't overwrite the build artifact in target/debug on recent toolchains #6162

Closed
skletsun opened this issue Oct 10, 2018 · 3 comments · Fixed by #6167
Closed

Comments

@skletsun
Copy link

Please check out the minimized project to reproduce: https://github.com/skletsun/cargo_feature_dylib_issue.
There is a workspace that contains a dynamic library and a project that depends on it. The library has a feature. The problem is that the same building and testing scenario does not work for recent toolchains:

Working scenario:

  1. cargo +1.26.2 build --features feature
  2. cargo +1.26.2 test --all -- --nocapture

Doesn't work:

  1. cargo +1.28.0 build --features feature
  2. cargo +1.28.0 test --all -- --nocapture

My current observations (tested on OSX): Working scenario - at step (1) library libcargo_check.dylib gets built within the target/debug/deps directory and then copied to target/debug. At step (2) it gets rebuilt (because of the missing feature), also copied (overwriting the old one) to the target/debug and then the test app is executed being linked against the shared library in the target/debug. When I try doing the same with newer toolchain (non-working scenario) it behaves almost same way except that on step (2) the library gets rebuilt into target/debug/deps BUT NOT COPIED to the target/debug and the test app is executed against the old version of the dynamic library in target/debug. And if I manually delete the outdated libcargo_check.dylib in target/debug then cargo +1.28.0 test --all -- --nocapture works as expected because the test app is executed against dynamic library located in target/debug/deps.

The original Rust project is a part of a complex project that involves Java, so the whole project gets built with Maven, which in turn executes cargo for building and testing in some order. My expectation is that cargo test should behave the same way as in previous versions of toolchains.

I tested it against a few versions of toolchain and can say that it works for 1.26.2 and 1.27.2, but doesn't work for 1.28.0 and 1.29.1.

Thanks in advance!

@alexcrichton
Copy link
Member

cc @ehuss, I think we were discussing this recently on discord, right? In that this is expected behavior currently, but one that we can indeed fix! (and probably should for dylib outputs at least)

@ehuss
Copy link
Contributor

ehuss commented Oct 11, 2018

I think it was in #6131 where we discussed that uplifting was changed.

There are multiple things here:

  • build/test/run commands don't clean the target directory. Artifacts from previous runs normally don't matter, but for shared libs they sometimes do.
  • The dylib search path includes the root target dir and the deps target dir, and the root one is taking precedence (swapping them here fixes this issue)

When we changed uplifting, it was a concern that so's might have issues, but at the time we couldn't think of anything specific. Looks like we found one! 😄

I see three possible solutions:

  1. Clean the base target directory before each command runs. Right now it's a dumping ground. However, I don't like the sound of this idea.
  2. Flip the order of the dylib path mentioned above.
  3. Copy all shareable libraries (dylib/cdylib) of dependencies into the target.

I don't have a preference on which. 2 is super easy, but I don't know the implications. 3 is a little harder, but doable.

@alexcrichton
Copy link
Member

Thanks for investigating this @ehuss! Option (2) there sounds like a great way to fix this to me, I definitely don't think we should have much fallout from switching the precedence there and we can always pursue (3) independently later if necessary

bors added a commit that referenced this issue Oct 12, 2018
Fix dylib reuse with uplift.

Due to #5460, the files that are copied to the root of the target dir depend on whether or not it is a direct target. This causes a problem with dylibs if you build a dylib directly, and then separately run a target that depends on that dylib.  It will run with the dylib path preferring the root dir, which picks up the old dylib.  This PR swaps the preference of the dylib search path so that the dylib in the `deps` dir is preferred.

This is maybe not the best solution for dynamic dependencies. I can imagine if you are trying to create a package, you'll want all of the shared libs along with the binary, and digging through the `deps` dir is not optimal.  However, unconditionally linking dependencies into the root has issues (which #5460 fixed).  There are other issues because shared libs do not include a hash in the filename.  A grander solution might involve coming up with a better strategy for organizing the `target` directory to avoid conflicts between targets.

Fixes #6162
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