Skip to content

Commit a34b4ec

Browse files
committed
change: Set +x via open file descriptor, not path
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.)
1 parent 02efddd commit a34b4ec

File tree

3 files changed

+11
-1
lines changed

3 files changed

+11
-1
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gix-worktree-state/Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,9 @@ gix-filter = { version = "^0.17.0", path = "../gix-filter" }
2929
io-close = "0.3.7"
3030
thiserror = "2.0.0"
3131
bstr = { version = "1.3.0", default-features = false }
32+
33+
[target.'cfg(unix)'.dependencies]
34+
rustix = { version = "0.38.20", default-features = false, features = [
35+
"std",
36+
"fs",
37+
] }

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,10 @@ pub(crate) fn finalize_entry(
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-
std::fs::set_permissions(path, new_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()))?;
291294
}
292295
}
293296
// NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well.

0 commit comments

Comments
 (0)