Take II: Use PYTHONEXECUTABLE not a custom rlocation

Author: arrdem

py/tools/py/src/activate.tmpl

@@ -1,3 +1,4 @@
+# -*- mode: sh -*-
 # Adapted from CPython Lib/venv/scripts/common/activate
 
 set -eu -o pipefail
@@ -58,9 +59,15 @@ deactivate nondestructive
 # The runfiles library code has some deps on this so we just set it :/
 : "${BASH_SOURCE:=$0}"
 
-VIRTUAL_ENV="$(realpath "$(dirname "$(dirname "${BASH_SOURCE}")")")"
+VIRTUAL_ENV="$(dirname "$(dirname "${BASH_SOURCE}")")"
 export VIRTUAL_ENV
 
+# HACK: (Ab)use the MacOS $PYTHONEXECUTABLE to record the `.runfiles`-relative
+# interpreter path. This helps us avoid issues with the interpreter's path being
+# `realpath`-ed in such a way that it escapes the `.runfiles` tree.
+PYTHONEXECUTABLE="${VIRTUAL_ENV}/bin/python"
+export PYTHONEXECUTABLE
+
 # unset PYTHONHOME if set
 # this will fail if PYTHONHOME is set to the empty string (which is bad anyway)
 # could use `if (set -u; : $PYTHONHOME) ;` in bash.

py/tools/py/src/runfiles_interpreter.tmpl

@@ -1,3 +1,4 @@
+# -*- mode: sh -*-
 # --- Runfiles-based interpreter setup --
 
 # If the runfiles dir is unset AND we will fail to find a runfiles manifest

py/tools/py/src/venv.rs

@@ -380,13 +380,6 @@ pub fn create_empty_venv<'a>(
     )
     .into_diagnostic()?;
 
-    // And include our horrid path machinery
-    fs::write(
-        &venv.site_dir.join("_aspect.py"),
-        include_str!("_aspect.py"),
-    )
-    .into_diagnostic()?;
-
     Ok(venv)
 }
 
@@ -448,11 +441,10 @@ pub fn populate_venv_with_copies(
     let action_bin_dir = action_src_dir.join(bin_dir);
 
     #[cfg(feature = "debug")]
-    {
-        println!("action_src_dir: {}", &action_src_dir.to_str().unwrap());
-        println!("action_bin_dir: {}", &action_bin_dir.to_str().unwrap());
-        println!("venv.site_dir: {}", &venv.site_dir.to_str().unwrap());
-    }
+    eprintln!("action_src_dir: {}", &action_src_dir.to_str().unwrap());
+
+    #[cfg(feature = "debug")]
+    eprintln!("action_bin_dir: {}", &action_bin_dir.to_str().unwrap());
 
     let source_pth = File::open(pth_file.src.as_path())
         .into_diagnostic()
@@ -467,7 +459,7 @@ pub fn populate_venv_with_copies(
         .write(
             b"\
 # Generated by Aspect py_binary
-# Sets up imports within the .runfiles tree
+# Contains relative import paths to non site-package trees within the .runfiles
 ",
         )
         .into_diagnostic()?;
@@ -540,16 +532,14 @@ pub fn populate_venv_with_copies(
             //
             // [1] https://github.com/python/cpython/blob/ce31ae5209c976d28d1c21fcbb06c0ae5e50a896/Lib/site.py#L215
 
+            let path_to_runfiles =
+                diff_paths(&action_bin_dir, action_bin_dir.join(&venv.site_dir)).unwrap();
+
             writeln!(dest_pth_writer, "# @{}", line).into_diagnostic()?;
             writeln!(
                 dest_pth_writer,
-                "import _aspect;_aspect.pathadd('{}', workspace='{}')",
-                line.replace("'", "\\'"),
-                if workspace == main_module {
-                    &""
-                } else {
-                    main_module.to_str().unwrap()
-                },
+                "{}",
+                path_to_runfiles.join(entry).to_str().unwrap()
             )
             .into_diagnostic()?;
         }

py/tools/venv_shim/src/main.rs

@@ -1,5 +1,6 @@
 use miette::{miette, Context, IntoDiagnostic};
 use std::env;
+use std::env::VarError;
 use std::fs;
 use std::io::{self, BufRead};
 use std::os::unix::process::CommandExt;
@@ -64,7 +65,6 @@ fn find_python_executables(version_from_cfg: &str, exclude_dir: &Path) -> Option
                 None
             }
         })
-        //.filter_map(|path| path.canonicalize().ok())
         .filter(|potential_executable| potential_executable.parent() != Some(exclude_dir))
         .filter(|potential_executable| compare_versions(version_from_cfg, &potential_executable))
         .collect();
