Skip to content

Commit

Permalink
change: Set +x via open file descriptor, not path
Browse files Browse the repository at this point in the history
When executable permissions were set on a file being checked out,
and when it was done after the file was already created, this was
done on its path, using `chmod` (via a higher level abstraction).
But we have an open `File` object for it already, at that point.

This uses `fchmod` on the existing open file descriptor (obtained
from the `File` object), instead of `chmod` on the path.

(Since GitoxideLabs#1764, the file mode has also been read first, to figure out
what new mode to set it to for +x. That uses `lstat` (via a higher
level abstraction). The change here is therefore really only half
of what should be done. To complete it, and also to avoid new
problems due to a new inconsistency, that should be done with
`fstat` on the file descriptor instead of `lstat` on the path. That
will be done in a directly fortcoming commit -- after portability
issues in the type of the `st_mode` field, which is not actually
`u32` on all systems, are examined.)
  • Loading branch information
EliahKagan committed Jan 24, 2025
1 parent f58f3ea commit 0becc91
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 1 deletion.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions gix-worktree-state/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,9 @@ gix-filter = { version = "^0.17.0", path = "../gix-filter" }
io-close = "0.3.7"
thiserror = "2.0.0"
bstr = { version = "1.3.0", default-features = false }

[target.'cfg(unix)'.dependencies]
rustix = { version = "0.38.20", default-features = false, features = [
"std",
"fs",
] }
5 changes: 4 additions & 1 deletion gix-worktree-state/src/checkout/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,10 @@ 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) {
std::fs::set_permissions(path, new_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()))?;
}
}
// NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well.
Expand Down

0 comments on commit 0becc91

Please sign in to comment.