Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When adding +x, get and set mode through the file descriptor #1803

Merged
merged 8 commits into from
Jan 25, 2025
Prev Previous commit
Next Next commit
Convert the mode more portably
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.
EliahKagan committed Jan 24, 2025
commit ddaf8a6cfc5ffbc87124605fe9644ca8d9bdaa29
9 changes: 5 additions & 4 deletions gix-worktree-state/src/checkout/entry.rs
Original file line number Diff line number Diff line change
@@ -287,10 +287,11 @@ pub(crate) fn finalize_entry(
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 the `fchmod` approach is kept, `set_mode_executable` shouldn't operate on std::fs::Permissions.
use std::os::{fd::AsFd, unix::fs::PermissionsExt};
rustix::fs::fchmod(file.as_fd(), new_perm.mode().into())
.map_err(|errno| std::io::Error::from_raw_os_error(errno.raw_os_error()))?;
// TODO: If we keep `fchmod`, maybe change `set_mode_executable` not to use `std::fs::Permissions`.
use std::os::unix::fs::PermissionsExt;
let mode = rustix::fs::Mode::from_bits(new_perm.mode())
.expect("`set_mode_executable` shouldn't preserve or add unknown bits");
rustix::fs::fchmod(&file, mode).map_err(std::io::Error::from)?;
}
}
// NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well.