Repository: PyO3/maturin (language: python)

Issue:
Maturin sdist Generation Fails When Using --no-isolation
### Bug Description

Hello,

I have been working on getting Rust builds with maturin in our company's build system. This system isolates builds in an environment already and then runs `sys.executable -m build --sdist --outdir=<staging dir> --no-isolation`. Maturin then fails with ` Found pyo3 bindings
💥 maturin failed
  Caused by: Invalid python interpreter version`.

I traced this issue to https://github.com/PyO3/maturin/blob/main/src/build_options.rs#L1206. Here, maturin calls `python_ver.split_once('.')...`. However, in a no-isolation situation, for sdist generation, maturin gets the python interpreter from `target.get_python`. This will return `python3` for non-venv linux systems. If `python3` is not available on the system, either because the symlink doesn't exist or because it isn't in the system PATH, maturin calls `find_interpreter_in_sysconfig`, which has the aforementioned `python_ver.split_once('.')`. When we try to `split_once` on that, it fails.

I'm happy to put up a fix for this if I can get some guidance on what the proper fix should be. We could first check for just a major version for example and go from there if that would work downstream.

Our build system is quite complicated, but I can reproduce this locally as well and with multiple python versions.

### Your maturin version (`maturin --version`)

1.7.4

### Your Python version (`python -V`)

3.12

### Your pip version (`pip -V`)

23.2.1

### What bindings you're using

pyo3

### Does `cargo build` work?

- [X] Yes, it works

### If on windows, have you checked that you aren't accidentally using unix path (those with the forward slash `/`)?

- [ ] Yes

### Steps to Reproduce

1. Create a minimal python/rust library with just PyO3 as a dependency.
2. In a linux system, (no venv), install build and maturin.
3. Run `python3.12 -m build --sdist --no-isolation .`
4. Maturin should crash with the same error.


Discussion / hints:
I just tried reproducing this with the [`python:3.12`](https://hub.docker.com/_/python) Docker image and I can successfully build the sample extension generated by `maturin init`:

```
🔗 Found pyo3 bindings
🐍 Found CPython 3.12 at /usr/local/bin/python3
📡 Using build options features from pyproject.toml
From `cargo package --list --allow-dirty --manifest-path /tmp/foobar/Cargo.toml`:
warning: manifest has no description, license, license-file, documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
📦 Built source distribution to /tmp/foobar/dist/foobar-0.1.0.tar.gz
foobar-0.1.0.tar.gz
Successfully built foobar-0.1.0.tar.gz
```
As you can see in the output above, the Python executable was resolved to `/usr/local/bin/python3`, which should trigger the issue, but it doesn't in this case. So I suspect that there is something else at play. 

@paddonizio is there something else I'm missing here to reproduce it? 
I was just testing out my fix and realized the same thing. Upon further investigation, I think the one additional piece is that `python3` cannot be in the PATH. In our build environment's case, we don't symlink python3.x to python3, and coincidentally, the environment I was testing in outside of our build system also does not have python3 as a symlink. When this happens, we end up in the `find_interpreter_in_sysconfig` code that has the version split issue described above. 
Thanks, I was able to reproduce it now by removing the `python3` symlink in the container. From what I can tell, the issue is that unless the interpreter is explicitly specified on the command line, maturin falls back to a default `python3` executable [here](https://github.com/PyO3/maturin/blob/83c7dd282e18026b7bb4fccd1f31f61f42b96abf/src/build_options.rs#L596). 

When building an sdist, the [`pep517 write-sdist`](https://github.com/PyO3/maturin/blob/83c7dd282e18026b7bb4fccd1f31f61f42b96abf/maturin/__init__.py#L124) subcommand is invoked, which unlike `build-wheel`, does not take an interpreter argument. And because maturin falls back to the hard coded `python3` value outside of a venv, it just expects to find a `python3` executable on the `PATH`.

I think `write-sdist` could also take a `--interpreter` argument, which can be set as `sys.executable` in `__init__.py` and also allows users of the `maturin` CLI to set the interpreter they want to use to build the sdist. 
@lkollar,

Thanks for your input. I put up a PR with your thoughts as it made sense to me. Upon looking though, I don't actually see where interpreter is even used for sdist generation, though I could have missed something. Either way, I tested this candidate on my local system and it worked. Let me know your thoughts.
Actually, that's a very good point, maybe there is no real need for the interpreter at all on this code path and the right thing to do would be to simply side step the interpreter detection when building an sdist? That would probably require making the interpreter an optional field of `BuildContext`. It would be great if a maintainer could chime in.

Fix patch:
diff --git a/deny.toml b/deny.toml
index 58d8d7fe..2d3b8d6b 100644
--- a/deny.toml
+++ b/deny.toml
@@ -77,6 +77,8 @@ feature-depth = 1
 ignore = [
     # multipart unmaintained
     "RUSTSEC-2023-0050",
+    # derivative unmaintained
+    "RUSTSEC-2024-0388",
     #{ id = "RUSTSEC-0000-0000", reason = "you can specify a reason the advisory is ignored" },
     #"a-crate-that-is-yanked@0.1.1", # you can also ignore yanked crate versions if you wish
     #{ crate = "a-crate-that-is-yanked@0.1.1", reason = "you can specify why you are ignoring the yanked crate" },
diff --git a/src/build_options.rs b/src/build_options.rs
index 6df3dacb..b6c0bb3a 100644
--- a/src/build_options.rs
+++ b/src/build_options.rs
@@ -487,12 +487,59 @@ impl BuildOptions {
 
     /// Tries to fill the missing metadata for a BuildContext by querying cargo and python
     #[instrument(skip_all)]
-    pub fn into_build_context(
-        self,
-        release: bool,
-        strip: bool,
-        editable: bool,
-    ) -> Result<BuildContext> {
+    pub fn into_build_context(self) -> BuildContextBuilder {
+        BuildContextBuilder::new(self)
+    }
+}
+
+#[derive(Debug)]
+pub struct BuildContextBuilder {
+    build_options: BuildOptions,
+    release: bool,
+    strip: bool,
+    editable: bool,
+    sdist_only: bool,
+}
+
+impl BuildContextBuilder {
+    fn new(build_options: BuildOptions) -> Self {
+        Self {
+            build_options,
+            release: false,
+            strip: false,
+            editable: false,
+            sdist_only: false,
+        }
+    }
+
+    pub fn release(mut self, release: bool) -> Self {
+        self.release = release;
+        self
+    }
+
+    pub fn strip(mut self, strip: bool) -> Self {
+        self.strip = strip;
+        self
+    }
+
+    pub fn editable(mut self, editable: bool) -> Self {
+        self.editable = editable;
+        self
+    }
+
+    pub fn sdist_only(mut self, sdist_only: bool) -> Self {
+        self.sdist_only = sdist_only;
+        self
+    }
+
+    pub fn build(self) -> Result<BuildContext> {
+        let Self {
+            build_options,
+            release,
+            strip,
+            editable,
+            sdist_only,
+        } = self;
         let ProjectResolver {
             project_layout,
             cargo_toml_path,
@@ -504,12 +551,15 @@ impl BuildOptions {
             mut cargo_options,
             cargo_metadata,
             mut pyproject_toml_maturin_options,
-        } = ProjectResolver::resolve(self.manifest_path.clone(), self.cargo.clone())?;
+        } = ProjectResolver::resolve(
+            build_options.manifest_path.clone(),
+            build_options.cargo.clone(),
+        )?;
         let pyproject = pyproject_toml.as_ref();
 
         let bridge = find_bridge(
             &cargo_metadata,
-            self.bindings.as_deref().or_else(|| {
+            build_options.bindings.as_deref().or_else(|| {
                 pyproject.and_then(|x| {
                     if x.bindings().is_some() {
                         pyproject_toml_maturin_options.push("bindings");
@@ -527,7 +577,7 @@ impl BuildOptions {
             );
         }
 
-        let mut target_triple = self.target.clone();
+        let mut target_triple = build_options.target.clone();
 
         let mut universal2 = target_triple.as_deref() == Some("universal2-apple-darwin");
         // Also try to determine universal2 from ARCHFLAGS environment variable
@@ -560,7 +610,7 @@ impl BuildOptions {
 
         let mut target = Target::from_target_triple(target_triple)?;
         if !target.user_specified && !universal2 {
-            if let Some(interpreter) = self.interpreter.first() {
+            if let Some(interpreter) = build_options.interpreter.first() {
                 if let Some(detected_target) =
                     crate::target::detect_arch_from_python(interpreter, &target)
                 {
@@ -569,38 +619,23 @@ impl BuildOptions {
             }
         }
 
-        let wheel_dir = match self.out {
+        let wheel_dir = match build_options.out {
             Some(ref dir) => dir.clone(),
             None => PathBuf::from(&cargo_metadata.target_directory).join("wheels"),
         };
 
         let generate_import_lib = is_generating_import_lib(&cargo_metadata)?;
-        let interpreter = if self.find_interpreter {
-            // Auto-detect interpreters
-            self.find_interpreters(
+        let interpreter = if sdist_only && env::var_os("MATURIN_TEST_PYTHON").is_none() {
+            // We don't need a python interpreter to build sdist only
+            Vec::new()
+        } else {
+            resolve_interpreters(
+                &build_options,
                 &bridge,
-                &[],
                 &target,
                 metadata23.requires_python.as_ref(),
                 generate_import_lib,
             )?
-        } else {
-            // User given list of interpreters
-            let interpreter = if self.interpreter.is_empty() && !target.cross_compiling() {
-                if cfg!(test) {
-                    match env::var_os("MATURIN_TEST_PYTHON") {
-                        Some(python) => vec![python.into()],
-                        None => vec![target.get_python()],
-                    }
-                } else {
-                    vec![target.get_python()]
-                }
-            } else {
-                // XXX: False positive clippy warning
-                #[allow(clippy::redundant_clone)]
-                self.interpreter.clone()
-            };
-            self.find_interpreters(&bridge, &interpreter, &target, None, generate_import_lib)?
         };
 
         if cargo_options.args.is_empty() {
@@ -613,9 +648,9 @@ impl BuildOptions {
         }
 
         let strip = pyproject.map(|x| x.strip()).unwrap_or_default() || strip;
-        let skip_auditwheel =
-            pyproject.map(|x| x.skip_auditwheel()).unwrap_or_default() || self.skip_auditwheel;
-        let auditwheel = self
+        let skip_auditwheel = pyproject.map(|x| x.skip_auditwheel()).unwrap_or_default()
+            || build_options.skip_auditwheel;
+        let auditwheel = build_options
             .auditwheel
             .or_else(|| pyproject.and_then(|x| x.auditwheel()))
             .unwrap_or(if skip_auditwheel {
@@ -623,9 +658,9 @@ impl BuildOptions {
             } else {
                 AuditWheelMode::Repair
             });
-        let platform_tags = if self.platform_tag.is_empty() {
+        let platform_tags = if build_options.platform_tag.is_empty() {
             #[cfg(feature = "zig")]
-            let use_zig = self.zig;
+            let use_zig = build_options.zig;
             #[cfg(not(feature = "zig"))]
             let use_zig = false;
             let compatibility = pyproject
@@ -658,7 +693,7 @@ impl BuildOptions {
                 Vec::new()
             }
         } else {
-            self.platform_tag
+            build_options.platform_tag
         };
 
         for platform_tag in &platform_tags {
@@ -683,7 +718,7 @@ impl BuildOptions {
             );
         }
 
-        let target_dir = self
+        let target_dir = build_options
             .cargo
             .target_dir
             .clone()
@@ -713,7 +748,7 @@ impl BuildOptions {
             strip,
             auditwheel,
             #[cfg(feature = "zig")]
-            zig: self.zig,
+            zig: build_options.zig,
             platform_tag: platform_tags,
             interpreter,
             cargo_metadata,
@@ -724,6 +759,43 @@ impl BuildOptions {
     }
 }
 
+fn resolve_interpreters(
+    build_options: &BuildOptions,
+    bridge: &BridgeModel,
+    target: &Target,
+    requires_python: Option<&VersionSpecifiers>,
+    generate_import_lib: bool,
+) -> Result<Vec<PythonInterpreter>, anyhow::Error> {
+    let interpreter = if build_options.find_interpreter {
+        // Auto-detect interpreters
+        build_options.find_interpreters(
+            bridge,
+            &[],
+            target,
+            requires_python,
+            generate_import_lib,
+        )?
+    } else {
+        // User given list of interpreters
+        let interpreter = if build_options.interpreter.is_empty() && !target.cross_compiling() {
+            if cfg!(test) {
+                match env::var_os("MATURIN_TEST_PYTHON") {
+                    Some(python) => vec![python.into()],
+                    None => vec![target.get_python()],
+                }
+            } else {
+                vec![target.get_python()]
+            }
+        } else {
+            // XXX: False positive clippy warning
+            #[allow(clippy::redundant_clone)]
+            build_options.interpreter.clone()
+        };
+        build_options.find_interpreters(bridge, &interpreter, target, None, generate_import_lib)?
+    };
+    Ok(interpreter)
+}
+
 /// Checks for bridge/platform type edge cases
 fn validate_bridge_type(
     bridge: &BridgeModel,
diff --git a/src/develop.rs b/src/develop.rs
index 6884d56f..517595d0 100644
--- a/src/develop.rs
+++ b/src/develop.rs
@@ -332,7 +332,12 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> {
         },
     };
 
-    let build_context = build_options.into_build_context(release, strip, true)?;
+    let build_context = build_options
+        .into_build_context()
+        .release(release)
+        .strip(strip)
+        .editable(true)
+        .build()?;
 
     let interpreter =
         PythonInterpreter::check_executable(&python, &target, build_context.bridge())?.ok_or_else(
diff --git a/src/main.rs b/src/main.rs
index 8836ef55..c366cd8e 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -273,7 +273,12 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
             strip,
         } => {
             assert_eq!(build_options.interpreter.len(), 1);
-            let context = build_options.into_build_context(true, strip, false)?;
+            let context = build_options
+                .into_build_context()
+                .release(true)
+                .strip(strip)
+                .editable(false)
+                .build()?;
 
             // Since afaik all other PEP 517 backends also return linux tagged wheels, we do so too
             let tags = match context.bridge() {
@@ -298,7 +303,12 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
             strip,
             editable,
         } => {
-            let build_context = build_options.into_build_context(true, strip, editable)?;
+            let build_context = build_options
+                .into_build_context()
+                .release(true)
+                .strip(strip)
+                .editable(editable)
+                .build()?;
             let wheels = build_context.build_wheels()?;
             assert_eq!(wheels.len(), 1);
             println!("{}", wheels[0].0.to_str().unwrap());
@@ -318,7 +328,13 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
                 },
                 ..Default::default()
             };
-            let build_context = build_options.into_build_context(false, false, false)?;
+            let build_context = build_options
+                .into_build_context()
+                .release(false)
+                .strip(false)
+                .editable(false)
+                .sdist_only(true)
+                .build()?;
             let (path, _) = build_context
                 .build_source_distribution()?
                 .context("Failed to build source distribution, pyproject.toml not found")?;
@@ -361,7 +377,12 @@ fn run() -> Result<()> {
             strip,
             sdist,
         } => {
-            let build_context = build.into_build_context(release, strip, false)?;
+            let build_context = build
+                .into_build_context()
+                .release(release)
+                .strip(strip)
+                .editable(false)
+                .build()?;
             if sdist {
                 build_context
                     .build_source_distribution()?
@@ -378,7 +399,12 @@ fn run() -> Result<()> {
             no_strip,
             no_sdist,
         } => {
-            let build_context = build.into_build_context(!debug, !no_strip, false)?;
+            let build_context = build
+                .into_build_context()
+                .release(!debug)
+                .strip(!no_strip)
+                .editable(false)
+                .build()?;
 
             if !build_context.release {
                 eprintln!("⚠️  Warning: You're publishing debug wheels");
@@ -427,7 +453,13 @@ fn run() -> Result<()> {
                 },
                 ..Default::default()
             };
-            let build_context = build_options.into_build_context(false, false, false)?;
+            let build_context = build_options
+                .into_build_context()
+                .release(false)
+                .strip(false)
+                .editable(false)
+                .sdist_only(true)
+                .build()?;
             build_context
                 .build_source_distribution()?
                 .context("Failed to build source distribution, pyproject.toml not found")?;


Test patch:
diff --git a/tests/common/errors.rs b/tests/common/errors.rs
index 0c9cfabe..98b2760d 100644
--- a/tests/common/errors.rs
+++ b/tests/common/errors.rs
@@ -19,7 +19,12 @@ pub fn abi3_without_version() -> Result<()> {
     ];
 
     let options = BuildOptions::try_parse_from(cli)?;
-    let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
+    let result = options
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build();
     if let Err(err) = result {
         assert_eq!(err.to_string(),
             "You have selected the `abi3` feature but not a minimum version (e.g. the `abi3-py36` feature). \
@@ -48,7 +53,11 @@ pub fn pyo3_no_extension_module() -> Result<()> {
 
     let options = BuildOptions::try_parse_from(cli)?;
     let result = options
-        .into_build_context(false, cfg!(feature = "faster-tests"), false)?
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build()?
         .build_wheels();
     if let Err(err) = result {
         if !(err
@@ -81,7 +90,12 @@ pub fn locked_doesnt_build_without_cargo_lock() -> Result<()> {
         "test-crates/targets/locked_doesnt_build_without_cargo_lock",
     ];
     let options = BuildOptions::try_parse_from(cli)?;
-    let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
+    let result = options
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build();
     if let Err(err) = result {
         let err_string = err
             .source()
@@ -115,7 +129,11 @@ pub fn invalid_manylinux_does_not_panic() -> Result<()> {
     ];
     let options: BuildOptions = BuildOptions::try_parse_from(cli)?;
     let result = options
-        .into_build_context(false, cfg!(feature = "faster-tests"), false)?
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build()?
         .build_wheels();
     if let Err(err) = result {
         assert_eq!(err.to_string(), "Error ensuring manylinux_2_99 compliance");
diff --git a/tests/common/integration.rs b/tests/common/integration.rs
index 9ff3803a..de5144db 100644
--- a/tests/common/integration.rs
+++ b/tests/common/integration.rs
@@ -125,7 +125,12 @@ pub fn test_integration(
     }
 
     let options: BuildOptions = BuildOptions::try_parse_from(cli)?;
-    let build_context = options.into_build_context(false, cfg!(feature = "faster-tests"), false)?;
+    let build_context = options
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build()?;
     let wheels = build_context.build_wheels()?;
 
     // For abi3 on unix, we didn't use a python interpreter, but we need one here
@@ -248,7 +253,12 @@ pub fn test_integration_conda(package: impl AsRef<Path>, bindings: Option<String
 
     let options = BuildOptions::try_parse_from(cli)?;
 
-    let build_context = options.into_build_context(false, cfg!(feature = "faster-tests"), false)?;
+    let build_context = options
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build()?;
     let wheels = build_context.build_wheels()?;
 
     let mut conda_wheels: Vec<(PathBuf, PathBuf)> = vec![];
diff --git a/tests/common/other.rs b/tests/common/other.rs
index 2ab2136a..4448885d 100644
--- a/tests/common/other.rs
+++ b/tests/common/other.rs
@@ -68,7 +68,12 @@ pub fn test_musl() -> Result<bool> {
         "test-crates/wheels/test_musl",
     ])?;
 
-    let build_context = options.into_build_context(false, cfg!(feature = "faster-tests"), false)?;
+    let build_context = options
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build()?;
     let built_lib =
         PathBuf::from("test-crates/targets/test_musl/x86_64-unknown-linux-musl/debug/hello-world");
     if built_lib.is_file() {
@@ -107,7 +112,12 @@ pub fn test_workspace_cargo_lock() -> Result<()> {
         "test-crates/wheels/test_workspace_cargo_lock",
     ])?;
 
-    let build_context = options.into_build_context(false, false, false)?;
+    let build_context = options
+        .into_build_context()
+        .release(false)
+        .strip(false)
+        .editable(false)
+        .build()?;
     let source_distribution = build_context.build_source_distribution()?;
     assert!(source_distribution.is_some());
 
@@ -137,7 +147,13 @@ pub fn test_source_distribution(
         ..Default::default()
     };
 
-    let mut build_context = build_options.into_build_context(false, false, false)?;
+    let mut build_context = build_options
+        .into_build_context()
+        .release(false)
+        .strip(false)
+        .editable(false)
+        .sdist_only(true)
+        .build()?;
 
     // Override the sdist generator for testing
     let mut pyproject_toml = build_context.pyproject_toml.take().unwrap();
@@ -207,7 +223,12 @@ fn build_wheel_files(package: impl AsRef<Path>, unique_name: &str) -> Result<Zip
         ..Default::default()
     };
 
-    let build_context = build_options.into_build_context(false, false, false)?;
+    let build_context = build_options
+        .into_build_context()
+        .release(false)
+        .strip(false)
+        .editable(false)
+        .build()?;
     let wheels = build_context
         .build_wheels()
         .context("Failed to build wheels")?;
@@ -263,7 +284,12 @@ pub fn abi3_python_interpreter_args() -> Result<()> {
         "test-crates/pyo3-pure/Cargo.toml",
         "--quiet",
     ])?;
-    let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
+    let result = options
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build();
     assert!(result.is_ok());
 
     // Case 2: maturin build -i python3.10, should work because python3.10 is in bundled sysconfigs
@@ -275,7 +301,12 @@ pub fn abi3_python_interpreter_args() -> Result<()> {
         "-i",
         "python3.10",
     ])?;
-    let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
+    let result = options
+        .into_build_context()
+        .release(false)
+        .strip(cfg!(feature = "faster-tests"))
+        .editable(false)
+        .build();
     assert!(result.is_ok());
 
     // Windows is a bit different so we exclude it from case 3 & 4
@@ -291,7 +322,12 @@ pub fn abi3_python_interpreter_args() -> Result<()> {
             "-i",
             "python2.7",
         ])?;
-        let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
+        let result = options
+            .into_build_context()
+            .release(false)
+            .strip(cfg!(feature = "faster-tests"))
+            .editable(false)
+            .build();
         assert!(result.is_err());
 
         // Case 4: maturin build -i python-does-not-exists, errors because python executable is not found
@@ -303,7 +339,12 @@ pub fn abi3_python_interpreter_args() -> Result<()> {
             "-i",
             "python-does-not-exists",
         ])?;
-        let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
+        let result = options
+            .into_build_context()
+            .release(false)
+            .strip(cfg!(feature = "faster-tests"))
+            .editable(false)
+            .build();
         assert!(result.is_err());
     }