Respect requires for non-build-backend PEP 517 builds (#530)

## Summary

This PR modifies `puffin-build` to be closer in behavior to
[pip](a15dd75d98/src/pip/_internal/pyproject.py (L53))
and
[build](de5b44b0c2/src/build/__init__.py (L94)).

Specifically, if a project contains a `[build-system]` field, but no
`build-backend`, we now perform a PEP 517 build (instead of using
`setup.py` directly) _and_ respect the `requires` of the
`[build-system]`. Without this change, we were failing to build source
distributions for packages like `ujson`.

Closes #527.

---------

Co-authored-by: konstin <konstin@mailbox.org>
This commit is contained in:
Charlie Marsh 2023-12-04 05:13:42 -05:00 committed by GitHub
parent 6dc8ebcb90
commit d96c18b3a8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 152 additions and 35 deletions

View file

@ -241,40 +241,58 @@ impl SourceBuild {
source_root
};
// Check if we have a PEP 517 build, a legacy setup.py, or an edge case
let build_system = if source_tree.join("pyproject.toml").is_file() {
// Check if we have a PEP 517 build backend.
let pep517_backend = if source_tree.join("pyproject.toml").is_file() {
let pyproject_toml: PyProjectToml =
toml::from_str(&fs::read_to_string(source_tree.join("pyproject.toml"))?)
.map_err(Error::InvalidPyprojectToml)?;
pyproject_toml.build_system
if let Some(build_system) = pyproject_toml.build_system {
Some(Pep517Backend {
// If `build-backend` is missing, inject the legacy setuptools backend, but
// retain the `requires`, to match `pip` and `build`. Note that while PEP 517
// says that in this case we "should revert to the legacy behaviour of running
// `setup.py` (either directly, or by implicitly invoking the
// `setuptools.build_meta:__legacy__` backend)", we found that in practice, only
// the legacy setuptools backend is allowed. See also:
// https://github.com/pypa/build/blob/de5b44b0c28c598524832dff685a98d5a5148c44/src/build/__init__.py#L114-L118
backend: build_system
.build_backend
.unwrap_or_else(|| "setuptools.build_meta:__legacy__".to_string()),
backend_path: build_system.backend_path,
requirements: build_system.requires,
})
} else {
// If a `pyproject.toml` is present, but `[build-system]` is missing, proceed with
// a PEP 517 build using the default backend, to match `pip` and `build`.
Some(Pep517Backend {
backend: "setuptools.build_meta:__legacy__".to_string(),
backend_path: None,
requirements: vec![
Requirement::from_str("wheel").unwrap(),
Requirement::from_str("setuptools >= 40.8.0").unwrap(),
],
})
}
} else {
None
};
let venv = gourgeist::create_venv(&temp_dir.path().join(".venv"), interpreter)?;
// There are packages such as DTLSSocket 0.1.16 that say:
// ```toml
// [build-system]
// requires = ["Cython<3", "setuptools", "wheel"]
// ```
// In this case, we need to install requires PEP 517 style but then call setup.py in the
// legacy way.
let requirements = if let Some(build_system) = build_system.as_ref() {
// .filter(|build_system| build_system.build_backend.is_some()) {
// Setup the build environment using PEP 517 or the legacy setuptools backend.
if let Some(pep517_backend) = pep517_backend.as_ref() {
let resolved_requirements = build_context
.resolve(&build_system.requires)
.resolve(&pep517_backend.requirements)
.await
.map_err(|err| {
Error::RequirementsInstall("build-system.requires (resolve)", err)
})?;
Error::RequirementsInstall("build-system.requires (resolve)", err)
})?;
build_context
.install(&resolved_requirements, &venv)
.await
.map_err(|err| {
Error::RequirementsInstall("build-system.requires (install)", err)
})?;
build_system.requires.clone()
} else {
let requirements = vec![
Requirement::from_str("wheel").unwrap(),
@ -296,25 +314,6 @@ impl SourceBuild {
.install(&resolved_requirements, &venv)
.await
.map_err(|err| Error::RequirementsInstall("setup.py build (install)", err))?;
requirements
};
// > If the pyproject.toml file is absent, or the build-backend key is missing, the
// > source tree is not using this specification, and tools should revert to the legacy
// > behaviour of running setup.py (either directly, or by implicitly invoking the
// > setuptools.build_meta:__legacy__ backend).
let pep517_backend = if let Some(build_system) = build_system {
if let Some(backend) = build_system.build_backend {
Some(Pep517Backend {
backend,
backend_path: build_system.backend_path,
requirements,
})
} else {
None
}
} else {
None
};
if let Some(pep517_backend) = &pep517_backend {

View file

@ -855,3 +855,75 @@ fn install_local_source_distribution() -> Result<()> {
Ok(())
}
/// The `ujson` package includes a `[build-system]`, but no `build-backend`. It lists some explicit
/// build requirements, but _also_ depends on `wheel` and `setuptools`:
/// ```toml
/// [build-system]
/// requires = ["setuptools>=42", "setuptools_scm[toml]>=3.4"]
/// ```
///
/// Like `pip` and `build`, we should use PEP 517 here and respect the `requires`, but use the
/// default build backend.
#[test]
fn install_ujson() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.touch()?;
requirements_txt.write_str("ujson @ https://files.pythonhosted.org/packages/43/1a/b0a027144aa5c8f4ea654f4afdd634578b450807bb70b9f8bad00d6f6d3c/ujson-5.7.0.tar.gz")?;
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-sync")
.arg("requirements.txt")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.current_dir(&temp_dir));
});
check_command(&venv, "import ujson", &temp_dir);
Ok(())
}
/// The `DTLSSocket` package includes a `[build-system]`, but no `build-backend`. It lists some
/// explicit build requirements that are necessary to build the distribution:
/// ```toml
/// [build-system]
/// requires = ["Cython<3", "setuptools", "wheel"]
/// ```
///
/// Like `pip` and `build`, we should use PEP 517 here and respect the `requires`, but use the
/// default build backend.
#[test]
fn install_dtls_socket() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.touch()?;
requirements_txt.write_str("DTLSSocket @ https://files.pythonhosted.org/packages/58/42/0a0442118096eb9fbc9dc70b45aee2957f7546b80545e2a05bd839380519/DTLSSocket-0.1.16.tar.gz")?;
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-sync")
.arg("requirements.txt")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.current_dir(&temp_dir));
});
check_command(&venv, "import DTLSSocket", &temp_dir);
Ok(())
}

View file

@ -0,0 +1,23 @@
---
source: crates/puffin-cli/tests/pip_sync.rs
info:
program: puffin
args:
- pip-sync
- requirements.txt
- "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpPcrNSG
env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpwgSxYj/.venv
---
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Unzipped 1 package in [TIME]
Installed 1 package in [TIME]
+ dtlssocket @ https://files.pythonhosted.org/packages/58/42/0a0442118096eb9fbc9dc70b45aee2957f7546b80545e2a05bd839380519/DTLSSocket-0.1.16.tar.gz

View file

@ -0,0 +1,23 @@
---
source: crates/puffin-cli/tests/pip_sync.rs
info:
program: puffin
args:
- pip-sync
- requirements.txt
- "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpXdV1Uk
env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpAX03SD/.venv
---
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Unzipped 1 package in [TIME]
Installed 1 package in [TIME]
+ ujson @ https://files.pythonhosted.org/packages/43/1a/b0a027144aa5c8f4ea654f4afdd634578b450807bb70b9f8bad00d6f6d3c/ujson-5.7.0.tar.gz