Write relative paths for scripts in data directory (#2348)

## 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 https://github.com/astral-sh/uv/issues/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.
This commit is contained in:
Charlie Marsh 2024-03-10 16:02:02 -07:00 committed by GitHub
parent ec83151666
commit 9566ac9b85
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 18 additions and 5 deletions

View File

@ -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 path = file.path();
let mut script = File::open(&path)?; let mut script = File::open(&path)?;
@ -461,14 +471,14 @@ fn install_script(
let start = format_shebang(&layout.sys_executable, &layout.os_name) let start = format_shebang(&layout.sys_executable, &layout.os_name)
.as_bytes() .as_bytes()
.to_vec(); .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)?; let size_and_encoded_hash = copy_and_hash(&mut start.chain(script), &mut target)?;
fs::remove_file(&path)?; fs::remove_file(&path)?;
Some(size_and_encoded_hash) Some(size_and_encoded_hash)
} else { } else {
// reading and writing is slow especially for large binaries, so we move them instead // reading and writing is slow especially for large binaries, so we move them instead
drop(script); drop(script);
fs::rename(&path, &target_path)?; fs::rename(&path, &script_absolute)?;
None None
}; };
#[cfg(unix)] #[cfg(unix)]
@ -476,9 +486,10 @@ fn install_script(
use std::fs::Permissions; use std::fs::Permissions;
use std::os::unix::fs::PermissionsExt; 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 let relative_to_site_packages = path
.strip_prefix(site_packages) .strip_prefix(site_packages)
.expect("Prefix must no change"); .expect("Prefix must no change");
@ -493,7 +504,9 @@ fn install_script(
path.simplified_display() 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 { if let Some((size, encoded_hash)) = size_and_encoded_hash {
entry.size = Some(size); entry.size = Some(size);
entry.hash = Some(encoded_hash); entry.hash = Some(encoded_hash);