Use the lockfile shipped with rust-src#255
Merged
bors[bot] merged 8 commits intojaparic:masterfrom Sep 18, 2019
Merged
Conversation
This reverts commit a4853d5. compiler-builtins is nowadays pulled in via crates.io, so let's give locking another shot.
Collaborator
Author
|
Hm @jethrogb this makes your |
Collaborator
|
Unfortunately the test framework doesn't show the test output, only that it didn't contain certain substrings... Are we passing |
Collaborator
Author
|
The build log looks like this: Details |
Collaborator
Author
|
In particular it says The lockfile references v1.0.35 of that crate, which likely is why the patch does not apply. |
Collaborator
Author
|
Seems like that worked! CI is green. |
Collaborator
Author
|
@jethrogb what do you think? Should we try to use the lockfile shipped with rust-src? |
Collaborator
|
Not if we have to keep changing the test so we match the version with whatever |
Collaborator
Author
|
I think the benefits far outweigh this cost. Right now, a new release of any of the crates libstd depends on can break the build any time. This has happened recently (when a new rustc-std-workspace-std was released), and it has happened before last year. Maybe we should run that test only on a pinned version of nightly. But I think being able to reliably and reproducibly build old versions of libstd is important, more important than having this particular test work with zero maintenance. |
Collaborator
Author
|
bors try Let's see which version the pinned 2018-12-01 nightly needs, then we can just pin the test to that. |
Contributor
tryBuild failed |
Collaborator
Author
|
bors try |
Contributor
tryBuild succeeded |
Collaborator
Author
|
That seems to have worked. With this, the patch test should not fail as nightly Rust changes. |
Collaborator
|
Ok but you're using the same cc version for every pinned nightly, this will break if we ever add another one. |
Collaborator
|
Can we parse the needed cc version from Cargo.lock? |
Collaborator
Author
It will. I can update the test then (make it depend on which nightly is pinned).
We probably can. I am just not convinced that's worth the effort. But if you insist, I can work on that. Not sure when I will have the time, though. |
jethrogb
reviewed
Sep 18, 2019
src/sysroot.rs
Outdated
|
|
||
| // rust-src comes with a lockfile for libstd. Use it. | ||
| let lockfile = src.path().join("..").join("Cargo.lock"); | ||
| fs::copy(lockfile, &td.join("Cargo.lock")).chain_err(|| "couldn't copy lock file")?; |
Collaborator
There was a problem hiding this comment.
Should this be a hard error?
Collaborator
Author
There was a problem hiding this comment.
Why not? All versions of rust-src that we support have this file. It is present since 2017.
Collaborator
There was a problem hiding this comment.
What if you're getting your source not from the rust-src package? Like git? Or a local git clone?
Collaborator
Author
There was a problem hiding this comment.
Then the lock file exists in the same place.
Collaborator
There was a problem hiding this comment.
Sure but the user may have deleted it. I'm ok with forcing that it exists, but the error message should be slightly clearer.
Collaborator
Author
There was a problem hiding this comment.
Fair. I made it "Cargo.lock file is missing from source dir".
Collaborator
|
r=me after minor change |
Collaborator
Author
|
bors r=jethrogb |
Contributor
Build succeeded |
bors bot
added a commit
that referenced
this pull request
Nov 8, 2019
262: version bump for 0.3.17 release r=jethrogb a=RalfJung Right now Miri is broken on macOS and Windows because an updated compiler_builtins has problems, and we don't use the lockfile that would make us use the old version. So, it's time to release a xargo with #255. Co-authored-by: Ralf Jung <post@ralfj.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts a prior revert.
compiler-builtins is nowadays pulled in via crates.io, so let's give locking another shot.