In `finalize_entry` and its callers, `set_executable_after_creation` was previously an `Option<&Path>`, because `chmod` was called (via a higher level abstraction) on the path to make the file executable. From #1764, `lstat` (via a higher level abstraction) was likewise called on the path to find out what its old mode was, to figure out what mode to set to achieve +x. Now that the `lstat` is replaced with `fstat`, and the `chmod` is replaced with `fchmod`, figuring out how to set +x and setting it are done entirely on the file descriptor, with the path unused. It turns out that this was the only use of that path in that context. Accordingly, this makes `set_executable_after_creation` a `bool` in both `finalize_entry` and its callers, and removes code that was only used for handling that path. This includes eliminating the `rela_path_as_path` variable in `checkout::chunk::process_delayed_filter_results`, which was apparently only used for this purpose. (All writes and reads on it were for preparing and passing a path to `finalize_entry`. The `finalize_entry` function had only used it to set the file executable, and had documented that this was its purpose.)
Unified · Split
| @@ -177,7 +177,6 @@ where · 176 unmodified lines | |||
| 177 | // We process each key and do as the filter process tells us, while collecting data about the overall progress. | 177 | // We process each key and do as the filter process tells us, while collecting data about the overall progress. |
| 178 | let keys: BTreeSet<_> = delayed_filter_results.iter().map(|d| d.key.clone()).collect(); | 178 | let keys: BTreeSet<_> = delayed_filter_results.iter().map(|d| d.key.clone()).collect(); |
| 179 | let mut unknown_paths = Vec::new(); | 179 | let mut unknown_paths = Vec::new(); |
| 180 | let mut rela_path_as_path = Default::default(); | ||
| 181 | for key in keys { | 180 | for key in keys { |
| 182 | loop { | 181 | loop { |
| 183 | let rela_paths = ctx.filters.driver_state_mut().list_delayed_paths(&key)?; | 182 | let rela_paths = ctx.filters.driver_state_mut().list_delayed_paths(&key)?; |
| @@ -229,10 +228,7 @@ where · 45 unmodified lines | |||
| 229 | entry::finalize_entry( | 228 | entry::finalize_entry( |
| 230 | delayed.entry, | 229 | delayed.entry, |
| 231 | write.inner.into_inner().map_err(std::io::IntoInnerError::into_error)?, | 230 | write.inner.into_inner().map_err(std::io::IntoInnerError::into_error)?, |
| 232 | set_executable_after_creation.then(|| { | 231 | set_executable_after_creation, |
| 233 | rela_path_as_path = gix_path::from_bstr(delayed.entry_path); | ||
| 234 | rela_path_as_path.as_ref() | ||
| 235 | }), | ||
| 236 | )?; | 232 | )?; |
| 237 | delayed_files += 1; | 233 | delayed_files += 1; |
| 238 | files.fetch_add(1, Ordering::Relaxed); | 234 | files.fetch_add(1, Ordering::Relaxed); |
| @@ -135,7 +135,7 @@ where · 134 unmodified lines | |||
| 135 | }; | 135 | }; |
| 136 | 136 | ||
| 137 | // For possibly existing, overwritten files, we must change the file mode explicitly. | 137 | // For possibly existing, overwritten files, we must change the file mode explicitly. |
| 138 | finalize_entry(entry, file, set_executable_after_creation.then_some(dest))?; | 138 | finalize_entry(entry, file, set_executable_after_creation)?; |
| 139 | num_bytes | 139 | num_bytes |
| 140 | } | 140 | } |
| 141 | gix_index::entry::Mode::SYMLINK => { | 141 | gix_index::entry::Mode::SYMLINK => { |
| @@ -275,16 +275,15 @@ pub(crate) fn open_file( · 133 unmodified lines | |||
| 275 | try_op_or_unlink(path, overwrite_existing, |p| options.open(p)).map(|f| (f, set_executable_after_creation)) | 275 | try_op_or_unlink(path, overwrite_existing, |p| options.open(p)).map(|f| (f, set_executable_after_creation)) |
| 276 | } | 276 | } |
| 277 | 277 | ||
| 278 | /// Close `file` and store its stats in `entry`, possibly setting `file` executable depending on | 278 | /// Close `file` and store its stats in `entry`, possibly setting `file` executable. |
| 279 | /// `set_executable_after_creation`. | ||
| 280 | pub(crate) fn finalize_entry( | 279 | pub(crate) fn finalize_entry( |
| 281 | entry: &mut gix_index::Entry, | 280 | entry: &mut gix_index::Entry, |
| 282 | file: std::fs::File, | 281 | file: std::fs::File, |
| 283 | #[cfg_attr(windows, allow(unused_variables))] set_executable_after_creation: Option<&Path>, | 282 | #[cfg_attr(windows, allow(unused_variables))] set_executable_after_creation: bool, |
| 284 | ) -> Result<(), crate::checkout::Error> { | 283 | ) -> Result<(), crate::checkout::Error> { |
| 285 | // For possibly existing, overwritten files, we must change the file mode explicitly. | 284 | // For possibly existing, overwritten files, we must change the file mode explicitly. |
| 286 | #[cfg(unix)] | 285 | #[cfg(unix)] |
| 287 | if let Some(path) = set_executable_after_creation { | 286 | if set_executable_after_creation { |
| 288 | set_executable(&file)?; | 287 | set_executable(&file)?; |
| 289 | } | 288 | } |
| 290 | // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. | 289 | // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. |