From c0b11b6a7415e4f2aca01f3594d523046ae6e123 Mon Sep 17 00:00:00 2001 From: Ed Morley <501702+edmorley@users.noreply.github.com> Date: Wed, 7 Aug 2024 15:22:00 +0100 Subject: [PATCH] Stop installing setuptools and wheel Currently the buildpack performs a system site-packages install of not only pip, but also setuptools and wheel. This has historically been necessary for pip to be able to build source distributions (sdists) for packages that don't ship with compatible wheels. However: - Thanks to PEP 518, packages can now (and many already do) specify an explicit build backend using `[build-system]` in their `pyproject.toml`. The dependencies specified in that config (such as setuptools and wheel) will be installed by pip into an isolated and ephemeral build environment as part of the source distribution build process. Such packages therefore don't need/use globally installed setuptools/wheel versions. - As of pip v22.1, pip will now default to the isolated build environment mode (along with a fallback legacy setuptools build backend), if the setuptools package isn't installed globally. This means that packages that haven't yet migrated to a PEP 518 `pyproject.toml` build backend config can still build even if setuptools isn't installed globally. There are a small number of rarely used packages in the wild that aren't compatible with build isolation mode, however, these typically require more build dependencies than just setuptools, which means they wouldn't have worked with this buildpack anyway. As such, it's no longer necessary for us to install setuptools and wheel globally. This matches the behaviour of the `venv` and `ensurepip` modules in Python 3.12+, where setuptools and wheel installation has also been removed. And it also matches the default behaviour of Poetry too, whose `install --sync` command removes any implicitly installed packages in the current environment (other than pip). See: https://peps.python.org/pep-0518/ https://pip.pypa.io/en/stable/reference/build-system/ https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/#build-isolation https://github.com/pypa/pip/pull/10717 https://github.com/python/cpython/pull/101039 https://github.com/pypa/get-pip/pull/218 https://github.com/astral-sh/uv/issues/2252 GUS-W-16437776. --- CHANGELOG.md | 4 +++ requirements/setuptools.txt | 1 - requirements/wheel.txt | 1 - src/errors.rs | 4 +-- src/layers/pip_cache.rs | 13 ++++--- src/layers/python.rs | 66 ++++++---------------------------- src/main.rs | 20 +++-------- src/packaging_tool_versions.rs | 30 ++-------------- tests/pip_test.rs | 61 ++++++++++--------------------- tests/python_version_test.rs | 14 +++----- 10 files changed, 52 insertions(+), 162 deletions(-) delete mode 100644 requirements/setuptools.txt delete mode 100644 requirements/wheel.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 08dcda7..064958c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Removed + +- Stopped explicitly installing setuptools and wheel. They will be automatically installed by pip into an isolated build environment if they are required for building a package. ([#243](https://github.com/heroku/buildpacks-python/pull/243)) + ## [0.13.0] - 2024-08-01 ### Changed diff --git a/requirements/setuptools.txt b/requirements/setuptools.txt deleted file mode 100644 index d125505..0000000 --- a/requirements/setuptools.txt +++ /dev/null @@ -1 +0,0 @@ -setuptools==70.3.0 diff --git a/requirements/wheel.txt b/requirements/wheel.txt deleted file mode 100644 index ee8fec0..0000000 --- a/requirements/wheel.txt +++ /dev/null @@ -1 +0,0 @@ -wheel==0.43.0 diff --git a/src/errors.rs b/src/errors.rs index 845e0d9..012c7d5 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -129,13 +129,13 @@ fn on_python_layer_error(error: PythonLayerError) { PythonLayerError::BootstrapPipCommand(error) => match error { StreamedCommandError::Io(io_error) => log_io_error( "Unable to bootstrap pip", - "running the command to install pip, setuptools and wheel", + "running the command to install pip", &io_error, ), StreamedCommandError::NonZeroExitStatus(exit_status) => log_error( "Unable to bootstrap pip", formatdoc! {" - The command to install pip, setuptools and wheel did not exit successfully ({exit_status}). + The command to install pip did not exit successfully ({exit_status}). See the log output above for more information. diff --git a/src/layers/pip_cache.rs b/src/layers/pip_cache.rs index b3493f6..fa21766 100644 --- a/src/layers/pip_cache.rs +++ b/src/layers/pip_cache.rs @@ -1,4 +1,4 @@ -use crate::packaging_tool_versions::PackagingToolVersions; +use crate::packaging_tool_versions::PIP_VERSION; use crate::python_version::PythonVersion; use crate::{BuildpackError, PythonBuildpack}; use libcnb::build::BuildContext; @@ -17,14 +17,13 @@ pub(crate) fn prepare_pip_cache( context: &BuildContext, env: &mut Env, python_version: &PythonVersion, - packaging_tool_versions: &PackagingToolVersions, ) -> Result<(), libcnb::Error> { let new_metadata = PipCacheLayerMetadata { arch: context.target.arch.clone(), distro_name: context.target.distro_name.clone(), distro_version: context.target.distro_version.clone(), python_version: python_version.to_string(), - packaging_tool_versions: packaging_tool_versions.clone(), + pip_version: PIP_VERSION.to_string(), }; let layer = context.cached_layer( @@ -74,9 +73,9 @@ pub(crate) fn prepare_pip_cache( Ok(()) } -// Timestamp based cache invalidation isn't used here since the Python/pip/setuptools/wheel -// versions will change often enough that it isn't worth the added complexity. Ideally pip -// would support cleaning up its own cache: https://github.com/pypa/pip/issues/6956 +// Timestamp based cache invalidation isn't used here since the Python and pip versions will +// change often enough that it isn't worth the added complexity. Ideally pip would support +// cleaning up its own cache: https://github.com/pypa/pip/issues/6956 #[derive(Deserialize, PartialEq, Serialize)] #[serde(deny_unknown_fields)] struct PipCacheLayerMetadata { @@ -84,5 +83,5 @@ struct PipCacheLayerMetadata { distro_name: String, distro_version: String, python_version: String, - packaging_tool_versions: PackagingToolVersions, + pip_version: String, } diff --git a/src/layers/python.rs b/src/layers/python.rs index 7c70bfd..5de98ba 100644 --- a/src/layers/python.rs +++ b/src/layers/python.rs @@ -1,4 +1,4 @@ -use crate::packaging_tool_versions::PackagingToolVersions; +use crate::packaging_tool_versions::PIP_VERSION; use crate::python_version::PythonVersion; use crate::utils::{self, DownloadUnpackArchiveError, StreamedCommandError}; use crate::{BuildpackError, PythonBuildpack}; @@ -17,7 +17,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::{fs, io}; -/// Creates a layer containing the Python runtime and the packages `pip`, `setuptools` and `wheel`. +/// Creates a layer containing the Python runtime and pip. // // We install both Python and the packaging tools into the same layer, since: // - We don't want to mix buildpack/packaging dependencies with the app's own dependencies @@ -31,25 +31,18 @@ use std::{fs, io}; // - This leaves just the system site-packages directory, which exists within the Python // installation directory and Python does not support moving it elsewhere. // - It matches what both local and official Docker image environments do. -#[allow(clippy::too_many_lines)] pub(crate) fn install_python_and_packaging_tools( context: &BuildContext, env: &mut Env, python_version: &PythonVersion, - packaging_tool_versions: &PackagingToolVersions, ) -> Result<(), libcnb::Error> { let new_metadata = PythonLayerMetadata { arch: context.target.arch.clone(), distro_name: context.target.distro_name.clone(), distro_version: context.target.distro_version.clone(), python_version: python_version.to_string(), - packaging_tool_versions: packaging_tool_versions.clone(), + pip_version: PIP_VERSION.to_string(), }; - let PackagingToolVersions { - pip_version, - setuptools_version, - wheel_version, - } = packaging_tool_versions; let layer = context.cached_layer( layer_name!("python"), @@ -71,9 +64,8 @@ pub(crate) fn install_python_and_packaging_tools( match layer.state { LayerState::Restored { .. } => { - log_info(format!("Using cached Python {python_version}")); log_info(format!( - "Using cached pip {pip_version}, setuptools {setuptools_version} and wheel {wheel_version}" + "Using cached Python {python_version} and pip {PIP_VERSION}" )); } LayerState::Empty { ref cause } => { @@ -117,9 +109,7 @@ pub(crate) fn install_python_and_packaging_tools( return Ok(()); } - log_info(format!( - "Installing pip {pip_version}, setuptools {setuptools_version} and wheel {wheel_version}" - )); + log_info(format!("Installing pip {PIP_VERSION}")); let python_stdlib_dir = layer_path.join(format!( "lib/python{}.{}", @@ -140,9 +130,7 @@ pub(crate) fn install_python_and_packaging_tools( "--no-cache-dir", "--no-input", "--quiet", - format!("pip=={pip_version}").as_str(), - format!("setuptools=={setuptools_version}").as_str(), - format!("wheel=={wheel_version}").as_str(), + format!("pip=={PIP_VERSION}").as_str(), ]) .current_dir(&context.app_dir) .env_clear() @@ -170,7 +158,7 @@ struct PythonLayerMetadata { distro_name: String, distro_version: String, python_version: String, - packaging_tool_versions: PackagingToolVersions, + pip_version: String, } /// Compare cached layer metadata to the new layer metadata to determine if the cache should be @@ -189,12 +177,7 @@ fn cache_invalidation_reasons( distro_name: cached_distro_name, distro_version: cached_distro_version, python_version: cached_python_version, - packaging_tool_versions: - PackagingToolVersions { - pip_version: cached_pip_version, - setuptools_version: cached_setuptools_version, - wheel_version: cached_wheel_version, - }, + pip_version: cached_pip_version, } = cached_metadata; let PythonLayerMetadata { @@ -202,12 +185,7 @@ fn cache_invalidation_reasons( distro_name, distro_version, python_version, - packaging_tool_versions: - PackagingToolVersions { - pip_version, - setuptools_version, - wheel_version, - }, + pip_version, } = new_metadata; let mut reasons = Vec::new(); @@ -236,18 +214,6 @@ fn cache_invalidation_reasons( )); } - if cached_setuptools_version != setuptools_version { - reasons.push(format!( - "The setuptools version has changed from {cached_setuptools_version} to {setuptools_version}" - )); - } - - if cached_wheel_version != wheel_version { - reasons.push(format!( - "The wheel version has changed from {cached_wheel_version} to {wheel_version}" - )); - } - reasons } @@ -423,11 +389,7 @@ mod tests { distro_name: "ubuntu".to_string(), distro_version: "22.04".to_string(), python_version: "3.11.0".to_string(), - packaging_tool_versions: PackagingToolVersions { - pip_version: "A.B.C".to_string(), - setuptools_version: "D.E.F".to_string(), - wheel_version: "G.H.I".to_string(), - }, + pip_version: "A.B.C".to_string(), } } @@ -462,11 +424,7 @@ mod tests { distro_name: "debian".to_string(), distro_version: "12".to_string(), python_version: "3.11.1".to_string(), - packaging_tool_versions: PackagingToolVersions { - pip_version: "A.B.C-new".to_string(), - setuptools_version: "D.E.F-new".to_string(), - wheel_version: "G.H.I-new".to_string(), - }, + pip_version: "A.B.C-new".to_string(), }; assert_eq!( cache_invalidation_reasons(&cached_metadata, &new_metadata), @@ -475,8 +433,6 @@ mod tests { "The OS has changed from ubuntu-22.04 to debian-12", "The Python version has changed from 3.11.0 to 3.11.1", "The pip version has changed from A.B.C to A.B.C-new", - "The setuptools version has changed from D.E.F to D.E.F-new", - "The wheel version has changed from G.H.I to G.H.I-new" ] ); } diff --git a/src/main.rs b/src/main.rs index 0c63e06..d35bbb2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,7 +13,6 @@ use crate::layers::pip_dependencies::PipDependenciesLayerError; use crate::layers::python::{self, PythonLayerError}; use crate::layers::{pip_cache, pip_dependencies}; use crate::package_manager::{DeterminePackageManagerError, PackageManager}; -use crate::packaging_tool_versions::PackagingToolVersions; use crate::python_version::PythonVersionError; use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder}; use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder}; @@ -53,7 +52,6 @@ impl Buildpack for PythonBuildpack { log_header("Determining Python version"); let python_version = python_version::determine_python_version(&context.app_dir) .map_err(BuildpackError::PythonVersion)?; - let packaging_tool_versions = PackagingToolVersions::default(); // We inherit the current process's env vars, since we want `PATH` and `HOME` from the OS // to be set (so that later commands can find tools like Git in the base image), along @@ -62,26 +60,16 @@ impl Buildpack for PythonBuildpack { // making sure that buildpack env vars take precedence in layers envs and command usage. let mut env = Env::from_current(); - // Create the layer containing the Python runtime, and the packages `pip`, `setuptools` and `wheel`. - log_header("Installing Python and packaging tools"); - python::install_python_and_packaging_tools( - &context, - &mut env, - &python_version, - &packaging_tool_versions, - )?; + // Create the layer containing the Python runtime and pip. + log_header("Installing Python and pip"); + python::install_python_and_packaging_tools(&context, &mut env, &python_version)?; // Create the layers for the application dependencies and package manager cache. // In the future support will be added for package managers other than pip. let dependencies_layer_dir = match package_manager { PackageManager::Pip => { log_header("Installing dependencies using pip"); - pip_cache::prepare_pip_cache( - &context, - &mut env, - &python_version, - &packaging_tool_versions, - )?; + pip_cache::prepare_pip_cache(&context, &mut env, &python_version)?; pip_dependencies::install_dependencies(&context, &mut env)? } }; diff --git a/src/packaging_tool_versions.rs b/src/packaging_tool_versions.rs index 5dbf972..28f2f27 100644 --- a/src/packaging_tool_versions.rs +++ b/src/packaging_tool_versions.rs @@ -1,36 +1,10 @@ -use serde::{Deserialize, Serialize}; use std::str; // We store these versions in requirements files so that Dependabot can update them. // Each file must contain a single package specifier in the format `package==1.2.3`, // from which we extract/validate the version substring at compile time. -const PIP_VERSION: &str = extract_requirement_version(include_str!("../requirements/pip.txt")); -const SETUPTOOLS_VERSION: &str = - extract_requirement_version(include_str!("../requirements/setuptools.txt")); -const WHEEL_VERSION: &str = extract_requirement_version(include_str!("../requirements/wheel.txt")); - -/// The versions of various packaging tools used during the build. -/// These are always installed, and are independent of the chosen package manager. -/// Strings are used instead of a semver version, since these packages don't use -/// semver, and we never introspect the version parts anyway. -#[allow(clippy::struct_field_names)] -#[derive(Clone, Deserialize, PartialEq, Serialize)] -#[serde(deny_unknown_fields)] -pub(crate) struct PackagingToolVersions { - pub(crate) pip_version: String, - pub(crate) setuptools_version: String, - pub(crate) wheel_version: String, -} - -impl Default for PackagingToolVersions { - fn default() -> Self { - Self { - pip_version: PIP_VERSION.to_string(), - setuptools_version: SETUPTOOLS_VERSION.to_string(), - wheel_version: WHEEL_VERSION.to_string(), - } - } -} +pub(crate) const PIP_VERSION: &str = + extract_requirement_version(include_str!("../requirements/pip.txt")); // Extract the version substring from an exact-version package specifier (such as `foo==1.2.3`). // This function should only be used to extract the version constants from the buildpack's own diff --git a/tests/pip_test.rs b/tests/pip_test.rs index 1663242..dedf027 100644 --- a/tests/pip_test.rs +++ b/tests/pip_test.rs @@ -1,17 +1,11 @@ -use crate::packaging_tool_versions::PackagingToolVersions; -use crate::tests::{builder, default_build_config, DEFAULT_PYTHON_VERSION}; +use crate::packaging_tool_versions::PIP_VERSION; +use crate::tests::{default_build_config, DEFAULT_PYTHON_VERSION}; use indoc::{formatdoc, indoc}; use libcnb_test::{assert_contains, assert_empty, BuildpackReference, PackResult, TestRunner}; #[test] #[ignore = "integration test"] fn pip_basic_install_and_cache_reuse() { - let PackagingToolVersions { - pip_version, - setuptools_version, - wheel_version, - } = PackagingToolVersions::default(); - let config = default_build_config("tests/fixtures/pip_basic"); TestRunner::default().build(&config, |context| { @@ -23,9 +17,9 @@ fn pip_basic_install_and_cache_reuse() { No Python version specified, using the current default of Python {DEFAULT_PYTHON_VERSION}. To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes - [Installing Python and packaging tools] + [Installing Python and pip] Installing Python {DEFAULT_PYTHON_VERSION} - Installing pip {pip_version}, setuptools {setuptools_version} and wheel {wheel_version} + Installing pip {PIP_VERSION} [Installing dependencies using pip] Running pip install @@ -40,7 +34,7 @@ fn pip_basic_install_and_cache_reuse() { // Check that: // - The correct env vars are set at run-time. // - pip is available at run-time too (and not just during the build). - // - The correct versions of pip/setuptools/wheel were installed. + // - The correct version of pip was installed. // - pip uses (via 'PYTHONUSERBASE') the user site-packages in the dependencies // layer, and so can find the typing-extensions package installed there. // - The "pip update available" warning is not shown (since it should be suppressed). @@ -69,10 +63,8 @@ fn pip_basic_install_and_cache_reuse() { Package Version ----------------- ------- - pip {pip_version} - setuptools {setuptools_version} + pip {PIP_VERSION} typing_extensions 4.7.1 - wheel {wheel_version} Defaulting to user installation because normal site-packages is not writeable Requirement already satisfied: typing-extensions in /layers/heroku_python/dependencies/lib/" } @@ -87,9 +79,8 @@ fn pip_basic_install_and_cache_reuse() { No Python version specified, using the current default of Python {DEFAULT_PYTHON_VERSION}. To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes - [Installing Python and packaging tools] - Using cached Python {DEFAULT_PYTHON_VERSION} - Using cached pip {pip_version}, setuptools {setuptools_version} and wheel {wheel_version} + [Installing Python and pip] + Using cached Python {DEFAULT_PYTHON_VERSION} and pip {PIP_VERSION} [Installing dependencies using pip] Using cached pip download/wheel cache @@ -111,17 +102,16 @@ fn pip_basic_install_and_cache_reuse() { #[test] #[ignore = "integration test"] fn pip_cache_invalidation_with_compatible_metadata() { - let PackagingToolVersions { - pip_version, - setuptools_version, - wheel_version, - } = PackagingToolVersions::default(); + // TODO: Re-enable this test the next time the default-Python/pip versions change, at which point + // there will be a historic buildpack version with compatible metadata that triggers invalidation. + #![allow(unreachable_code)] + return; let config = default_build_config("tests/fixtures/pip_basic"); TestRunner::default().build( config.clone().buildpacks([BuildpackReference::Other( - "docker://docker.io/heroku/buildpack-python:0.10.0".to_string(), + "docker://docker.io/heroku/buildpack-python:TODO".to_string(), )]), |context| { context.rebuild(config, |rebuild_context| { @@ -133,13 +123,12 @@ fn pip_cache_invalidation_with_compatible_metadata() { No Python version specified, using the current default of Python {DEFAULT_PYTHON_VERSION}. To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes - [Installing Python and packaging tools] + [Installing Python and pip] Discarding cache since: - The Python version has changed from 3.12.3 to {DEFAULT_PYTHON_VERSION} - - The pip version has changed from 24.0 to {pip_version} - - The setuptools version has changed from 69.5.1 to {setuptools_version} + - The pip version has changed from 24.0 to {PIP_VERSION} Installing Python {DEFAULT_PYTHON_VERSION} - Installing pip {pip_version}, setuptools {setuptools_version} and wheel {wheel_version} + Installing pip {PIP_VERSION} [Installing dependencies using pip] Discarding cached pip download/wheel cache @@ -162,23 +151,11 @@ fn pip_cache_invalidation_with_compatible_metadata() { #[test] #[ignore = "integration test"] fn pip_cache_invalidation_with_incompatible_metadata() { - // TODO: Enable this test on Heroku-24 the next time there is an incompatible metadata change, - // meaning we can bump the historic buildpack version to one that also supports Ubuntu 24.04. - if builder() == "heroku/builder:24" { - return; - } - - let PackagingToolVersions { - pip_version, - setuptools_version, - wheel_version, - } = PackagingToolVersions::default(); - let config = default_build_config("tests/fixtures/pip_basic"); TestRunner::default().build( config.clone().buildpacks([BuildpackReference::Other( - "docker://docker.io/heroku/buildpack-python:0.8.4".to_string(), + "docker://docker.io/heroku/buildpack-python:0.13.0".to_string(), )]), |context| { context.rebuild(config, |rebuild_context| { @@ -190,10 +167,10 @@ fn pip_cache_invalidation_with_incompatible_metadata() { No Python version specified, using the current default of Python {DEFAULT_PYTHON_VERSION}. To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes - [Installing Python and packaging tools] + [Installing Python and pip] Discarding cache since the buildpack cache format has changed Installing Python {DEFAULT_PYTHON_VERSION} - Installing pip {pip_version}, setuptools {setuptools_version} and wheel {wheel_version} + Installing pip {PIP_VERSION} [Installing dependencies using pip] Discarding cached pip download/wheel cache diff --git a/tests/python_version_test.rs b/tests/python_version_test.rs index 96e0b77..52e1cfa 100644 --- a/tests/python_version_test.rs +++ b/tests/python_version_test.rs @@ -1,4 +1,4 @@ -use crate::packaging_tool_versions::PackagingToolVersions; +use crate::packaging_tool_versions::PIP_VERSION; use crate::tests::{ builder, default_build_config, DEFAULT_PYTHON_VERSION, LATEST_PYTHON_3_10, LATEST_PYTHON_3_11, LATEST_PYTHON_3_12, LATEST_PYTHON_3_7, LATEST_PYTHON_3_8, LATEST_PYTHON_3_9, @@ -20,7 +20,7 @@ fn python_version_unspecified() { No Python version specified, using the current default of Python {DEFAULT_PYTHON_VERSION}. To use a different version, see: https://devcenter.heroku.com/articles/python-runtimes - [Installing Python and packaging tools] + [Installing Python and pip] Installing Python {DEFAULT_PYTHON_VERSION} "} ); @@ -78,12 +78,6 @@ fn python_3_12() { } fn builds_with_python_version(fixture_path: &str, python_version: &str) { - let PackagingToolVersions { - pip_version, - setuptools_version, - wheel_version, - } = PackagingToolVersions::default(); - TestRunner::default().build(default_build_config(fixture_path), |context| { assert_empty!(context.pack_stderr); assert_contains!( @@ -92,9 +86,9 @@ fn builds_with_python_version(fixture_path: &str, python_version: &str) { [Determining Python version] Using Python version {python_version} specified in runtime.txt - [Installing Python and packaging tools] + [Installing Python and pip] Installing Python {python_version} - Installing pip {pip_version}, setuptools {setuptools_version} and wheel {wheel_version} + Installing pip {PIP_VERSION} "} ); // There's no sensible default process type we can set for Python apps.