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

Pass target environment for rustdoc #3205

Merged
merged 1 commit into from
Oct 17, 2016

Conversation

matklad
Copy link
Member

@matklad matklad commented Oct 17, 2016

This should fix #3200, but I am not sure that this is a correct fix, and I need some input to figure this out.

rustdoc is invoked in two places, in cargo_test.rs and in cargo_rustc/mod.rs. Before the refactoring PR, these invocations used different LD_LIBRARY paths. The one in cargo_rustc used "host" version, while the one in cargo_test used "target" version.

The original PR changed both to "host", this PR switches both to "target". Is this correct, or should we stick with different environments for building documentation and doctests?

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

I think this is correct, thanks! We can only run rustdoc tests in the case that host == target, so in that case it shouldn't matter which is used so long as both trigger the same library path logic.

@bors: r+

@bors
Copy link
Collaborator

bors commented Oct 17, 2016

📌 Commit 21cdb63 has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Oct 17, 2016

⌛ Testing commit 21cdb63 with merge 6acf50f...

bors added a commit that referenced this pull request Oct 17, 2016
Pass target environment for rustdoc

This should fix #3200, but I am not sure that this is a correct fix, and I need some input to figure this out.

`rustdoc` is invoked in two places, in `cargo_test.rs` and in `cargo_rustc/mod.rs`. Before the refactoring PR, these invocations used different LD_LIBRARY paths. [The one in cargo_rustc](https://github.com/rust-lang/cargo/pull/3198/files#diff-59acd1a3101aebbb591ac7ab51c19d9eR427) used "host" version, while [the one in cargo_test](https://github.com/rust-lang/cargo/blob/a8baa5b8f36e88170c8c56523b6eb72efc2cc55e/src/cargo/ops/cargo_test.rs#L131) used "target" version.

The original PR changed both to "host", this PR switches both to "target". Is this correct, or should we stick with different environments for building documentation and doctests?
@bors
Copy link
Collaborator

bors commented Oct 17, 2016

💔 Test failed - cargo-win-gnu-64

@alexcrichton
Copy link
Member

@bors: retry

On Mon, Oct 17, 2016 at 9:00 AM, bors notifications@github.com wrote:

💔 Test failed - cargo-win-gnu-64
https://buildbot.rust-lang.org/builders/cargo-win-gnu-64/builds/749


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3205 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95MjVIIPZHUHFqKGP5CEltg2hBVhVks5q05uKgaJpZM4KYpWt
.

@bors
Copy link
Collaborator

bors commented Oct 17, 2016

⚡ Previous build results for cargo-linux-32, cargo-linux-64, cargo-mac-64, cargo-win-msvc-64 are reusable. Rebuilding only cargo-cross-linux, cargo-mac-32, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32...

@bors
Copy link
Collaborator

bors commented Oct 17, 2016

💔 Test failed - cargo-win-gnu-64

@alexcrichton
Copy link
Member

@bors: retry

On Mon, Oct 17, 2016 at 9:13 AM, bors notifications@github.com wrote:

💔 Test failed - cargo-win-gnu-64
https://buildbot.rust-lang.org/builders/cargo-win-gnu-64/builds/750


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3205 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95Pn3lgDgVElTp8bTi7jku2p6EUKlks5q0565gaJpZM4KYpWt
.

@bors
Copy link
Collaborator

bors commented Oct 17, 2016

⚡ Previous build results for cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-msvc-32, cargo-win-msvc-64 are reusable. Rebuilding only cargo-cross-linux, cargo-win-gnu-64...

@bors
Copy link
Collaborator

bors commented Oct 17, 2016

☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64
Approved by: alexcrichton
Pushing 6acf50f to master...

@bors bors merged commit 21cdb63 into rust-lang:master Oct 17, 2016
@matklad matklad deleted the fix-rustdoc-ld-path branch December 14, 2016 15:27
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 this pull request may close these issues.

A linkage error when running tests relying on a sys crate
4 participants