Remove Monotrail-specific code from install-wheel-rs (#68)

I think this isn't necessary to support in this generic crate. If we
choose to adopt Monotrail-style concepts, we'll likely need to rework
them anyway.

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 {

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<Path> 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<T: Deref<Target = Path>> {
-    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<T: AsRef<Path>> {
+    /// absolute path
+    venv_base: T,
+    python_version: (u8, u8),
 }
 
-impl<T: Deref<Target = Path>> InstallLocation<T> {
-    /// 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<T: AsRef<Path>> InstallLocation<T> {
+    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<PathBuf> {
     pub fn acquire_lock(&self) -> io::Result<InstallLocation<LockedDir>> {
-        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,
         })
     }
 }

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};

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(())
         })

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<Self> {
         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),
             )
         })?;

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,

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<R: Read + Seek>(
 }
 
 pub(crate) fn get_shebang(location: &InstallLocation<LockedDir>) -> 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<RecordEntry>,
 ) -> 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<Path>,
 ) -> Result<String, Error> {
     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 {

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 {