Skip to content

Commit

Permalink
Write relative paths for scripts in data directory (#2348)
Browse files Browse the repository at this point in the history
## Summary

In #2000, I shipped a regression whereby we stopped writing relative
paths for scripts within `data` directories. The net effect here is that
we aren't _uninstalling_ binaries in all cases. (This does _not_ apply
to entrypoints, only scripts in `data` directories.)

Closes #2330.

## Test Plan

Most Python packages ship entrypoints, not binaries, so I don't know how
to test this cheaply. But I did test it locally by verifying that `uv`
is now removed from the `bin` directory after an uninstall.
  • Loading branch information
charliermarsh authored Mar 10, 2024
1 parent ec83151 commit 9566ac9
Showing 1 changed file with 18 additions and 5 deletions.
23 changes: 18 additions & 5 deletions crates/install-wheel-rs/src/wheel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,17 @@ fn install_script(
)));
}

let target_path = layout.scheme.scripts.join(file.file_name());
let script_absolute = layout.scheme.scripts.join(file.file_name());
let script_relative =
pathdiff::diff_paths(&script_absolute, site_packages).ok_or_else(|| {
Error::Io(io::Error::new(
io::ErrorKind::Other,
format!(
"Could not find relative path for: {}",
script_absolute.simplified_display()
),
))
})?;

let path = file.path();
let mut script = File::open(&path)?;
Expand All @@ -461,24 +471,25 @@ fn install_script(
let start = format_shebang(&layout.sys_executable, &layout.os_name)
.as_bytes()
.to_vec();
let mut target = File::create(&target_path)?;
let mut target = File::create(&script_absolute)?;
let size_and_encoded_hash = copy_and_hash(&mut start.chain(script), &mut target)?;
fs::remove_file(&path)?;
Some(size_and_encoded_hash)
} else {
// reading and writing is slow especially for large binaries, so we move them instead
drop(script);
fs::rename(&path, &target_path)?;
fs::rename(&path, &script_absolute)?;
None
};
#[cfg(unix)]
{
use std::fs::Permissions;
use std::os::unix::fs::PermissionsExt;

fs::set_permissions(&target_path, Permissions::from_mode(0o755))?;
fs::set_permissions(&script_absolute, Permissions::from_mode(0o755))?;
}

// Find the existing entry in the `RECORD`.
let relative_to_site_packages = path
.strip_prefix(site_packages)
.expect("Prefix must no change");
Expand All @@ -493,7 +504,9 @@ fn install_script(
path.simplified_display()
))
})?;
entry.path = target_path.display().to_string();

// Update the entry in the `RECORD`.
entry.path = script_relative.simplified_display().to_string();
if let Some((size, encoded_hash)) = size_and_encoded_hash {
entry.size = Some(size);
entry.hash = Some(encoded_hash);
Expand Down

0 comments on commit 9566ac9

Please sign in to comment.