Skip to content

Commit

Permalink
Make set_executable_after_creation everywhere bool
Browse files Browse the repository at this point in the history
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 GitoxideLabs#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.)
  • Loading branch information
EliahKagan committed Jan 24, 2025
1 parent ee685dc commit 22a6092
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 10 deletions.
6 changes: 1 addition & 5 deletions gix-worktree-state/src/checkout/chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ where
// We process each key and do as the filter process tells us, while collecting data about the overall progress.
let keys: BTreeSet<_> = delayed_filter_results.iter().map(|d| d.key.clone()).collect();
let mut unknown_paths = Vec::new();
let mut rela_path_as_path = Default::default();
for key in keys {
loop {
let rela_paths = ctx.filters.driver_state_mut().list_delayed_paths(&key)?;
Expand Down Expand Up @@ -229,10 +228,7 @@ where
entry::finalize_entry(
delayed.entry,
write.inner.into_inner().map_err(std::io::IntoInnerError::into_error)?,
set_executable_after_creation.then(|| {
rela_path_as_path = gix_path::from_bstr(delayed.entry_path);
rela_path_as_path.as_ref()
}),
set_executable_after_creation,
)?;
delayed_files += 1;
files.fetch_add(1, Ordering::Relaxed);
Expand Down
9 changes: 4 additions & 5 deletions gix-worktree-state/src/checkout/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ where
};

// For possibly existing, overwritten files, we must change the file mode explicitly.
finalize_entry(entry, file, set_executable_after_creation.then_some(dest))?;
finalize_entry(entry, file, set_executable_after_creation)?;
num_bytes
}
gix_index::entry::Mode::SYMLINK => {
Expand Down Expand Up @@ -275,16 +275,15 @@ pub(crate) fn open_file(
try_op_or_unlink(path, overwrite_existing, |p| options.open(p)).map(|f| (f, set_executable_after_creation))
}

/// Close `file` and store its stats in `entry`, possibly setting `file` executable depending on
/// `set_executable_after_creation`.
/// Close `file` and store its stats in `entry`, possibly setting `file` executable.
pub(crate) fn finalize_entry(
entry: &mut gix_index::Entry,
file: std::fs::File,
#[cfg_attr(windows, allow(unused_variables))] set_executable_after_creation: Option<&Path>,
#[cfg_attr(windows, allow(unused_variables))] set_executable_after_creation: bool,
) -> Result<(), crate::checkout::Error> {
// For possibly existing, overwritten files, we must change the file mode explicitly.
#[cfg(unix)]
if let Some(path) = set_executable_after_creation {
if set_executable_after_creation {
set_executable(&file)?;
}
// NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well.
Expand Down

0 comments on commit 22a6092

Please sign in to comment.