@@ -77,16 +77,32 @@ fn find_python_executables(version_from_cfg: &str, exclude_dir: &Path) -> Option
 }
 
 fn main() -> miette::Result<()> {
-    let current_exe = env::current_exe().unwrap();
+    let venv_home_path = PathBuf::from(
+        env::var("VIRTUAL_ENV")
+            .into_diagnostic()
+            .wrap_err("[aspect] $VIRTUAL_ENV was unbound! A venv must be activated.")
+            .unwrap(),
+    );
+
+    let venv_interpreter_path: PathBuf = env::var("PYTHONEXECUTABLE")
+        .map(|x| PathBuf::from(x))
+        .map_err(|_| Ok::<PathBuf, VarError>(venv_home_path.join("bin/python")))
+        .unwrap();
+
+    let excluded_interpreters_dir = &venv_home_path.join("bin");
+
     let args: Vec<_> = env::args().collect();
 
-    #[cfg(feature = "debug")]
-    eprintln!("[aspect] Current executable path: {:?}", current_exe);
+    //#[cfg(feature = "debug")]
+    eprintln!(
+        "[aspect] Current executable path: {:?}",
+        &venv_interpreter_path
+    );
 
-    let Some(pyvenv_cfg_path) = find_pyvenv_cfg(&current_exe) else {
+    let Some(pyvenv_cfg_path) = find_pyvenv_cfg(&venv_interpreter_path) else {
         return Err(miette!("pyvenv.cfg not found one directory level up."));
     };
-    #[cfg(feature = "debug")]
+    //#[cfg(feature = "debug")]
     eprintln!("[aspect] Found pyvenv.cfg at: {:?}", &pyvenv_cfg_path);
 
     let version_info_result = extract_pyvenv_version_info(&pyvenv_cfg_path)
@@ -101,33 +117,32 @@ fn main() -> miette::Result<()> {
         return Err(miette!("version_info key not found in pyvenv.cfg."));
     };
 
-    #[cfg(feature = "debug")]
-    eprintln!("[aspect] version_info from pyvenv.cfg: {}", &version_info);
+    //#[cfg(feature = "debug")]
+    eprintln!("[aspect] version_info from pyvenv.cfg: {:?}", &version_info);
 
     let Some(target_python_version) = parse_version_info(&version_info) else {
         return Err(miette!("Could not parse version_info as x.y."));
     };
 
-    #[cfg(feature = "debug")]
+    //#[cfg(feature = "debug")]
     eprintln!(
         "[aspect] Parsed target Python version (major.minor): {}",
         &target_python_version
     );
 
-    let exclude_dir = current_exe.parent().unwrap().canonicalize().unwrap();
+    //#[cfg(feature = "debug")]
+    eprintln!("[aspect] Ignoring dir {:?}", &excluded_interpreters_dir);
 
-    #[cfg(feature = "debug")]
-    eprintln!("[aspect] Ignoring dir {:?}", &exclude_dir);
-
-    let Some(python_executables) = find_python_executables(&target_python_version, &exclude_dir)
+    let Some(python_executables) =
+        find_python_executables(&target_python_version, &excluded_interpreters_dir)
     else {
         return Err(miette!(
             "No suitable Python interpreter found in PATH matching version '{}'.",
             &version_info,
         ));
     };
 
-    #[cfg(feature = "debug")]
+    //#[cfg(feature = "debug")]
     {
         eprintln!("[aspect] Found potential Python interpreters in PATH with matching version:");
         for exe in &python_executables {
@@ -148,30 +163,27 @@ fn main() -> miette::Result<()> {
         }
     };
 
-    let Some(interpreter_path) = python_executables.get(index) else {
+    let Some(actual_interpreter_path) = python_executables.get(index) else {
         return Err(miette!(
             "Unable to find another interpreter at index {}",
             index
         ));
     };
 
-    let exe_path = current_exe.to_string_lossy().into_owned();
     let exec_args = &args[1..];
 
-    #[cfg(feature = "debug")]
+    //#[cfg(feature = "debug")]
     eprintln!(
         "[aspect] Attempting to execute: {:?} with argv[0] as {:?} and args as {:?}",
-        interpreter_path, exe_path, exec_args,
+        &actual_interpreter_path, &venv_interpreter_path, exec_args,
     );
 
-    let mut cmd = Command::new(&interpreter_path);
+    let mut cmd = Command::new(&actual_interpreter_path);
     cmd.args(exec_args);
 
     // Lie about the value of argv0 to hoodwink the interpreter as to its
     // location on Linux-based platforms.
-    if cfg!(target_os = "linux") {
-        cmd.arg0(&exe_path);
-    }
+    cmd.arg0(&venv_interpreter_path);
 
     // On MacOS however, there are facilities for asking the C runtime/OS
     // what the real name of the interpreter executable is, and that value
@@ -181,7 +193,7 @@ fn main() -> miette::Result<()> {
     // https://github.com/python/cpython/blob/68e72cf3a80362d0a2d57ff0c9f02553c378e537/Modules/getpath.c#L778
     // https://docs.python.org/3/using/cmdline.html#envvar-PYTHONEXECUTABLE
     if cfg!(target_os = "macos") {
-        cmd.env("PYTHONEXECUTABLE", &exe_path);
+        cmd.env("PYTHONEXECUTABLE", &venv_interpreter_path);
     }
 
     // Re-export the counter so it'll go up