This extracts the logic that calls `fstat`, transforms the mode with `let_readers_execute`, and writes it back through the same file descriptor with `fchmod`, introducing a new helper function, `set_executable`, for it. The reason to do this is that it makes the issue of what kind of `Error` appears in the returned `Result` clearer. This may be slightly clearer to humans. It's significnatly clearer to the Rust compiler, allowing both `map_err(std::io::Error::from)` calls to be eliminated without having to write anything confusing or inelegant. (This is separate from the reason the `let_readers_execute` function, which does not access the filesystem, is its own function. That is so it's easy to test the mode transformation logic.)
Unified · Split
| @@ -285,9 +285,7 @@ pub(crate) fn finalize_entry( · 284 unmodified lines | |||
| 285 | // For possibly existing, overwritten files, we must change the file mode explicitly. | 285 | // For possibly existing, overwritten files, we must change the file mode explicitly. |
| 286 | #[cfg(unix)] | 286 | #[cfg(unix)] |
| 287 | if let Some(path) = set_executable_after_creation { | 287 | if let Some(path) = set_executable_after_creation { |
| 288 | let old_raw_mode = rustix::fs::fstat(&file).map_err(std::io::Error::from)?.st_mode; | 288 | set_executable(&file)?; |
| 289 | let new_mode = let_readers_execute(old_raw_mode); | ||
| 290 | rustix::fs::fchmod(&file, new_mode).map_err(std::io::Error::from)?; | ||
| 291 | } | 289 | } |
| 292 | // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. | 290 | // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. |
| 293 | // revisit this once there is a bug to fix. | 291 | // revisit this once there is a bug to fix. |
| @@ -296,6 +294,17 @@ pub(crate) fn finalize_entry( · 2 unmodified lines | |||
| 296 | Ok(()) | 294 | Ok(()) |
| 297 | } | 295 | } |
| 298 | 296 | ||
| 297 | /// Use `fstat` and `fchmod` on a file descriptor to make a regular file executable. | ||
| 298 | /// | ||
| 299 | /// See `let_readers_execute` for the exact details of how the mode is transformed. | ||
| 300 | #[cfg(unix)] | ||
| 301 | fn set_executable(file: &std::fs::File) -> Result<(), std::io::Error> { | ||
| 302 | let old_raw_mode = rustix::fs::fstat(&file)?.st_mode; | ||
| 303 | let new_mode = let_readers_execute(old_raw_mode); | ||
| 304 | rustix::fs::fchmod(&file, new_mode)?; | ||
| 305 | Ok(()) | ||
| 306 | } | ||
| 307 | |||
| 299 | /// Given the st_mode of a regular file, compute the mode with executable bits safely added. | 308 | /// Given the st_mode of a regular file, compute the mode with executable bits safely added. |
| 300 | /// | 309 | /// |
| 301 | /// Currently this adds executable bits for whoever has read bits already. It doesn't use the umask. | 310 | /// Currently this adds executable bits for whoever has read bits already. It doesn't use the umask. |