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 to Rust 2018 #217

Closed
brson opened this issue Jan 3, 2019 · 12 comments
Closed

Upgrade to Rust 2018 #217

brson opened this issue Jan 3, 2019 · 12 comments
Assignees

Comments

@brson
Copy link

brson commented Jan 3, 2019

After TiKV itself is successfully upgraded (tikv/tikv#3896) we can bump prometheus to Rust 2018 as well. Do a major version bump.

@brson brson self-assigned this Jan 3, 2019
This was referenced Jan 3, 2019
@brson
Copy link
Author

brson commented Jan 8, 2019

In thinking about backwards compatibility I'm inclined to explicitly break support for Rust 2015 when we do this upgrade - it's possible to be compatible with both (in some situations, I believe), but by doing so we have to be vigilant about breaking 2015 compatibility. I'm inclined to insert some boilerplate code into each of fail, prometheus, and grpc that is definitely 2018 only and breaks the build under 2015 so we don't have to worry about breaking 2015 compatibility in a minor version bump.

Let me know if anybody is opposed to that.

@Hoverbear
Copy link
Contributor

@brson When you get there please investigate using const fn in our upgrade:

This is a common use case:

lazy_static! {
    static ref TIKV_REQUEST_DURATION_HISTOGRAM_VEC: HistogramVec = register_histogram_vec!(
        "tikv_request_duration_seconds",
        "Bucketed histogram of TiKV requests duration",
        &["type"]
    )
}

@brson
Copy link
Author

brson commented Jan 16, 2019

I have a branch here that does the upgrade https://github.com/brson/rust-prometheus/tree/rust-2018

I have not looked into const fn yet but I will.

@brson
Copy link
Author

brson commented Jan 18, 2019

I'm also trying to figure out how far back in the Rust stable toolchain one has to go before cargo starts just ignoring edition = "2018", since that's when it's theoretically possible to be compatible with both 2015 and 2018. I am hopefully though that cargo started rejecting edition = "2018 at the same time it started accepting crate in paths, making it very unlikely to have a single crate that's compatible with Rust 2018 and pre-2018 toolchains.

@brson
Copy link
Author

brson commented Jan 23, 2019

The tikv toolchain is upgraded to a version that supports 2018 now, so I think once it's had a few days on master w/o being reverted it's safe to go ahead. I'll do so sometime next week.

@brson brson mentioned this issue Jan 28, 2019
@brson
Copy link
Author

brson commented Jan 29, 2019

Oh, I haven't done anything with const fn in the posted patch yet.

@brson
Copy link
Author

brson commented Jan 29, 2019

I filed another bug on const fn because I doubt I'm going to do that work in the upgrade PR.

@brson
Copy link
Author

brson commented Feb 22, 2019

I don't see any CI here. Not sure if that's expected or not.

@Hoverbear
Copy link
Contributor

@brson This is an issue, nothing to test. :)

@brson
Copy link
Author

brson commented Feb 22, 2019

Lol 😆

@brson brson assigned nrc and unassigned brson Mar 14, 2019
@brson
Copy link
Author

brson commented Mar 14, 2019

Reassigned to @nrc since he has a newer PR up

@breezewish
Copy link
Member

Fixed in #242

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

No branches or pull requests

4 participants