nix-rust / nix

Rust friendly bindings to *nix APIs
MIT License
2.57k stars 650 forks source link

chore: ignore lint unsafe_op_in_unsafe_fn for usage of env::remove_var #2421

Closed SteveLauC closed 1 month ago

SteveLauC commented 1 month ago

What does this PR do

std::env::remove_var has been made unsafe. Due to lint unsafe_op_in_unsafe_fn, we need to wrap it in an unsafe block.

This PR is for the following CI failure (full log here):

error[E0133]: call to unsafe function `std::env::remove_var` is unsafe and requires unsafe block
  --> src/env.rs:52:17
   |
52 |                 env::remove_var(name);
   |                 ^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
   |
   = note: for more information, see issue #71668 <https://github.com/rust-lang/rust/issues/71668>
   = note: consult the function's documentation for information on how to avoid undefined behavior
note: an unsafe function restricts its caller, but its body is safe by default
  --> src/env.rs:41:1
   |
41 | pub unsafe fn clearenv() -> std::result::Result<(), ClearEnvError> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: the lint level is defined here
  --> src/lib.rs:94:9
   |
94 | #![deny(unsafe_op_in_unsafe_fn)]
   |         ^^^^^^^^^^^^^^^^^^^^^^

std::env::remove_var() will become unsafe ONLY in Rust 2024, Nix is using Rust 2021, so the above issue should not happen. I have filed an issue for this bug, before it gets fixed, ignore the lint unsafe_op_in_unsafe_fn here.

Checklist:

SteveLauC commented 1 month ago

Well, we should only do this for toolchains newer than nightly 2024-05-31, this can be achieved with the help of rustversion.

But I kinda think introducing a new dependency just for this issue is not worthwhile, maybe we should just ignore the lint for this case.

SteveLauC commented 1 month ago

This is a breaking change and should be only available in Rust 2024, we are using Rust 2021 so we should not encounter this. I have filed an issue to the Rust repo.