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

Update tempfile and uuid to remove rand v0.5 #2195

Merged
merged 1 commit into from
Feb 22, 2020

Conversation

JohnTitor
Copy link
Member

@jtgeibel
Copy link
Member

This change looks great to me! I'd like to hold off on lockfile changes for a bit until we merge #1864. After that, I'd like to aggressively bump and deploy backend dependency changes like this. See also #1265 and #1805. Long story short, we had some segfault issues in production the last few times we tried a cargo update so we'll have to slice and dice the updates and watch for any issues.

In the meantime, I recently got access to https://github.com/conduit-rust and have started opening some PRs there which will enable us to remove some old dependencies pulled in by those crates. Would you mind if I ping you for review on some of those dependency updates?

@JohnTitor
Copy link
Member Author

JohnTitor commented Feb 19, 2020

I'd like to hold off on lockfile changes for a bit until we merge #1864.

I'm fine with that.

Would you mind if I ping you for review on some of those dependency updates?

Sure!

Btw, if we introduce .gitattributes like rust-lang/rust and (it seems unrelated here) new format of lockfile, we can avoid conflicts as possible, I believe. How about?

@jtgeibel
Copy link
Member

new format of lockfile, we can avoid conflicts as possible

I would love to switch, but the only way I know to do that is to delete the lockfile and allow cargo to regenerate it. So my plan is to incrementally work towards a cargo update first so that the switch is a no-op. Unless there is an easy way to keep our existing pinned dependencies while also switching, then I'm all for that.

@Eh2406
Copy link
Contributor

Eh2406 commented Feb 19, 2020

There is a out of tree subcommand for switching: https://github.com/RustSec/cargo-lock#command-line-interface
Or there are ways to trick cargo into switching: rust-lang/cargo#7070 (comment)

For testing locally there's no configuration to generate the new format, just preservation of the new format if it's found. If you edit a Cargo.lock though to remove the version/source information in one of the dependencies entries (just one, no need to do the entire lock file) then Cargo will think it's the new format and change it. For example with Cargo I changed memchr 2.2.0 (registry+https://github.com/rust-lang/crates.io-index) to memchr and then ./target/debug/cargo fetch generated a new lock file.

@JohnTitor
Copy link
Member Author

@Eh2406 Thanks, it's definitely helpful!

@jtgeibel Then, I'll submit a PR to introduce new format once #1864 lands.

@Mark-Simulacrum
Copy link
Member

Note that at least on rustc-perf we may have run into problems with dependabot's handling of the new lockfile format (I think, though can't verify), see rust-lang/rustc-perf#617 -- I've filed a support query with GitHub today, though no responses yet.

@JohnTitor
Copy link
Member Author

IIRC we don't use dependabot for Rust here. I'm not sure about the future though.

@jtgeibel
Copy link
Member

I hope to enable dependabot on the backend (we use it already on the frontend) once we manually get all our dependencies up to date. If the new lockfile format is an issue for dependabot we could hold off on the conversion, but I'm also not too worried. I think I would trade the short-term improvement of fewer merge conflicts for the potential delay in enabling dependabot once we're ready.

@bors
Copy link
Contributor

bors commented Feb 20, 2020

☔ The latest upstream changes (presumably #1864) made this pull request unmergeable. Please resolve the merge conflicts.

bors added a commit that referenced this pull request Feb 21, 2020
Introduce new lockfile format

Introduce new lockfile format, as suggested in #2195.

r? @jtgeibel
@JohnTitor
Copy link
Member Author

Rebased, it's ready to go.

@jtgeibel
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 22, 2020

📌 Commit 7423739 has been approved by jtgeibel

@bors
Copy link
Contributor

bors commented Feb 22, 2020

⌛ Testing commit 7423739 with merge 9653391...

@bors
Copy link
Contributor

bors commented Feb 22, 2020

☀️ Test successful - checks-travis
Approved by: jtgeibel
Pushing 9653391 to master...

@bors bors merged commit 9653391 into rust-lang:master Feb 22, 2020
@JohnTitor JohnTitor deleted the reduce-rand branch February 22, 2020 03:56
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.

6 participants