Skip to content

Commit 69b84e7

Browse files
committed
Improve warning suppressions in finalize_entry
- Make the conditional `unused_variable` suppression specific to the `set_executable_after_creation` parameter, which is all that needs it. This also makes it more immediately clear that nothing along the lines of `chmod +x` is done (or relevant) on Windows. - Allow `useless_conversion` for the conversion from `u32` to `rustix::fs::RawMode`. The `RawMode` type is what `rustix::fs` calls `mode_t` as used for the `st_mode` field of a `stat` structure. On most operating systems, it is `u32`, but this is not guaranteed by POSIX (which does not even guarantee that it is unsigned). It is `u16` at least on macOS and possibly other systems, and I am unsure if there are systems this code can run on where it is some other type besides `u16` or `u32`. For now, this does not attempt to make that suppression conditional, even though it is only needed when `RawMode` is already `u32`. I don't if there is a good way to express the condition under which it should apply. If there is, then it could be made conditional or, if it is truly reliable, then the conversion itself could be made conditional (though I suspect it is omitted when not needed in release builds, by optimization).
1 parent dc4ef3f commit 69b84e7

File tree

1 file changed

+5
-4
lines changed

1 file changed

+5
-4
lines changed

gix-worktree-state/src/checkout/entry.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,20 +275,21 @@ pub(crate) fn open_file(
275275
try_op_or_unlink(path, overwrite_existing, |p| options.open(p)).map(|f| (f, set_executable_after_creation))
276276
}
277277

278-
/// Close `file` and store its stats in `entry`, possibly setting `file` executable depending on `set_executable_after_creation`.
279-
#[cfg_attr(windows, allow(unused_variables))]
278+
/// Close `file` and store its stats in `entry`, possibly setting `file` executable depending on
279+
/// `set_executable_after_creation`.
280280
pub(crate) fn finalize_entry(
281281
entry: &mut gix_index::Entry,
282282
file: std::fs::File,
283-
set_executable_after_creation: Option<&Path>,
283+
#[cfg_attr(windows, allow(unused_variables))] set_executable_after_creation: Option<&Path>,
284284
) -> Result<(), crate::checkout::Error> {
285285
// For possibly existing, overwritten files, we must change the file mode explicitly.
286286
#[cfg(unix)]
287287
if let Some(path) = set_executable_after_creation {
288288
let old_perm = std::fs::symlink_metadata(path)?.permissions();
289289
if let Some(new_perm) = set_mode_executable(old_perm) {
290-
// TODO: If we keep `fchmod`, maybe change `set_mode_executable` not to use `std::fs::Permissions`.
290+
// TODO: If we keep `fchmod`, maybe `set_mode_executable` shouldn't use `std::fs::Permissions`.
291291
use std::os::unix::fs::PermissionsExt;
292+
#[allow(clippy::useless_conversion)] // mode_t is u32 on many but not all OSes. It's u16 on macOS.
292293
let raw_mode = new_perm.mode().try_into().expect("mode fits in `st_mode`");
293294
let mode = rustix::fs::Mode::from_bits(raw_mode)
294295
.expect("`set_mode_executable` shouldn't preserve or add unknown bits");

0 commit comments

Comments
 (0)