Skip to content

Commit 5024376

Browse files
committed
change: Get and set mode using std, still on open file descriptor
This replaces explicit `fstat` and `fchmod` calls (via `rustix`) with `File::metadata` and `File::set_permissions`, respectively. In practice, on Unix-like systems: - `File::metadata` either: * calls `fstat`, or * calls `statx` in a way that causes it to operate similarly to `fstat` (this is used on Linux, in versions with `statx`). This is not explicitly documented, though calling `stat` or `lstat`, or calling `statx` in a way that would cause it to behave like `stat` or `lstat` rather than `fstat`, would not preserve the documented behavior of `File::metadata`, which operates on a `File` and does not take a path. - `File::set_permissions` calls `fchmod`. This is explicitly documented and `fchmod` is listed as an alias for documentation purposes. But it is noted as an implementation detail that may change without warning. However, calling `chmod` or `lchmod` would not preserve the documented behavior of `File::set_permissions`, which (like `File::metadata`) operates on a file and does not take a path. While these details can, in principle, change without warning, per https://doc.rust-lang.org/stable/std/io/index.html#platform-specific-behavior, to preserve the documented semantics of operating on the file referenced by the file descriptor, it seems like the behavior would still have to be correct for our use. In our use here, what matters is that we use the file descriptor and not the path. The change in this commit intends to maintain the effect of GitoxideLabs#1803 without requiring `gix-worktree-state` to depend directly on `rustix`, and to benefit from the standard library's slightly higher level interface where modes are treated as `u32` even on operating systems where they are different (e.g. `u16` on macOS). Importantly, this continues to differ from the behavior before GitoxideLabs#1803 of using functions that operated on paths. The current general correspondence between Rust `std`, POSIX functions, and `statx`, where the rightmost three columns describe `statx` calls, is: | std::fs::* | POSIX | fd | path | flags | |------------------|-------|----------|------|---------------------| | metadata | stat | AT_FDCWD | path | | | symlink_metadata | lstat | AT_FDCWD | path | AT_SYMLINK_NOFOLLOW | | File::metadata | fstat | file | "" | AT_EMPTY_PATH | It may be that some uses of `rustix::fs` can be similarly replaced in `gix_fs`, but this commit does not include any such changes.
1 parent 810b5cf commit 5024376

File tree

3 files changed

+11
-22
lines changed

3 files changed

+11
-22
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
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: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,3 @@ 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: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,10 @@ pub(crate) fn finalize_entry(
298298
/// See `let_readers_execute` for the exact details of how the mode is transformed.
299299
#[cfg(unix)]
300300
fn set_executable(file: &std::fs::File) -> Result<(), std::io::Error> {
301-
let old_raw_mode = rustix::fs::fstat(file)?.st_mode;
302-
let new_mode = let_readers_execute(old_raw_mode);
303-
rustix::fs::fchmod(file, new_mode)?;
301+
use std::os::unix::fs::{MetadataExt, PermissionsExt};
302+
let old_mode = file.metadata()?.mode();
303+
let new_mode = let_readers_execute(old_mode);
304+
file.set_permissions(std::fs::Permissions::from_mode(new_mode))?;
304305
Ok(())
305306
}
306307

@@ -309,17 +310,13 @@ fn set_executable(file: &std::fs::File) -> Result<(), std::io::Error> {
309310
/// Currently this adds executable bits for whoever has read bits already. It doesn't use the umask.
310311
/// Set-user-ID and set-group-ID bits are unset for safety. The sticky bit is also unset.
311312
///
312-
/// This returns only mode bits, not file type. The return value can be passed to chmod or fchmod.
313+
/// This returns only mode bits, not file type. The return value can be used in chmod or fchmod.
313314
#[cfg(unix)]
314-
fn let_readers_execute(mut raw_mode: rustix::fs::RawMode) -> rustix::fs::Mode {
315-
assert_eq!(
316-
raw_mode & 0o170000,
317-
0o100000,
318-
"bug in caller if not from a regular file"
319-
);
320-
raw_mode &= 0o777; // Clear type, non-rwx mode bits (setuid, setgid, sticky).
321-
raw_mode |= (raw_mode & 0o444) >> 2; // Let readers also execute.
322-
rustix::fs::Mode::from_bits(raw_mode).expect("all bits recognized")
315+
fn let_readers_execute(mut mode: u32) -> u32 {
316+
assert_eq!(mode & 0o170000, 0o100000, "bug in caller if not from a regular file");
317+
mode &= 0o777; // Clear type, non-rwx mode bits (setuid, setgid, sticky).
318+
mode |= (mode & 0o444) >> 2; // Let readers also execute.
319+
mode
323320
}
324321

325322
#[cfg(all(test, unix))]
@@ -372,8 +369,7 @@ mod tests {
372369
(0o106400, 0o500),
373370
(0o102462, 0o572),
374371
];
375-
for (st_mode, raw_expected) in cases {
376-
let expected = rustix::fs::Mode::from_bits(raw_expected).expect("expected mode is a mode");
372+
for (st_mode, expected) in cases {
377373
let actual = super::let_readers_execute(st_mode);
378374
assert_eq!(
379375
actual, expected,

0 commit comments

Comments
 (0)