The previous conversion worked on Linux but failed on macOS where modes, as represented in system data structures, are 16-bit rather than 32-bit. The previous commit broke builds on macOS (and possibly other OSes not currently tested on CI), which this should fix. This commit also does some other refactorings: - Simplify the way conversions are represented. - Express with `expect` that, by this point, there are no unknown bits, rather than doing something that would silently preserve or silently discard them. There would have to be a bug (and probably in `gix-worktree-state` itself) for that not to be the case. - Clarify the TODO comment, and also weaken it since there might be some reason for `set_mode_executable` to keep using `Permissions` in some way.
Unified · Split
| @@ -287,10 +287,11 @@ pub(crate) fn finalize_entry( · 286 unmodified lines | ||
| 287 | 287 | if let Some(path) = set_executable_after_creation { |
| 288 | 288 | let old_perm = std::fs::symlink_metadata(path)?.permissions(); |
| 289 | 289 | if let Some(new_perm) = set_mode_executable(old_perm) { |
| 290 | − // TODO: If the `fchmod` approach is kept, `set_mode_executable` shouldn't operate on std::fs::Permissions. | |
| 291 | − use std::os::{fd::AsFd, unix::fs::PermissionsExt}; | |
| 292 | − rustix::fs::fchmod(file.as_fd(), new_perm.mode().into()) | |
| 293 | − .map_err(|errno| std::io::Error::from_raw_os_error(errno.raw_os_error()))?; | |
| 290 | + // TODO: If we keep `fchmod`, maybe change `set_mode_executable` not to use `std::fs::Permissions`. | |
| 291 | + use std::os::unix::fs::PermissionsExt; | |
| 292 | + let mode = rustix::fs::Mode::from_bits(new_perm.mode()) | |
| 293 | + .expect("`set_mode_executable` shouldn't preserve or add unknown bits"); | |
| 294 | + rustix::fs::fchmod(&file, mode).map_err(std::io::Error::from)?; | |
| 294 | 295 | } |
| 295 | 296 | } |
| 296 | 297 | // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. |