search

rust-osdev / x86_64

Library to program x86_64 hardware.
https://docs.rs/x86_64
Apache License 2.0
797 stars 132 forks source link

CI failure because of stabilized `const_mut_refs` feature #499

Closed phil-opp closed 1 month ago

phil-opp commented 1 month ago

The const_mut_refs feature was stabilized on nightly, which results in the following warning:

warning: the feature `const_mut_refs` has been stable since 1.83.0-nightly and no longer requires an attribute to enable
 --> src/lib.rs:5:43
  |
5 | #![cfg_attr(feature = "const_fn", feature(const_mut_refs))] // GDT::append()

On the CI we treat warnings as errors, which leads to build errors: https://github.com/rust-osdev/x86_64/actions/runs/11208838715/job/31153121364

There are multiple ways to handle this:

I'm leaning towards the third option, but I wanted to hear your opinions @josephlr and @Freax13.

Once const_mut_refs is stablilized on stable, we can use rustversion to make the GDT::append function const on stable builds too. Then we can maybe deprecate the const_fn feature.

Freax13 commented 1 month ago

The const_mut_refs feature was stabilized on nightly, which results in the following warning:

warning: the feature `const_mut_refs` has been stable since 1.83.0-nightly and no longer requires an attribute to enable
 --> src/lib.rs:5:43
  |
5 | #![cfg_attr(feature = "const_fn", feature(const_mut_refs))] // GDT::append()

On the CI we treat warnings as errors, which leads to build errors: https://github.com/rust-osdev/x86_64/actions/runs/11208838715/job/31153121364

There are multiple ways to handle this:

  • Remove the #![cfg_attr(feature = "const_fn", feature(const_mut_refs))] feature gate. This will break people on older nightlies.

I'm not sure that we can prevent breaking older nightlies in the long run. The problem is that it's fairly common for nightly features to be renamed or split into multiple smaller features.

  • Try to use rustversion if possible. I tried it, but I get an "inner macro attributes are unstable" error.

This might become feasible once cfg(version) is stabilized.

  • Set the stable_features lint to warn on CI. This way, the warning is not turned into an error.

That's fine with me.

I'm leaning towards the third option, but I wanted to hear your opinions @josephlr and @Freax13.

Once const_mut_refs is stablilized on stable, we can use rustversion to make the GDT::append function const on stable builds too. Then we can maybe deprecate the const_fn feature.

👍

phil-opp commented 1 month ago

I'm not sure that we can prevent breaking older nightlies in the long run. The problem is that it's fairly common for nightly features to be renamed or split into multiple smaller features.

I fully agree! We definitely shouldn't put any extra work into supporting older nightlies. People that pin specific nightly versions can also pin specific x86_64 versions.

I just thought that setting stable_features = warn is not any extra work and it will probably spare us some work in the future. However, it will result in warnings, which are probably visible in downstream crates too. I'm not sure if this is preferable.

Freax13 commented 1 month ago

I just thought that setting stable_features = warn is not any extra work and it will probably spare us some work in the future.

Sounds good to me.

However, it will result in warnings, which are probably visible in downstream crates too. I'm not sure if this is preferable.

I don't think they are visible. AFAIK warnings only get displayed for crates referenced using path = "...", but not for crates referenced using version = "..." or even git = "...".

phil-opp commented 1 month ago

I tried -Dwarnings -Wstable-features in https://github.com/rust-osdev/x86_64/pull/500, but it does not seem to work like I assumed. I'll open a PR to just remove the const_mut_refs feature instead.