diff --git a/crates/gourgeist/src/packages.rs b/crates/gourgeist/src/packages.rs index de2622582..1410a69dc 100644 --- a/crates/gourgeist/src/packages.rs +++ b/crates/gourgeist/src/packages.rs @@ -51,10 +51,7 @@ pub(crate) fn install_base_packages( info: &InterpreterInfo, paths: &VenvPaths, ) -> Result<(), Error> { - let install_location = InstallLocation::Venv { - venv_base: location.canonicalize()?, - python_version: (info.major, info.minor), - }; + let install_location = InstallLocation::new(location.canonicalize()?, (info.major, info.minor)); let install_location = install_location.acquire_lock()?; // TODO: Use the json api instead @@ -79,8 +76,6 @@ pub(crate) fn install_base_packages( false, false, &[], - // Only relevant for monotrail style installation - "", paths.interpreter.as_std_path(), ) .map_err(|err| Error::InstallWheel { diff --git a/crates/install-wheel-rs/src/install_location.rs b/crates/install-wheel-rs/src/install_location.rs index 9da800f9e..429cd7ae5 100644 --- a/crates/install-wheel-rs/src/install_location.rs +++ b/crates/install-wheel-rs/src/install_location.rs @@ -1,11 +1,8 @@ -//! Multiplexing between venv install and monotrail install +use std::io; +use std::path::{Path, PathBuf}; use fs2::FileExt; -use fs_err as fs; use fs_err::File; -use std::io; -use std::ops::Deref; -use std::path::{Path, PathBuf}; use tracing::{error, warn}; const INSTALL_LOCKFILE: &str = "install-wheel-rs.lock"; @@ -65,124 +62,65 @@ impl Drop for LockedDir { } } -impl Deref for LockedDir { - type Target = Path; - - fn deref(&self) -> &Self::Target { +impl AsRef for LockedDir { + fn as_ref(&self) -> &Path { &self.path } } -/// Multiplexing between venv install and monotrail install -/// -/// For monotrail, we have a structure that is {monotrail}/{normalized(name)}/{version}/tag +/// A virtual environment into which a wheel can be installed. /// /// We use a lockfile to prevent multiple instance writing stuff on the same time /// As of pip 22.0, e.g. `pip install numpy; pip install numpy; pip install numpy` will -/// nondeterministically fail -/// -/// I was also thinking about making a shared lock on the import side, but monotrail install -/// is supposedly atomic (by directory renaming), while for venv installation there can't be -/// atomicity (we need to add lots of different file without a top level directory / key-turn -/// file we could rename) and the locking would also need to happen in the import mechanism -/// itself to ensure -pub enum InstallLocation> { - Venv { - /// absolute path - venv_base: T, - python_version: (u8, u8), - }, - Monotrail { - monotrail_root: T, - python: PathBuf, - python_version: (u8, u8), - }, +/// non-deterministically fail. +pub struct InstallLocation> { + /// absolute path + venv_base: T, + python_version: (u8, u8), } -impl> InstallLocation { - /// Returns the location of the python interpreter - pub fn get_python(&self) -> PathBuf { - match self { - InstallLocation::Venv { venv_base, .. } => { - if cfg!(windows) { - venv_base.join("Scripts").join("python.exe") - } else { - // canonicalize on python would resolve the symlink - venv_base.join("bin").join("python") - } - } - // TODO: For monotrail use the monotrail launcher - InstallLocation::Monotrail { python, .. } => python.clone(), +impl> InstallLocation { + pub fn new(venv_base: T, python_version: (u8, u8)) -> Self { + Self { + venv_base, + python_version, } } - pub fn get_python_version(&self) -> (u8, u8) { - match self { - InstallLocation::Venv { python_version, .. } => *python_version, - InstallLocation::Monotrail { python_version, .. } => *python_version, + /// Returns the location of the `python` interpreter. + pub fn python(&self) -> PathBuf { + if cfg!(windows) { + self.venv_base.as_ref().join("Scripts").join("python.exe") + } else { + // canonicalize on python would resolve the symlink + self.venv_base.as_ref().join("bin").join("python") } } - /// TODO: This function is unused? - pub fn is_installed(&self, normalized_name: &str, version: &str) -> bool { - match self { - InstallLocation::Venv { - venv_base, - python_version, - } => { - let site_packages = if cfg!(target_os = "windows") { - venv_base.join("Lib").join("site-packages") - } else { - venv_base - .join("lib") - .join(format!("python{}.{}", python_version.0, python_version.1)) - .join("site-packages") - }; - site_packages - .join(format!("{normalized_name}-{version}.dist-info")) - .is_dir() - } - InstallLocation::Monotrail { monotrail_root, .. } => monotrail_root - .join(format!("{normalized_name}-{version}")) - .is_dir(), - } + pub fn python_version(&self) -> (u8, u8) { + self.python_version + } + + pub fn venv_base(&self) -> &T { + &self.venv_base } } impl InstallLocation { pub fn acquire_lock(&self) -> io::Result> { - let root = match self { - Self::Venv { venv_base, .. } => venv_base, - Self::Monotrail { monotrail_root, .. } => monotrail_root, - }; - - // If necessary, create monotrail dir - fs::create_dir_all(root)?; - - let locked_dir = if let Some(locked_dir) = LockedDir::try_acquire(root)? { + let locked_dir = if let Some(locked_dir) = LockedDir::try_acquire(&self.venv_base)? { locked_dir } else { warn!( "Could not acquire exclusive lock for installing, is another installation process \ running? Sleeping until lock becomes free" ); - LockedDir::acquire(root)? + LockedDir::acquire(&self.venv_base)? }; - Ok(match self { - Self::Venv { python_version, .. } => InstallLocation::Venv { - venv_base: locked_dir, - python_version: *python_version, - }, - Self::Monotrail { - python_version, - python, - .. - } => InstallLocation::Monotrail { - monotrail_root: locked_dir, - python: python.clone(), - python_version: *python_version, - }, + Ok(InstallLocation { + venv_base: locked_dir, + python_version: self.python_version, }) } } diff --git a/crates/install-wheel-rs/src/lib.rs b/crates/install-wheel-rs/src/lib.rs index 9685ec828..65428e3b6 100644 --- a/crates/install-wheel-rs/src/lib.rs +++ b/crates/install-wheel-rs/src/lib.rs @@ -1,4 +1,4 @@ -//! Takes a wheel and installs it, either in a venv or for monotrail. +//! Takes a wheel and installs it into a venv.. use std::io; use std::io::{Read, Seek}; diff --git a/crates/install-wheel-rs/src/main.rs b/crates/install-wheel-rs/src/main.rs index 427328bf1..c70f03b8b 100644 --- a/crates/install-wheel-rs/src/main.rs +++ b/crates/install-wheel-rs/src/main.rs @@ -31,10 +31,7 @@ struct Args { fn main() -> Result<(), Error> { let args = Args::parse(); let venv_base = args.venv.canonicalize()?; - let location = InstallLocation::Venv { - venv_base, - python_version: (args.major, args.minor), - }; + let location = InstallLocation::new(venv_base, (args.major, args.minor)); let locked_dir = location.acquire_lock()?; let wheels: Vec<(PathBuf, WheelFilename)> = args @@ -69,9 +66,7 @@ fn main() -> Result<(), Error> { args.compile, !args.skip_hashes, &[], - // Only relevant for monotrail style installation - "", - location.get_python(), + location.python(), )?; Ok(()) }) diff --git a/crates/install-wheel-rs/src/python_bindings.rs b/crates/install-wheel-rs/src/python_bindings.rs index 50384e7fd..ebf11bb76 100644 --- a/crates/install-wheel-rs/src/python_bindings.rs +++ b/crates/install-wheel-rs/src/python_bindings.rs @@ -40,10 +40,10 @@ impl LockedVenv { #[allow(clippy::needless_pass_by_value)] pub(crate) fn new(py: Python, venv: PathBuf) -> PyResult { Ok(Self { - location: InstallLocation::Venv { - venv_base: LockedDir::acquire(&venv)?, - python_version: (py.version_info().major, py.version_info().minor), - }, + location: InstallLocation::new( + LockedDir::acquire(&venv)?, + (py.version_info().major, py.version_info().minor), + ), }) } @@ -65,8 +65,6 @@ impl LockedVenv { true, true, &[], - // unique_version can be anything since it's only used to monotrail - "", Path::new(&sys_executable), ) })?; diff --git a/crates/install-wheel-rs/src/unpacked.rs b/crates/install-wheel-rs/src/unpacked.rs index 87f4a82c9..380566ce0 100644 --- a/crates/install-wheel-rs/src/unpacked.rs +++ b/crates/install-wheel-rs/src/unpacked.rs @@ -34,26 +34,19 @@ pub fn install_wheel( let name = &filename.distribution; let _my_span = span!(Level::DEBUG, "install_wheel", name = name.as_str()); - let InstallLocation::Venv { - venv_base: base_location, - .. - } = location - else { - return Err(Error::InvalidWheel( - "Monotrail installation is not supported yet".to_string(), - )); - }; + let base_location = location.venv_base(); // TODO(charlie): Pass this in. let site_packages_python = format!( "python{}.{}", - location.get_python_version().0, - location.get_python_version().1 + location.python_version().0, + location.python_version().1 ); let site_packages = if cfg!(target_os = "windows") { - base_location.join("Lib").join("site-packages") + base_location.as_ref().join("Lib").join("site-packages") } else { base_location + .as_ref() .join("lib") .join(site_packages_python) .join("site-packages") @@ -94,7 +87,7 @@ pub fn install_wheel( if data_dir.is_dir() { debug!(name = name.as_str(), "Installing data"); install_data( - base_location, + base_location.as_ref(), &site_packages, &data_dir, &name, diff --git a/crates/install-wheel-rs/src/wheel.rs b/crates/install-wheel-rs/src/wheel.rs index 8dafdf889..4ec0030ee 100644 --- a/crates/install-wheel-rs/src/wheel.rs +++ b/crates/install-wheel-rs/src/wheel.rs @@ -11,7 +11,7 @@ use fs_err as fs; use fs_err::{DirEntry, File}; use mailparse::MailHeaderMap; use sha2::{Digest, Sha256}; -use tempfile::{tempdir, TempDir}; +use tempfile::tempdir; use tracing::{debug, error, span, warn, Level}; use walkdir::WalkDir; use zip::result::ZipError; @@ -23,7 +23,7 @@ use wheel_filename::WheelFilename; use crate::install_location::{InstallLocation, LockedDir}; use crate::record::RecordEntry; use crate::script::Script; -use crate::{normalize_name, Error}; +use crate::Error; /// `#!/usr/bin/env python` pub const SHEBANG_PYTHON: &str = "#!/usr/bin/env python"; @@ -262,25 +262,19 @@ fn unpack_wheel_files( } pub(crate) fn get_shebang(location: &InstallLocation) -> String { - if matches!(location, InstallLocation::Venv { .. }) { - let path = location.get_python().display().to_string(); - let path = if cfg!(windows) { - // https://stackoverflow.com/a/50323079 - const VERBATIM_PREFIX: &str = r"\\?\"; - if let Some(stripped) = path.strip_prefix(VERBATIM_PREFIX) { - stripped.to_string() - } else { - path - } + let path = location.python().display().to_string(); + let path = if cfg!(windows) { + // https://stackoverflow.com/a/50323079 + const VERBATIM_PREFIX: &str = r"\\?\"; + if let Some(stripped) = path.strip_prefix(VERBATIM_PREFIX) { + stripped.to_string() } else { path - }; - format!("#!{path}") + } } else { - // This will use the monotrail binary moonlighting as python. `python` alone doesn't, - // we need env to find the python link we put in PATH - SHEBANG_PYTHON.to_string() - } + path + }; + format!("#!{path}") } /// To get a launcher on windows we write a minimal .exe launcher binary and then attach the actual @@ -336,8 +330,6 @@ pub(crate) fn write_script_entrypoints( entrypoints: &[Script], record: &mut Vec, ) -> Result<(), Error> { - // for monotrail - fs::create_dir_all(site_packages.join(&bin_rel()))?; for entrypoint in entrypoints { let entrypoint_relative = if cfg!(windows) { // On windows we actually build an .exe wrapper @@ -677,9 +669,6 @@ fn install_script( // // > The b'#!pythonw' convention is allowed. b'#!pythonw' indicates a GUI script // > instead of a console script. - // - // We do this in venvs as required, but in monotrail mode we use a fake shebang - // (#!/usr/bin/env python) for injection monotrail as python into PATH later let placeholder_python = b"#!python"; // scripts might be binaries, so we read an exact number of bytes instead of the first line as string let mut start = Vec::new(); @@ -776,11 +765,10 @@ pub(crate) fn install_data( let target_path = venv_base .join("include") .join("site") - // TODO: Also use just python here in monotrail .join(format!( "python{}.{}", - location.get_python_version().0, - location.get_python_version().1 + location.python_version().0, + location.python_version().1 )) .join(dist_name); move_folder_recorded(&data_entry.path(), &target_path, site_packages, record)?; @@ -908,51 +896,23 @@ pub fn install_wheel( // initially used to the console scripts, currently unused. Keeping it because we likely need // it for validation later _extras: &[String], - unique_version: &str, sys_executable: impl AsRef, ) -> Result { let name = &filename.distribution; let _my_span = span!(Level::DEBUG, "install_wheel", name = name.as_str()); - let (temp_dir_final_location, base_location) = match location { - InstallLocation::Venv { venv_base, .. } => (None, venv_base.to_path_buf()), - InstallLocation::Monotrail { monotrail_root, .. } => { - let name_version_dir = monotrail_root - .join(normalize_name(name)) - .join(unique_version); - fs::create_dir_all(&name_version_dir)?; - let final_location = name_version_dir.join(filename.get_tag()); - // temp dir and rename for atomicity - // well, except for windows, because there renaming fails for undeterminable reasons - // with an os error 5 permission denied. - if cfg!(not(windows)) { - let temp_dir = TempDir::new_in(&name_version_dir)?; - let base_location = temp_dir.path().to_path_buf(); - (Some((temp_dir, final_location)), base_location) - } else { - fs::create_dir(&final_location)?; - (None, final_location) - } - } - }; + let base_location = location.venv_base(); - let site_packages_python = match location { - InstallLocation::Venv { .. } => { - format!( - "python{}.{}", - location.get_python_version().0, - location.get_python_version().1 - ) - } - // Monotrail installation is for multiple python versions (depending on the wheel tag) - // Potentially needs to be changed to creating pythonx.y symlinks for each python version - // we use it with (on install in that python version) - InstallLocation::Monotrail { .. } => "python".to_string(), - }; + let site_packages_python = format!( + "python{}.{}", + location.python_version().0, + location.python_version().1 + ); let site_packages = if cfg!(target_os = "windows") { - base_location.join("Lib").join("site-packages") + base_location.as_ref().join("Lib").join("site-packages") } else { base_location + .as_ref() .join("lib") .join(site_packages_python) .join("site-packages") @@ -1014,7 +974,7 @@ pub fn install_wheel( if data_dir.is_dir() { debug!(name = name.as_str(), "Installing data"); install_data( - &base_location, + base_location.as_ref(), &site_packages, &data_dir, &name, @@ -1022,8 +982,6 @@ pub fn install_wheel( &console_scripts, &gui_scripts, &mut record, - // For the monotrail install, we want to keep the fake shebang for our own - // later replacement logic )?; // 2.c If applicable, update scripts starting with #!python to point to the correct interpreter. // Script are unsupported through data @@ -1039,7 +997,7 @@ pub fn install_wheel( bytecode_compile( &site_packages, unpacked_paths, - location.get_python_version(), + location.python_version(), sys_executable.as_ref(), name.as_str(), &mut record, @@ -1060,12 +1018,6 @@ pub fn install_wheel( record_writer.serialize(entry)?; } - // rename for atomicity - // well, except for windows, see comment above - if let Some((_temp_dir, final_location)) = temp_dir_final_location { - fs::rename(base_location, final_location)?; - } - Ok(filename.get_tag()) } @@ -1091,7 +1043,7 @@ fn find_dist_info( [] => { return Err(Error::InvalidWheel( "Missing .dist-info directory".to_string(), - )) + )); } [dist_info] => (*dist_info).to_string(), _ => { @@ -1219,7 +1171,7 @@ mod test { assert_eq!( relative_to( Path::new("/home/ferris/carcinization/lib/python/site-packages/foo/__init__.py"), - Path::new("/home/ferris/carcinization/lib/python/site-packages") + Path::new("/home/ferris/carcinization/lib/python/site-packages"), ) .unwrap(), Path::new("foo/__init__.py") @@ -1227,7 +1179,7 @@ mod test { assert_eq!( relative_to( Path::new("/home/ferris/carcinization/lib/marker.txt"), - Path::new("/home/ferris/carcinization/lib/python/site-packages") + Path::new("/home/ferris/carcinization/lib/python/site-packages"), ) .unwrap(), Path::new("../../marker.txt") @@ -1235,7 +1187,7 @@ mod test { assert_eq!( relative_to( Path::new("/home/ferris/carcinization/bin/foo_launcher"), - Path::new("/home/ferris/carcinization/lib/python/site-packages") + Path::new("/home/ferris/carcinization/lib/python/site-packages"), ) .unwrap(), Path::new("../../../bin/foo_launcher") @@ -1256,7 +1208,7 @@ mod test { Script::from_value( "launcher", "foo.bar:main", - Some(&["bar".to_string(), "baz".to_string()]) + Some(&["bar".to_string(), "baz".to_string()]), ) .unwrap(), Some(Script { @@ -1273,7 +1225,7 @@ mod test { Script::from_value( "launcher", "foomod:main_bar [bar,baz]", - Some(&["bar".to_string(), "baz".to_string()]) + Some(&["bar".to_string(), "baz".to_string()]), ) .unwrap(), Some(Script { diff --git a/crates/puffin-installer/src/lib.rs b/crates/puffin-installer/src/lib.rs index bcae42b8f..30c4c24e2 100644 --- a/crates/puffin-installer/src/lib.rs +++ b/crates/puffin-installer/src/lib.rs @@ -114,10 +114,7 @@ pub async fn install( ); // Phase 3: Install each wheel. - let location = InstallLocation::Venv { - venv_base: python.venv().to_path_buf(), - python_version: python.simple_version(), - }; + let location = InstallLocation::new(python.venv().to_path_buf(), python.simple_version()); let locked_dir = location.acquire_lock()?; for wheel in wheels {