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.

Sign up for GitHub

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

Jump to bottom

Revert "use Cargo.lock file from rust-src if present" #190

Merged
merged 3 commits into from
Dec 28, 2017
Merged

Revert "use Cargo.lock file from rust-src if present" #190

merged 3 commits into from
Dec 28, 2017

Conversation

japaric
Copy link
Owner

@japaric japaric commented Dec 27, 2017 •
edited by RalfJung
Loading

This reverts 517a614.

@japaric
Copy link
Owner Author

japaric commented Dec 27, 2017

@homunkulus r+

@homunkulus
Copy link
Contributor

📌 Commit 192bbc2 has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit 192bbc2 with merge 64c7275...

japaric pushed a commit that referenced this pull request Dec 27, 2017
@homunkulus
Auto merge of #190 - japaric:no-lockfile, r=japaric
64c7275 
Revert "use Cargo.lock file from rust-src if present"

None
@homunkulus
Copy link
Contributor

💔 Test failed - status-appveyor

@japaric
Copy link
Owner Author

japaric commented Dec 27, 2017

@homunkulus r+

@homunkulus
Copy link
Contributor

📌 Commit 3533108 has been approved by japaric

japaric pushed a commit that referenced this pull request Dec 27, 2017
@homunkulus
Auto merge of #190 - japaric:no-lockfile, r=japaric
191ca4d 
Revert "use Cargo.lock file from rust-src if present"

None
@homunkulus
Copy link
Contributor

⌛ Testing commit 3533108 with merge 191ca4d...

@homunkulus
Copy link
Contributor

💔 Test failed - status-appveyor

@japaric
Copy link
Owner Author

japaric commented Dec 28, 2017

@homunkulus r+

@homunkulus
Copy link
Contributor

📌 Commit 682cdbe has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit 682cdbe with merge 4289077...

japaric pushed a commit that referenced this pull request Dec 28, 2017
@homunkulus
Auto merge of #190 - japaric:no-lockfile, r=japaric
4289077 
Revert "use Cargo.lock file from rust-src if present"

None
@japaric
Copy link
Owner Author

japaric commented Dec 28, 2017

@homunkulus r+

@homunkulus
Copy link
Contributor

📌 Commit e4d493f has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit e4d493f with merge aca7071...

japaric pushed a commit that referenced this pull request Dec 28, 2017
@homunkulus
Auto merge of #190 - japaric:no-lockfile, r=japaric
aca7071 
Revert "use Cargo.lock file from rust-src if present"

None
@homunkulus
Copy link
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: japaric
Pushing aca7071 to master...

@homunkulus homunkulus merged commit e4d493f into master Dec 28, 2017
@homunkulus homunkulus mentioned this pull request Dec 28, 2017
Closed
@japaric japaric deleted the no-lockfile branch December 28, 2017 01:20
@RalfJung
Copy link
Collaborator

RalfJung commented Sep 9, 2019

Hm, is there any discussion anywhere of why this revert happened?

As a matter of principle, using the lockfile seems to make much more sense for the reasons laid out in rust-lang/wg-cargo-std-aware#12: it is the only way that we can reliably build old versions of libstd. And also practically speaking, every now and then an update lands on crates.io that is not forwards-compatible enough to keep libstd working -- it might add a warning (and we build with deny(warnings)), or it might circumvent our patch section. Applying the lockfile is crucial to avoid such issues.

@RalfJung RalfJung mentioned this pull request Sep 9, 2019
Merged
bors bot added a commit that referenced this pull request Sep 18, 2019
@bors @RalfJung
Merge #255
a880315 
255: Use the lockfile shipped with rust-src r=jethrogb a=RalfJung

This reverts a [prior revert](#190).
compiler-builtins is nowadays pulled in via crates.io, so let's give locking another shot.

Co-authored-by: Ralf Jung <post@ralfj.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@japaric @homunkulus @RalfJung