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

Upgrade rustup used on AppVeyor #3996

Merged
merged 1 commit into from
May 6, 2017
Merged

Conversation

alexcrichton
Copy link
Member

No description provided.

@rust-highfive
Copy link

r? @brson

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

@alexcrichton
Copy link
Member Author

I've totally forgotten the error, let's find out what it is again!

@bors: r+

@bors
Copy link
Contributor

bors commented May 4, 2017

📌 Commit 3e1ed70 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented May 4, 2017

⌛ Testing commit 3e1ed70 with merge 655a073...

@bors
Copy link
Contributor

bors commented May 4, 2017

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member Author

@bors: r+

@bors
Copy link
Contributor

bors commented May 5, 2017

📌 Commit 4ff9b5e has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented May 5, 2017

⌛ Testing commit 4ff9b5e with merge f1ea4e7...

bors added a commit that referenced this pull request May 5, 2017
@bors
Copy link
Contributor

bors commented May 5, 2017

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member Author

@bors: r+

@bors
Copy link
Contributor

bors commented May 5, 2017

📌 Commit d869907 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented May 5, 2017

⌛ Testing commit d869907 with merge eae061f...

@bors
Copy link
Contributor

bors commented May 5, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member Author

alexcrichton commented May 6, 2017 via email

@bors
Copy link
Contributor

bors commented May 6, 2017

⌛ Testing commit d869907 with merge 20a085e...

bors added a commit that referenced this pull request May 6, 2017
@bors
Copy link
Contributor

bors commented May 6, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 20a085e to master...

@bors bors merged commit d869907 into rust-lang:master May 6, 2017
bors added a commit that referenced this pull request May 9, 2017
fix `cargo test` of dylib projects for end user runs too

Fixes running `cargo test` and `cargo test --target <target>` for dylib projects.

Moves the logic just landed in #3996 into cargo itself, so cargo sets the dylib path for anyone running `cargo test` or `cargo run`. Current master sets the dylib path only for  `cargo test` on cargo itself.

This PR pins to rustup 1.2.0 for the purposes of testing. If rust-lang/rustup#1093 ends up working out, then this PR would only be important for non-rustup users and people doing cross testing, `cargo test --target <target>`.

Arguably https://github.com/mcgoo/cargo/blob/ed273851f8bc76f726eda4a2e2a7bb470c3718bc/src/cargo/ops/cargo_rustc/context.rs#L249-L253 should point to lib/rustlib/\<host triple\>/lib instead of sysroot/lib, because I think if the libs are different, you will never be able to compile a working plugin anyway, and for the host=target case you get the  lib/rustlib/\<host triple\>/lib anyhow. Is there ever a case where the lib/rustlib/\<host triple\>/lib and sysroot/lib versions of the libs would be expected to differ?

This is not a huge deal for me one way or the other - it doesn't impact my workflow at all. I nearly dropped it when I saw @alexcrichton had made it all work in 3996, but I think it's worth doing because it removes a surprise. It certainly would have saved me a couple of days of confusion. Either way, thanks for looking it over.
@alexcrichton alexcrichton deleted the rustup-up branch May 11, 2017 14:53
@ehuss ehuss added this to the 1.19.0 milestone Feb 6, 2022
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.

5 participants