-
Notifications
You must be signed in to change notification settings - Fork 266
Update nightly version and upgrade futures dep #904
Conversation
This is now blocked by rust-lang/rust#57518. Anything after nightly-2019-01-08 segfaults when running in Nix. So looks like we can't upgrade yet without completely breaking CI, until this is fixed upstream. |
well if the idea is just to get off the forked futures-preview, lets just change the build to 2019-01-08 no? We don't actually have to wait for the upstream problems to be fixed to solve that problem. |
And actually I think the real issue here is that we aren't pinning all our version by committing cargo lock. Isn't it? I think its time we actually did that to solve the problem so we don't keep having this issue come back and bite us. |
It seems like we all agree that the real problem is that things break because of upstream changes. Thus the latest commit that pins this back to 2018-12-26 and alpha.11, which works for CI. Note that to make this work I had to override the broken non-internally pinned dependencies in futures-previews to the sub-crates. |
@zippy this is nice in that we don't need a fork anymore, but it doesn't change anything else. Anyone who tries to use a more recent nightly will still see breaking changes, as in |
I don't think the moving forward of nightly is the problem. The code actually compiles on anything up to nightly 2019-01-08 with this change. It's not the advance of nightly that was the problem (except for because of the nix overlay issue) it was that the futures wasn't pinned. AND we tell people in the readme that they must use the pinned version of nightly and enforce that in both the Makefile and the nix shell, so that should be enough. However I've still bumped to 2019-01-08 just to be moving things forward... |
@sphinxc0re, can you re-review? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kind of a cheatty approve since I added a bunch of the code here....
If we're ready to upgrade our nightly version (it's been almost a month), we can get off of the futures-preview fork. We've had a few issues come in with people running into the
Pin::set
discrepancy, probably because they are on more recent nightly versions.