Skip to content

Commit

Permalink
Improve warning suppressions in finalize_entry
Browse files Browse the repository at this point in the history
- 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).
  • Loading branch information
EliahKagan committed Jan 22, 2025
1 parent 3afb4f1 commit 074dc7a
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions gix-worktree-state/src/checkout/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,20 +275,21 @@ pub(crate) fn open_file(
try_op_or_unlink(path, overwrite_existing, |p| options.open(p)).map(|f| (f, set_executable_after_creation))
}

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

0 comments on commit 074dc7a

Please sign in to comment.