libservo: Improve finding python (#33385)

* libservo: Improve finding python

Servo is built in a virtual environment, which sets `VIRTUAL_ENV` to
the base path of the venv.
PYTHON3 is only set if mach or the user set it.

Additional changes:

- Use Path / var_os for Paths instead of strings.
 In General using Path APIs is preferable, since in rare cases
 valid paths may not be valid utf-8.
- Don't search for python 3.8 anymore, since
  we require a newer version anyway.
- Don't add the .exe suffix anymore, since
  Command::new() will take care of that.

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
Signed-off-by: Jonathan Schwender <jonathan.schwender@huawei.com>

* script: Improve finding python

Synchronize `find_python` in scripts build.rs with the version in
libservo.

Signed-off-by: Jonathan Schwender <jonathan.schwender@huawei.com>

* Apply suggestions from code review

Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Signed-off-by: Jonathan Schwender <55576758+jschwe@users.noreply.github.com>
Signed-off-by: Jonathan Schwender <jonathan.schwender@huawei.com>

* Fix finding venv python on windows

- On Windows the venv scripts and python binaries are in the
  `Scripts` subdirectory instead of `bin`.
- We shouldn't check if the executable in the venv binary dir
  exists, since the actual file could have a `.exe` suffix.
  We don't need to consider the `.exe` suffix for the filename,
  since `Command` will handle that for us.
  We also anyway check the validity of the candidate file, by
  running `$candidate --version`.

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>

---------

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
Signed-off-by: Jonathan Schwender <jonathan.schwender@huawei.com>
Signed-off-by: Jonathan Schwender <55576758+jschwe@users.noreply.github.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>

components/script/build.rs

@@ -64,26 +64,55 @@ impl<'a> phf_shared::PhfHash for Bytes<'a> {
     }
 }
 
-fn find_python() -> String {
+/// Tries to find a suitable python
-    env::var("PYTHON3").ok().unwrap_or_else(|| {
+///
-        let candidates = if cfg!(windows) {
+/// Algorithm
-            ["python.exe", "python"]
+/// 1. Trying to find python3/python in $VIRTUAL_ENV (this should be from Servo's venv)
-        } else {
+/// 2. Checking PYTHON3 (set by mach)
-            ["python3", "python"]
+/// 3. Falling back to the system installation.
+///
+/// Note: This function should be kept in sync with the version in `components/servo/build.rs`
+fn find_python() -> PathBuf {
+    let mut candidates = vec![];
+    if let Some(venv) = env::var_os("VIRTUAL_ENV") {
+        let bin_directory = PathBuf::from(venv).join("bin");
+
+        let python3 = bin_directory.join("python3");
+        if python3.exists() {
+            candidates.push(python3);
+        }
+        let python = bin_directory.join("python");
+        if python.exists() {
+            candidates.push(python);
+        }
     };
-        for &name in &candidates {
+    if let Some(python3) = env::var_os("PYTHON3") {
-            if Command::new(name)
+        let python3 = PathBuf::from(python3);
+        if python3.exists() {
+            candidates.push(python3);
+        }
+    }
+
+    let system_python = ["python3", "python"].map(PathBuf::from);
+    candidates.extend_from_slice(&system_python);
+
+    for name in &candidates {
+        // Command::new() allows us to omit the `.exe` suffix on windows
+        if Command::new(&name)
             .arg("--version")
             .output()
-                .ok()
+            .is_ok_and(|out| out.status.success())
-                .map_or(false, |out| out.status.success())
         {
             return name.to_owned();
         }
     }
+    let candidates = candidates
+        .into_iter()
+        .map(|c| c.into_os_string())
+        .collect::<Vec<_>>();
     panic!(
-            "Can't find python (tried {})! Try fixing PATH or setting the PYTHON3 env var",
+        "Can't find python (tried {:?})! Try enabling Servo's Python venv, \
-            candidates.join(", ")
+        setting the PYTHON3 env var or adding python3 to PATH.",
+        candidates.join(", ".as_ref())
     )
-    })
 }

components/servo/build.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::{env, fs};
 
@@ -23,26 +23,47 @@ fn main() {
     fs::write(path, output.stdout).unwrap();
 }
 
-fn find_python() -> String {
+/// Tries to find a suitable python
-    env::var("PYTHON3").ok().unwrap_or_else(|| {
+///
-        let candidates = if cfg!(windows) {
+/// Algorithm
-            ["python3.8.exe", "python38.exe", "python.exe"]
+/// 1. Trying to find python3/python in $VIRTUAL_ENV (this should be from servos venv)
-        } else {
+/// 2. Checking PYTHON3 (set by mach)
-            ["python3.8", "python3", "python"]
+/// 3. Falling back to the system installation.
-        };
+///
-        for &name in &candidates {
+/// Note: This function should be kept in sync with the version in `components/script/build.rs`
-            if Command::new(name)
+fn find_python() -> PathBuf {
+    let mut candidates = vec![];
+    if let Some(venv) = env::var_os("VIRTUAL_ENV") {
+        // See: https://docs.python.org/3/library/venv.html#how-venvs-work
+        let bin_dir = if cfg!(windows) { "Scripts" } else { "bin" };
+        let bin_directory = PathBuf::from(venv).join(bin_dir);
+        candidates.push(bin_directory.join("python3"));
+        candidates.push(bin_directory.join("python"));
+    }
+    if let Some(python3) = env::var_os("PYTHON3") {
+        candidates.push(PathBuf::from(python3));
+    }
+
+    let system_python = ["python3", "python"].map(PathBuf::from);
+    candidates.extend_from_slice(&system_python);
+
+    for name in &candidates {
+        // Command::new() allows us to omit the `.exe` suffix on windows
+        if Command::new(&name)
             .arg("--version")
             .output()
-                .ok()
+            .is_ok_and(|out| out.status.success())
-                .map_or(false, |out| out.status.success())
         {
             return name.to_owned();
         }
     }
+    let candidates = candidates
+        .into_iter()
+        .map(|c| c.into_os_string())
+        .collect::<Vec<_>>();
     panic!(
-            "Can't find python (tried {})! Try fixing PATH or setting the PYTHON3 env var",
+        "Can't find python (tried {:?})! Try enabling Servo's Python venv, \
-            candidates.join(", ")
+        setting the PYTHON3 env var or adding python3 to PATH.",
+        candidates.join(", ".as_ref())
     )
-    })
 }