From 55f2be72e2c81ccc9b3514de6e2afe10bbff9bbe Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 9 Jan 2024 20:27:06 -0500 Subject: [PATCH] Default to PEP 517-based builds (#843) ## Summary Our current setup uses the legacy `setup.py`-based builds if a `pyproject.toml` file isn't present. This matches pip's behavior. However, `pypa/build` uses PEP 517-based builds in such cases, and it looks like pip plans to make that the default (https://github.com/pypa/pip/issues/9175), with the limiting factor being performance issues related to isolated builds. This is now the default behavior, but the `--legacy-setup-py` flag allows users to opt-in to using `setup.py` directly for distributions that lack a `pyproject.toml`. --- crates/puffin-build/src/lib.rs | 165 +++++++++++------- crates/puffin-cli/src/commands/pip_compile.rs | 3 + crates/puffin-cli/src/commands/pip_install.rs | 4 +- crates/puffin-cli/src/commands/pip_sync.rs | 4 +- crates/puffin-cli/src/main.rs | 31 ++++ crates/puffin-cli/tests/pip_compile.rs | 87 +++++++++ crates/puffin-cli/tests/pip_sync.rs | 71 ++++++++ crates/puffin-dev/src/build.rs | 5 +- crates/puffin-dev/src/install_many.rs | 5 +- crates/puffin-dev/src/resolve_cli.rs | 3 + crates/puffin-dev/src/resolve_many.rs | 4 +- crates/puffin-dispatch/src/lib.rs | 10 +- crates/puffin-resolver/tests/resolver.rs | 10 +- crates/puffin-traits/src/lib.rs | 17 +- 14 files changed, 342 insertions(+), 77 deletions(-) diff --git a/crates/puffin-build/src/lib.rs b/crates/puffin-build/src/lib.rs index bfe703321..70359c23b 100644 --- a/crates/puffin-build/src/lib.rs +++ b/crates/puffin-build/src/lib.rs @@ -11,9 +11,7 @@ use std::process::Output; use std::str::FromStr; use std::sync::Arc; -use distribution_types::Resolution; use fs_err as fs; -use fs_err::DirEntry; use indoc::formatdoc; use itertools::Itertools; use once_cell::sync::Lazy; @@ -26,10 +24,11 @@ use tokio::process::Command; use tokio::sync::Mutex; use tracing::{debug, info_span, instrument, Instrument}; +use distribution_types::Resolution; use pep508_rs::Requirement; use puffin_extract::extract_source; use puffin_interpreter::{Interpreter, Virtualenv}; -use puffin_traits::{BuildContext, BuildKind, SourceBuildTrait}; +use puffin_traits::{BuildContext, BuildKind, SetupPyStrategy, SourceBuildTrait}; /// e.g. `pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory` static MISSING_HEADER_RE: Lazy = Lazy::new(|| { @@ -38,11 +37,22 @@ static MISSING_HEADER_RE: Lazy = Lazy::new(|| { ) .unwrap() }); + /// e.g. `/usr/bin/ld: cannot find -lncurses: No such file or directory` static LD_NOT_FOUND_RE: Lazy = Lazy::new(|| { Regex::new(r"/usr/bin/ld: cannot find -l([a-zA-Z10-9]+): No such file or directory").unwrap() }); +/// The default backend to use when PEP 517 is used without a `build-system` section. +static DEFAULT_BACKEND: Lazy = Lazy::new(|| 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(), + ], +}); + #[derive(Error, Debug)] pub enum Error { #[error(transparent)] @@ -89,8 +99,6 @@ pub enum MissingLibrary { #[derive(Debug, Error)] pub struct MissingHeaderCause { missing_library: MissingLibrary, - // I've picked this over the better readable package name to make clear that you need to - // look for the build dependencies of that version or git commit respectively package_id: String, } @@ -109,7 +117,7 @@ impl Display for MissingHeaderCause { f, "This error likely indicates that you need to install the library that provides a shared library \ for {library} for {package_id} (e.g. lib{library}-dev)", - library=library, package_id=self.package_id + library = library, package_id = self.package_id ) } } @@ -171,7 +179,7 @@ pub struct PyProjectToml { } /// `[build-backend]` from pyproject.toml -#[derive(Debug)] +#[derive(Debug, Clone, PartialEq, Eq)] struct Pep517Backend { /// The build backend string such as `setuptools.build_meta:__legacy__` or `maturin` from /// `build-backend.backend` in pyproject.toml @@ -222,7 +230,7 @@ impl Pep517Backend { #[derive(Debug, Default, Clone)] pub struct SourceBuildContext { /// Cache the first resolution of `pip`, `setuptools` and `wheel` we made for setup.py (and - /// some PEP 517) builds so we can reuse it + /// some PEP 517) builds so we can reuse it. setup_py_resolution: Arc>>, } @@ -234,8 +242,9 @@ pub struct SourceBuildContext { pub struct SourceBuild { temp_dir: TempDir, source_tree: PathBuf, - /// `Some` if this is a PEP 517 build + /// If performing a PEP 517 build, the backend to use. pep517_backend: Option, + /// The virtual environment in which to build the source distribution. venv: Virtualenv, /// Populated if `prepare_metadata_for_build_wheel` was called. /// @@ -258,6 +267,7 @@ impl SourceBuild { /// contents from an archive if necessary. /// /// `source_dist` is for error reporting only. + #[allow(clippy::too_many_arguments)] pub async fn setup( source: &Path, subdirectory: Option<&Path>, @@ -265,6 +275,7 @@ impl SourceBuild { build_context: &impl BuildContext, source_build_context: SourceBuildContext, package_id: String, + setup_py: SetupPyStrategy, build_kind: BuildKind, ) -> Result { let temp_dir = tempdir()?; @@ -283,6 +294,8 @@ impl SourceBuild { source_root }; + let default_backend: Pep517Backend = DEFAULT_BACKEND.clone(); + // Check if we have a PEP 517 build backend. let pep517_backend = match fs::read_to_string(source_tree.join("pyproject.toml")) { Ok(toml) => { @@ -306,74 +319,91 @@ impl SourceBuild { } 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(), - ], - }) + Some(default_backend.clone()) + } + } + Err(err) if err.kind() == io::ErrorKind::NotFound => { + // We require either a `pyproject.toml` or a `setup.py` file at the top level. + if !source_tree.join("setup.py").is_file() { + return Err(Error::InvalidSourceDist( + "The archive contains neither a `pyproject.toml` nor a `setup.py` file at the top level" + .to_string(), + )); + } + + // If no `pyproject.toml` is present, by default, proceed with a PEP 517 build using + // the default backend, to match `build`. `pip` uses `setup.py` directly in this + // case (which we allow via `SetupPyStrategy::Setuptools`), but plans to make PEP + // 517 builds the default in the future. + // See: https://github.com/pypa/pip/issues/9175. + match setup_py { + SetupPyStrategy::Pep517 => Some(default_backend.clone()), + SetupPyStrategy::Setuptools => None, } } - Err(err) if err.kind() == io::ErrorKind::NotFound => None, Err(err) => return Err(err.into()), }; let venv = gourgeist::create_venv(&temp_dir.path().join(".venv"), interpreter.clone())?; - // 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(&pep517_backend.requirements) - .await - .map_err(|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) - })?; + // Setup the build environment. + let resolved_requirements = if let Some(pep517_backend) = pep517_backend.as_ref() { + if pep517_backend.requirements == default_backend.requirements { + let mut resolution = source_build_context.setup_py_resolution.lock().await; + if let Some(resolved_requirements) = &*resolution { + resolved_requirements.clone() + } else { + let resolved_requirements = build_context + .resolve(&default_backend.requirements) + .await + .map_err(|err| { + Error::RequirementsInstall("setup.py build (resolve)", err) + })?; + *resolution = Some(resolved_requirements.clone()); + resolved_requirements + } + } else { + build_context + .resolve(&pep517_backend.requirements) + .await + .map_err(|err| { + Error::RequirementsInstall("build-system.requires (resolve)", err) + })? + } } else { - let requirements = vec![ - Requirement::from_str("wheel").unwrap(), - Requirement::from_str("setuptools").unwrap(), - Requirement::from_str("pip").unwrap(), - ]; + // Install default requirements for `setup.py`-based builds. let mut resolution = source_build_context.setup_py_resolution.lock().await; - let resolved_requirements = if let Some(resolved_requirements) = &*resolution { + if let Some(resolved_requirements) = &*resolution { resolved_requirements.clone() } else { let resolved_requirements = build_context - .resolve(&requirements) + .resolve(&default_backend.requirements) .await .map_err(|err| Error::RequirementsInstall("setup.py build (resolve)", err))?; *resolution = Some(resolved_requirements.clone()); resolved_requirements - }; - build_context - .install(&resolved_requirements, &venv) - .await - .map_err(|err| Error::RequirementsInstall("setup.py build (install)", err))?; + } }; + build_context + .install(&resolved_requirements, &venv) + .await + .map_err(|err| Error::RequirementsInstall("build-system.requires (install)", err))?; + + // If we're using the default backend configuration, skip `get_requires_for_build_*`, since + // we already installed the requirements above. if let Some(pep517_backend) = &pep517_backend { - create_pep517_build_environment( - &source_tree, - &venv, - pep517_backend, - build_context, - &package_id, - build_kind, - ) - .await?; - } else if !source_tree.join("setup.py").is_file() { - return Err(Error::InvalidSourceDist( - "The archive contains neither a `pyproject.toml` nor a `setup.py` file at the top level" - .to_string(), - )); + if pep517_backend != &default_backend { + create_pep517_build_environment( + &source_tree, + &venv, + pep517_backend, + build_context, + &package_id, + build_kind, + ) + .await?; + } } Ok(Self { @@ -390,12 +420,11 @@ impl SourceBuild { /// Try calling `prepare_metadata_for_build_wheel` to get the metadata without executing the /// actual build. pub async fn get_metadata_without_build(&mut self) -> Result, Error> { - // setup.py builds don't support this. let Some(pep517_backend) = &self.pep517_backend else { return Ok(None); }; - // We've already called this method, but return the existing result is easier than erroring + // We've already called this method; return the existing result. if let Some(metadata_dir) = &self.metadata_directory { return Ok(Some(metadata_dir.clone())); } @@ -503,7 +532,7 @@ impl SourceBuild { )); } let dist = fs::read_dir(self.source_tree.join("dist"))?; - let dist_dir = dist.collect::>>()?; + let dist_dir = dist.collect::>>()?; let [dist_wheel] = dist_dir.as_slice() else { return Err(Error::from_command_output( format!( @@ -622,8 +651,8 @@ async fn create_pep517_build_environment( "#, pep517_backend.backend_import(), build_kind }; let span = info_span!( - "get_requires_for_build_wheel", - script="build_wheel", + "run_python_script", + script=format!("get_requires_for_build_{}", build_kind), python_version = %venv.interpreter().version() ); let output = run_python_script(venv, &script, source_tree) @@ -644,6 +673,7 @@ async fn create_pep517_build_environment( .map_err(|err| err.to_string()) .and_then(|last_line| last_line.ok_or("Missing message".to_string())) .and_then(|message| serde_json::from_str(&message).map_err(|err| err.to_string())); + let extra_requires: Vec = extra_requires.map_err(|err| { Error::from_command_output( format!( @@ -653,14 +683,14 @@ async fn create_pep517_build_environment( package_id, ) })?; + // Some packages (such as tqdm 4.66.1) list only extra requires that have already been part of // the pyproject.toml requires (in this case, `wheel`). We can skip doing the whole resolution // and installation again. // TODO(konstin): Do we still need this when we have a fast resolver? - if !extra_requires.is_empty() - && !extra_requires - .iter() - .all(|req| pep517_backend.requirements.contains(req)) + if extra_requires + .iter() + .any(|req| !pep517_backend.requirements.contains(req)) { debug!("Installing extra requirements for build backend"); let requirements: Vec = pep517_backend @@ -679,6 +709,7 @@ async fn create_pep517_build_environment( .await .map_err(|err| Error::RequirementsInstall("build-system.requires (install)", err))?; } + Ok(()) } diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index c6c544ccb..9cefcfa6c 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -23,6 +23,7 @@ use puffin_installer::Downloader; use puffin_interpreter::{Interpreter, PythonVersion}; use puffin_normalize::ExtraName; use puffin_resolver::{Manifest, PreReleaseMode, ResolutionMode, ResolutionOptions, Resolver}; +use puffin_traits::SetupPyStrategy; use requirements_txt::EditableRequirement; use crate::commands::reporters::{DownloadReporter, ResolverReporter}; @@ -44,6 +45,7 @@ pub(crate) async fn pip_compile( prerelease_mode: PreReleaseMode, upgrade_mode: UpgradeMode, index_urls: IndexUrls, + setup_py: SetupPyStrategy, no_build: bool, python_version: Option, exclude_newer: Option>, @@ -141,6 +143,7 @@ pub(crate) async fn pip_compile( &interpreter, &index_urls, interpreter.sys_executable().to_path_buf(), + setup_py, no_build, ) .with_options(options); diff --git a/crates/puffin-cli/src/commands/pip_install.rs b/crates/puffin-cli/src/commands/pip_install.rs index 7f308d40d..a45f9dff3 100644 --- a/crates/puffin-cli/src/commands/pip_install.rs +++ b/crates/puffin-cli/src/commands/pip_install.rs @@ -28,7 +28,7 @@ use puffin_normalize::PackageName; use puffin_resolver::{ Manifest, PreReleaseMode, ResolutionGraph, ResolutionMode, ResolutionOptions, Resolver, }; -use puffin_traits::OnceMap; +use puffin_traits::{OnceMap, SetupPyStrategy}; use requirements_txt::EditableRequirement; use crate::commands::reporters::{DownloadReporter, InstallReporter, ResolverReporter}; @@ -48,6 +48,7 @@ pub(crate) async fn pip_install( index_urls: IndexUrls, reinstall: &Reinstall, link_mode: LinkMode, + setup_py: SetupPyStrategy, no_build: bool, strict: bool, exclude_newer: Option>, @@ -144,6 +145,7 @@ pub(crate) async fn pip_install( &interpreter, &index_urls, venv.python_executable(), + setup_py, no_build, ) .with_options(options); diff --git a/crates/puffin-cli/src/commands/pip_sync.rs b/crates/puffin-cli/src/commands/pip_sync.rs index ad7e1b2a1..ac8907bbd 100644 --- a/crates/puffin-cli/src/commands/pip_sync.rs +++ b/crates/puffin-cli/src/commands/pip_sync.rs @@ -14,7 +14,7 @@ use puffin_client::{RegistryClient, RegistryClientBuilder}; use puffin_dispatch::BuildDispatch; use puffin_installer::{Downloader, InstallPlan, Reinstall, ResolvedEditable, SitePackages}; use puffin_interpreter::Virtualenv; -use puffin_traits::OnceMap; +use puffin_traits::{OnceMap, SetupPyStrategy}; use pypi_types::Yanked; use requirements_txt::EditableRequirement; @@ -30,6 +30,7 @@ pub(crate) async fn pip_sync( reinstall: &Reinstall, link_mode: LinkMode, index_urls: IndexUrls, + setup_py: SetupPyStrategy, no_build: bool, strict: bool, cache: Cache, @@ -69,6 +70,7 @@ pub(crate) async fn pip_sync( venv.interpreter(), &index_urls, venv.python_executable(), + setup_py, no_build, ); diff --git a/crates/puffin-cli/src/main.rs b/crates/puffin-cli/src/main.rs index ff9562ec6..aeb9a1771 100644 --- a/crates/puffin-cli/src/main.rs +++ b/crates/puffin-cli/src/main.rs @@ -14,6 +14,7 @@ use puffin_installer::Reinstall; use puffin_interpreter::PythonVersion; use puffin_normalize::{ExtraName, PackageName}; use puffin_resolver::{PreReleaseMode, ResolutionMode}; +use puffin_traits::SetupPyStrategy; use requirements::ExtrasSpecification; use crate::commands::{extra_name_with_clap_error, ExitStatus}; @@ -166,6 +167,11 @@ struct PipCompileArgs { #[clap(long)] upgrade: bool, + /// Use legacy `setuptools` behavior when building source distributions without a + /// `pyproject.toml`. + #[clap(long)] + legacy_setup_py: bool, + /// Don't build source distributions. /// /// When enabled, resolving will not run arbitrary code. The cached wheels of already-built @@ -228,6 +234,11 @@ struct PipSyncArgs { #[clap(long, conflicts_with = "index_url", conflicts_with = "extra_index_url")] no_index: bool, + /// Use legacy `setuptools` behavior when building source distributions without a + /// `pyproject.toml`. + #[clap(long)] + legacy_setup_py: bool, + /// Don't build source distributions. /// /// When enabled, resolving will not run arbitrary code. The cached wheels of already-built @@ -324,6 +335,11 @@ struct PipInstallArgs { #[clap(long, conflicts_with = "index_url", conflicts_with = "extra_index_url")] no_index: bool, + /// Use legacy `setuptools` behavior when building source distributions without a + /// `pyproject.toml`. + #[clap(long)] + legacy_setup_py: bool, + /// Don't build source distributions. /// /// When enabled, resolving will not run arbitrary code. The cached wheels of already-built @@ -480,6 +496,11 @@ async fn inner() -> Result { args.prerelease, args.upgrade.into(), index_urls, + if args.legacy_setup_py { + SetupPyStrategy::Setuptools + } else { + SetupPyStrategy::Pep517 + }, args.no_build, args.python_version, args.exclude_newer, @@ -502,6 +523,11 @@ async fn inner() -> Result { &reinstall, args.link_mode, index_urls, + if args.legacy_setup_py { + SetupPyStrategy::Setuptools + } else { + SetupPyStrategy::Pep517 + }, args.no_build, args.strict, cache, @@ -547,6 +573,11 @@ async fn inner() -> Result { index_urls, &reinstall, args.link_mode, + if args.legacy_setup_py { + SetupPyStrategy::Setuptools + } else { + SetupPyStrategy::Pep517 + }, args.no_build, args.strict, args.exclude_newer, diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index 90143dd0c..9554dc7fb 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -2813,3 +2813,90 @@ fn trailing_slash() -> Result<()> { Ok(()) } + +/// Resolve a project without a `pyproject.toml`, using the PEP 517 build backend (default). +#[test] +fn compile_legacy_sdist_pep_517() -> Result<()> { + let temp_dir = TempDir::new()?; + let cache_dir = TempDir::new()?; + let venv = create_venv_py312(&temp_dir, &cache_dir); + + let requirements_in = temp_dir.child("requirements.in"); + requirements_in.write_str("flake8 @ https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.0.tar.gz")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("requirements.in") + .arg("--cache-dir") + .arg(cache_dir.path()) + .arg("--exclude-newer") + .arg(EXCLUDE_NEWER) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by Puffin v0.0.1 via the following command: + # puffin pip-compile requirements.in --cache-dir [CACHE_DIR] + flake8 @ https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.0.tar.gz + mccabe==0.7.0 + # via flake8 + pycodestyle==2.10.0 + # via flake8 + pyflakes==3.0.1 + # via flake8 + + ----- stderr ----- + Resolved 4 packages in [TIME] + "###); + }); + + Ok(()) +} + +/// Resolve a project without a `pyproject.toml`, using `setuptools` directly. +#[test] +fn compile_legacy_sdist_setuptools() -> Result<()> { + let temp_dir = TempDir::new()?; + let cache_dir = TempDir::new()?; + let venv = create_venv_py312(&temp_dir, &cache_dir); + + let requirements_in = temp_dir.child("requirements.in"); + requirements_in.write_str("flake8 @ https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.0.tar.gz")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("requirements.in") + .arg("--legacy-setup-py") + .arg("--cache-dir") + .arg(cache_dir.path()) + .arg("--exclude-newer") + .arg(EXCLUDE_NEWER) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by Puffin v0.0.1 via the following command: + # puffin pip-compile requirements.in --legacy-setup-py --cache-dir [CACHE_DIR] + flake8 @ https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.0.tar.gz + mccabe==0.7.0 + # via flake8 + pycodestyle==2.10.0 + # via flake8 + pyflakes==3.0.1 + # via flake8 + + ----- stderr ----- + Resolved 4 packages in [TIME] + "###); + }); + + Ok(()) +} diff --git a/crates/puffin-cli/tests/pip_sync.rs b/crates/puffin-cli/tests/pip_sync.rs index f23308d5b..a1a908102 100644 --- a/crates/puffin-cli/tests/pip_sync.rs +++ b/crates/puffin-cli/tests/pip_sync.rs @@ -2591,3 +2591,74 @@ fn incompatible_wheel() -> Result<()> { Ok(()) } + +/// Install a project without a `pyproject.toml`, using the PEP 517 build backend (default). +#[test] +fn sync_legacy_sdist_pep_517() -> 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_in = temp_dir.child("requirements.in"); + requirements_in.write_str("flake8 @ https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.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.in") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + flake8==6.0.0 (from https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.0.tar.gz) + "###); + }); + + Ok(()) +} + +/// Install a project without a `pyproject.toml`, using `setuptools` directly. +#[test] +fn sync_legacy_sdist_setuptools() -> 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_in = temp_dir.child("requirements.in"); + requirements_in.write_str("flake8 @ https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.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.in") + .arg("--legacy-setup-py") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + flake8==6.0.0 (from https://files.pythonhosted.org/packages/66/53/3ad4a3b74d609b3b9008a10075c40e7c8909eae60af53623c3888f7a529a/flake8-6.0.0.tar.gz) + "###); + }); + + Ok(()) +} diff --git a/crates/puffin-dev/src/build.rs b/crates/puffin-dev/src/build.rs index 7b89a68a0..06bad8010 100644 --- a/crates/puffin-dev/src/build.rs +++ b/crates/puffin-dev/src/build.rs @@ -12,7 +12,7 @@ use puffin_cache::{Cache, CacheArgs}; use puffin_client::RegistryClientBuilder; use puffin_dispatch::BuildDispatch; use puffin_interpreter::Virtualenv; -use puffin_traits::{BuildContext, BuildKind}; +use puffin_traits::{BuildContext, BuildKind, SetupPyStrategy}; #[derive(Parser)] pub(crate) struct BuildArgs { @@ -55,6 +55,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result { let venv = Virtualenv::from_env(platform, &cache)?; let client = RegistryClientBuilder::new(cache.clone()).build(); let index_urls = IndexUrls::default(); + let setup_py = SetupPyStrategy::default(); let build_dispatch = BuildDispatch::new( &client, @@ -62,6 +63,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result { venv.interpreter(), &index_urls, venv.python_executable(), + setup_py, false, ); @@ -72,6 +74,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result { &build_dispatch, SourceBuildContext::default(), args.sdist.display().to_string(), + setup_py, build_kind, ) .await?; diff --git a/crates/puffin-dev/src/install_many.rs b/crates/puffin-dev/src/install_many.rs index b06ecec6f..eacf65335 100644 --- a/crates/puffin-dev/src/install_many.rs +++ b/crates/puffin-dev/src/install_many.rs @@ -25,7 +25,7 @@ use puffin_installer::Downloader; use puffin_interpreter::Virtualenv; use puffin_normalize::PackageName; use puffin_resolver::DistFinder; -use puffin_traits::{BuildContext, OnceMap}; +use puffin_traits::{BuildContext, OnceMap, SetupPyStrategy}; #[derive(Parser)] pub(crate) struct InstallManyArgs { @@ -60,13 +60,16 @@ pub(crate) async fn install_many(args: InstallManyArgs) -> Result<()> { let venv = Virtualenv::from_env(platform, &cache)?; let client = RegistryClientBuilder::new(cache.clone()).build(); let index_urls = IndexUrls::default(); + let setup_py = SetupPyStrategy::default(); let tags = venv.interpreter().tags()?; + let build_dispatch = BuildDispatch::new( &client, &cache, venv.interpreter(), &index_urls, venv.python_executable(), + setup_py, args.no_build, ); diff --git a/crates/puffin-dev/src/resolve_cli.rs b/crates/puffin-dev/src/resolve_cli.rs index 25cda199e..3cfc9ab15 100644 --- a/crates/puffin-dev/src/resolve_cli.rs +++ b/crates/puffin-dev/src/resolve_cli.rs @@ -17,6 +17,7 @@ use puffin_client::RegistryClientBuilder; use puffin_dispatch::BuildDispatch; use puffin_interpreter::Virtualenv; use puffin_resolver::{Manifest, ResolutionOptions, Resolver}; +use puffin_traits::SetupPyStrategy; #[derive(ValueEnum, Default, Clone)] pub(crate) enum ResolveCliFormat { @@ -50,6 +51,7 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> { let venv = Virtualenv::from_env(platform, &cache)?; let client = RegistryClientBuilder::new(cache.clone()).build(); let index_urls = IndexUrls::default(); + let setup_py = SetupPyStrategy::default(); let build_dispatch = BuildDispatch::new( &client, @@ -57,6 +59,7 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> { venv.interpreter(), &index_urls, venv.python_executable(), + setup_py, args.no_build, ); diff --git a/crates/puffin-dev/src/resolve_many.rs b/crates/puffin-dev/src/resolve_many.rs index 6c23a35c5..9b3d5f3e4 100644 --- a/crates/puffin-dev/src/resolve_many.rs +++ b/crates/puffin-dev/src/resolve_many.rs @@ -20,7 +20,7 @@ use puffin_client::{RegistryClient, RegistryClientBuilder}; use puffin_dispatch::BuildDispatch; use puffin_interpreter::Virtualenv; use puffin_normalize::PackageName; -use puffin_traits::BuildContext; +use puffin_traits::{BuildContext, SetupPyStrategy}; #[derive(Parser)] pub(crate) struct ResolveManyArgs { @@ -74,6 +74,7 @@ pub(crate) async fn resolve_many(args: ResolveManyArgs) -> Result<()> { let venv = Virtualenv::from_env(platform, &cache)?; let client = RegistryClientBuilder::new(cache.clone()).build(); let index_urls = IndexUrls::default(); + let setup_py = SetupPyStrategy::default(); let build_dispatch = BuildDispatch::new( &client, @@ -81,6 +82,7 @@ pub(crate) async fn resolve_many(args: ResolveManyArgs) -> Result<()> { venv.interpreter(), &index_urls, venv.python_executable(), + setup_py, args.no_build, ); let build_dispatch = Arc::new(build_dispatch); diff --git a/crates/puffin-dispatch/src/lib.rs b/crates/puffin-dispatch/src/lib.rs index 903cd21fd..a3831a24c 100644 --- a/crates/puffin-dispatch/src/lib.rs +++ b/crates/puffin-dispatch/src/lib.rs @@ -17,7 +17,7 @@ use puffin_client::RegistryClient; use puffin_installer::{Downloader, InstallPlan, Installer, Reinstall, SitePackages}; use puffin_interpreter::{Interpreter, Virtualenv}; use puffin_resolver::{Manifest, ResolutionOptions, Resolver}; -use puffin_traits::{BuildContext, BuildKind, OnceMap}; +use puffin_traits::{BuildContext, BuildKind, OnceMap, SetupPyStrategy}; /// The main implementation of [`BuildContext`], used by the CLI, see [`BuildContext`] /// documentation. @@ -27,6 +27,7 @@ pub struct BuildDispatch<'a> { interpreter: &'a Interpreter, index_urls: &'a IndexUrls, base_python: PathBuf, + setup_py: SetupPyStrategy, no_build: bool, source_build_context: SourceBuildContext, options: ResolutionOptions, @@ -40,6 +41,7 @@ impl<'a> BuildDispatch<'a> { interpreter: &'a Interpreter, index_urls: &'a IndexUrls, base_python: PathBuf, + setup_py: SetupPyStrategy, no_build: bool, ) -> Self { Self { @@ -48,6 +50,7 @@ impl<'a> BuildDispatch<'a> { interpreter, index_urls, base_python, + setup_py, no_build, source_build_context: SourceBuildContext::default(), options: ResolutionOptions::default(), @@ -81,6 +84,10 @@ impl<'a> BuildContext for BuildDispatch<'a> { self.no_build } + fn setup_py_strategy(&self) -> SetupPyStrategy { + self.setup_py + } + async fn resolve<'data>(&'data self, requirements: &'data [Requirement]) -> Result { let markers = self.interpreter.markers(); let tags = self.interpreter.tags()?; @@ -232,6 +239,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { self, self.source_build_context.clone(), package_id.to_string(), + self.setup_py, build_kind, ) .await?; diff --git a/crates/puffin-resolver/tests/resolver.rs b/crates/puffin-resolver/tests/resolver.rs index a46040e26..74ffdf450 100644 --- a/crates/puffin-resolver/tests/resolver.rs +++ b/crates/puffin-resolver/tests/resolver.rs @@ -20,7 +20,7 @@ use puffin_interpreter::{Interpreter, Virtualenv}; use puffin_resolver::{ Manifest, PreReleaseMode, ResolutionGraph, ResolutionMode, ResolutionOptions, Resolver, }; -use puffin_traits::{BuildContext, BuildKind, SourceBuildTrait}; +use puffin_traits::{BuildContext, BuildKind, SetupPyStrategy, SourceBuildTrait}; // Exclude any packages uploaded after this date. static EXCLUDE_NEWER: Lazy> = Lazy::new(|| { @@ -49,6 +49,14 @@ impl BuildContext for DummyContext { panic!("The test should not need to build source distributions") } + fn no_build(&self) -> bool { + false + } + + fn setup_py_strategy(&self) -> SetupPyStrategy { + SetupPyStrategy::default() + } + async fn resolve<'a>(&'a self, _requirements: &'a [Requirement]) -> Result { panic!("The test should not need to build source distributions") } diff --git a/crates/puffin-traits/src/lib.rs b/crates/puffin-traits/src/lib.rs index 7fe2a38d3..e8e5e9f2a 100644 --- a/crates/puffin-traits/src/lib.rs +++ b/crates/puffin-traits/src/lib.rs @@ -68,9 +68,10 @@ pub trait BuildContext { /// Whether source distribution building is disabled. This [`BuildContext::setup_build`] calls /// will fail in this case. This method exists to avoid fetching source distributions if we know /// we can't build them - fn no_build(&self) -> bool { - false - } + fn no_build(&self) -> bool; + + /// The strategy to use when building source distributions that lack a `pyproject.toml`. + fn setup_py_strategy(&self) -> SetupPyStrategy; /// Resolve the given requirements into a ready-to-install set of package versions. fn resolve<'a>( @@ -123,6 +124,16 @@ pub trait SourceBuildTrait { -> impl Future> + Send + 'a; } +/// The strategy to use when building source distributions that lack a `pyproject.toml`. +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub enum SetupPyStrategy { + /// Perform a PEP 517 build. + #[default] + Pep517, + /// Perform a build by invoking `setuptools` directly. + Setuptools, +} + #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub enum BuildKind { /// A regular PEP 517 wheel build