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

Version 1.1.0 bumps minimum compiler version #114

Closed
jethrogb opened this Issue Aug 6, 2018 · 13 comments

Comments

Projects
None yet
4 participants
@jethrogb
Copy link

jethrogb commented Aug 6, 2018

Version 1.1.0 bumps the minimum compiler version because of the use of associated constants. This is not a semver-compatible change and should be reverted.

@BurntSushi

This comment has been minimized.

Copy link
Member

BurntSushi commented Aug 6, 2018

There is no consensus on whether bumping the MSRV is a semver incompatible change or not.

@jethrogb

This comment has been minimized.

Copy link
Author

jethrogb commented Aug 6, 2018

My CI started failing overnight because of this release even though I employ lockfiles everywhere.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Aug 6, 2018

What does "employing lockfiles everywhere" mean? If your versions are locked, then you wouldn't pick up a new version with a higher compiler version requirement.

@jethrogb

This comment has been minimized.

Copy link
Author

jethrogb commented Aug 6, 2018

Yeah, I suppose that's quite puzzling. I've narrowed it down to an optional dependency that only gets activated through a particular feature. This dependency is not in the lockfile.

@jethrogb

This comment has been minimized.

Copy link
Author

jethrogb commented Aug 6, 2018

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Aug 8, 2018

Yeh the 1.1.0 release didn't bump the minimum compiler version. The additional build.rs specifically exists to avoid bumping the minimum compiler version.

@jethrogb

This comment has been minimized.

Copy link
Author

jethrogb commented Aug 8, 2018

That might not be working correctly then:

02:00:25   --> /home/jenkins/.cargo/registry/src/github.com-1ecc6299db9ec823/lazy_static-1.1.0/src/heap_lazy.rs:17:5
02:00:25    |
02:00:25 17 |     pub const INIT: Self = Lazy(0 as *const T, ONCE_INIT);
02:00:25    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
02:00:25    |
02:00:25    = help: add #![feature(associated_consts)] to the crate attributes to enable
02:00:25 
02:00:25 error: aborting due to previous error(s)
02:00:25 
02:00:25 error: Could not compile `lazy_static`.

I don't get this error on 1.0.2

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Aug 8, 2018

@jethrogb Oh are you compiling lazy_static on a compiler before 1.21.0? While we didn't bump our documented minimum supported version from 1.21.0 we might've bumped our technically supported minimum version by actually using a feature that stabilized close to 1.21.0?

@jethrogb

This comment has been minimized.

Copy link
Author

jethrogb commented Aug 8, 2018

I'm on 1.18

we didn't bump our documented minimum supported version from 1.21.0

Documented where exactly?

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Aug 9, 2018

Documented where exactly?

It's documented as the explicit version we test in Travis through our .travis.yml, but you could reasonably argue that totally doesn't count as documentation. It's clearly not discoverable...

Maybe we should adopt the approach some other crates take and stick a badge in the readme with our minimum version?

@BurntSushi

This comment has been minimized.

Copy link
Member

BurntSushi commented Aug 15, 2018

Digging a little deeper here...

There are no rigid policies on how to handle all of this, but it is a bit of a mess. This also means regex's documented minimum version is technically wrong because it is less than lazy_static's documented minimum version (a dependency of regex). I don't really know what to do about it. I'm not sure it's worth this amount of fuss at this point, at least until we get some sort of LTS release that core crates can rally around.

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Aug 15, 2018

Hm it is a bit of a mess... I think we've made a few assumptions that lead to this specific case:

  • I assumed that because we test 1.21.0 explicitly in CI that it's our minimum version and bumping up to 1.21.0 is ok and expected
  • Others have assumed that because their library that depends on lazy_static happens to compile on a given version that lazy_static supports that version

Some sort of LTS release would nicely cut through those assumptions in the future. It seems like our efforts right now across the ecosystem to manage minimum versions are a bit too ad-hoc to scale.

@BurntSushi

This comment has been minimized.

Copy link
Member

BurntSushi commented Aug 15, 2018

It seems like our efforts right now across the ecosystem to manage minimum versions are a bit too ad-hoc to scale.

I agree. I watch the repos of (almost?) every dependency I use, including this one, and specifically look out for this and I still missed it. :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment