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

mod.rs:add_plugin_deps() does not strip native= from path elements #3957

Closed
mcgoo opened this issue Apr 26, 2017 · 3 comments · Fixed by #3974
Closed

mod.rs:add_plugin_deps() does not strip native= from path elements #3957

mcgoo opened this issue Apr 26, 2017 · 3 comments · Fixed by #3974

Comments

@mcgoo
Copy link
Contributor

mcgoo commented Apr 26, 2017

I have a build.rs script in a crate that imports a -sys crate I am working on. Toolchain is x86_64-pc-windows-msvc. The -sys crate emits cargo:rustc-link-search=native=c:\lib and the build script is unable to load because it can't find a dll. The path environment variable contains ...;native=C:\lib;...

A workaround is to emit cargo:rustc-link-search=c:\lib instead.

Bug is present in cargo 0.19 from git, and cargo-0.17.0-nightly (f9e5481 2017-03-03) from rust 1.16 stable.

@alexcrichton
Copy link
Member

Thanks for the report, you're thinking of this location, right? looks like Cargo actually already has logic to handle this it just needs to be applied to more locations!

@mcgoo
Copy link
Contributor Author

mcgoo commented Apr 27, 2017

Yes, that is the correct place. I put a panic in there if the path contains "native=".

I didn't say it above, but the build.rs can't find the DLL but cargo run can.

It looks like cargo run uses Compilation::fill_env(..), but the code you mention above is the !is_host leg of the if so I don't think it gets called? It seems that code adds too much and filters dirs that are outside the target dir, which would not work for what I need.

It looks like the right fix would be related to plugins_dylib_path which gets set here. I traced it back through Layout to Filesystem but I'm not sure how it is supposed to work.

It seems like the build script runner is deliberately avoiding using Compilation like everything else does, so maybe a local fix is fine? It just seems that as soon as you allow a build script to include crates generated as part of the build itself, it starts to need to need the same environment as everything else.

If a local fix is fine, maybe it's as simple as adding paths with native, framework, all, or no qualification, and skip paths with "crate" and "dependency"?

@alexcrichton
Copy link
Member

Oh yes of course, this is just a bug in Cargo itself. Would you be interested in making a PR for this? I'd' be willing to help out and answer questions if so!

mcgoo added a commit to mcgoo/cargo that referenced this issue Apr 28, 2017
mcgoo added a commit to mcgoo/cargo that referenced this issue May 8, 2017
Make dynamic library search path handling for build scripts mirror the
behaviour for cargo run etc. -L paths are taken and stripped of the
native= and similar prefixes and added to the dynamic library search
path if they are inside the target dir.
Resolves rust-lang#3957
bors added a commit that referenced this issue May 8, 2017
fix dynamic search path for build scripts

fixes #3957
@bors bors closed this as completed in #3974 May 8, 2017
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.

2 participants