Repository: pypa/build
Pull request #627 (2023-06-26 05:14:02)

main: ensure config_settings are passed to get_requires_for_build
Fixes builds for packages like psycopg2(for example if the `--pg-config` config setting needs to be set) that can hard error in some cases without `config_settings` being passed to their `get_requires_for_build` hooks.

layday: Please see https://github.com/pypa/build/issues/264.
jameshilliard: > Please see #264.

I'm a bit confused what's going on there, does the issue there apply to just `--global-option` usage or to `--build-option` as well? I'm using the `--build-option` [here](https://patchwork.ozlabs.org/project/buildroot/patch/20230626181531.2312002-5-james.hilliard1@gmail.com/) in my setuptools pep517 migration patch, does that look like the right approach or are there better options available?
henryiii: The problem is that if we fix this bug some usages of setuptools break.

IMO, I'd be happy to have this with some sort of flag that would allow it to be done correctly but have an opt-in to the old behavior to keep setuptools working, and maybe setuptools would finally fix it on their side. (CC @abravalheri)
jameshilliard: > I'd be happy to have this with some sort of flag that would allow it to be done correctly but have an opt-in to the old behavior to keep setuptools working

Any preference for what I should name the flag?
henryiii: `--no-config-settings-for-requires`, maybe? @layday, does that sound like a good path forward?
layday: Given that passing config settings to setuptools' `get_requires` is a practical impossibility the path of least resistance would be for setuptools to actually exclude config settings from `get_requires` until such time that they are namespaced. This can be done by e.g. providing an alternative `get_requires` config setting-less backend which can serve as a stopgap, much like `__legacy__`.
jameshilliard: > passing config settings to setuptools' `get_requires` is a practical impossibility the path of least resistance would be for setuptools to actually exclude config settings from `get_requires` until such time that they are namespaced

What's a good way to reproduce the issue with passing config settings to `get_requires`? It's unclear to me exactly what the issue is right now.

Did setuptools [adding the `--build-option` config](https://github.com/pypa/setuptools/pull/3380) change the situation?
layday: Passing any option that egg_info and sdist/bdist_wheel do not share will result in an error.

Sent with [Proton Mail](https://proton.me/) secure email.

------- Original Message -------
On Tuesday, June 27th, 2023 at 01:58, James Hilliard ***@***.***> wrote:

>> passing config settings to setuptools' get_requires is a practical impossibility the path of least resistance would be for setuptools to actually exclude config settings from get_requires until such time that they are namespaced
>
> What's a good way to reproduce the issue with passing config settings to get_requires? It's unclear to me exactly what the issue is right now.
>
> Did setuptools [adding the --build-option config](https://github.com/pypa/setuptools/pull/3380) change the situation?
>
> —
> Reply to this email directly, [view it on GitHub](https://github.com/pypa/build/pull/627#issuecomment-1608428708), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AHNRFWGCFNSRYNTI6VCMZX3XNIHZVANCNFSM6AAAAAAZTUGI2Y).
> You are receiving this because you were mentioned.Message ID: ***@***.***>
FFY00: I commented on https://github.com/pypa/build/issues/264#issuecomment-1609513063, but essentially I think we should proceed with this change special-casing setuptools to default to the old behavior.
FFY00: > `--no-config-settings-for-requires`, maybe? @layday, does that sound like a good path forward?

I'd prefer to not add more config options, it makes maintenance harder (remember this is something that will end up hardcoded in build scripts for decades!) and makes it more reasonable for build backends to not have a universal `config_settings` interface across all hooks. This is something that I'd definitely be open to considering if there's a good argument for it, but so far it's just "setuptools does not support (certain?) arguments in the `config_settings` for non `build_*` hooks".

We do need a way to temporarily switch to the old behavior, but I think an environment variable would be better suited here, as it won't cause breakage when we remove support for it (after which we would issue a warning if the envvar is set for a while at least).

> Given that passing config settings to setuptools' `get_requires` is a practical impossibility the path of least resistance would be for setuptools to actually exclude config settings from `get_requires` until such time that they are namespaced.

I agree.

> This can be done by e.g. providing an alternative `get_requires` config setting-less backend which can serve as a stopgap, much like `__legacy__`.

That will still require new versions of setuptools 🫤
henryiii: Environment variable sounds fine.

> That will still require new versions of setuptools 🫤

I think the idea was that a package could provide a custom build backend?

```toml
[build-system]
requires = ["setuptools"]
build-backend = "backend"
backend-path = ["_custom_build"]
```

```python
# _custom_build.py
import setuptools.build_meta
from setuptools.build_meta import *

def get_requires_for_build_sdist(config_settings=None):
    return setuptools.build_meta.get_requires_for_build_sdist()
# etc.
```

Though I don't think that every package that's affected should be adding a workaround, or even chaining to some new "setuptools.build_meta.__modern__" or something backend either; setuptools should be able to handle config-settings without errors for these other commands.
henryiii: Also, since this might take a while, what about having something like `BUILD_PASS_CONFIG_SETTINGS_TO_REQUIRES`, were setting it to 0 will be the current behavior, and 1 will be the new behavior. If unset, the next version of build will default to 0, while a future version of build will change the default to 1. That way, we can ship a workaround for this now without breaking lots of setuptools projects, and in the future, once there's a released version of setuptools that won't break when this is set, we can make the change so then projects with old setuptools have to opt-into the old workaround.

Otherwise, I don't see a good way to allow non-setuptools backends to work correctly quickly.
layday: I don't think an env var is much better than a flag.  Making something less discoverable doesn't mean we can make breaking changes to it more easily.  A `BUILD_PASS_CONFIG_SETTINGS_TO_REQUIRES` flag is still something we'll probably want to deprecate before removal; worse, we'll need to implement custom error handling post-removal instead of having argparse error on being passed an unknown option.  I don't like either option personally.  I think we should:

* Delay merging this PR and work with the setuptools folks to provide an alternative (be it a new backend or a new way to pass config settings)
* *Or* allow passing different config settings per hook from build - we can discuss how this is gonna look
henryiii: I’d like two more approvals on this before merging of possible. Ideally @layday and @FFY00.
layday: I'm convinced by the findings in #264 that this is the best way forward.

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 074282d..d6bc397 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -22,20 +22,20 @@ repos:
     hooks:
       - id: validate-pyproject
   - repo: https://github.com/psf/black
-    rev: 23.3.0
+    rev: 23.7.0
     hooks:
       - id: black
   - repo: https://github.com/asottile/blacken-docs
-    rev: 1.14.0
+    rev: 1.15.0
     hooks:
       - id: blacken-docs
-        additional_dependencies: [black==23.3.0]
+        additional_dependencies: [black==23.7.0]
   - repo: https://github.com/pre-commit/mirrors-prettier
-    rev: "v3.0.0-alpha.9-for-vscode"
+    rev: "v3.0.0"
     hooks:
       - id: prettier
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.0.276
+    rev: v0.0.280
     hooks:
       - id: ruff
         args: [--fix, --format, grouped, --show-fixes]
diff --git a/pyproject.toml b/pyproject.toml
index dfeb60a..0457578 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -36,7 +36,7 @@ dependencies = [
   "pyproject_hooks",
   # not actually a runtime dependency, only supplied as there is not "recommended dependency" support
   'colorama; os_name == "nt"',
-  'importlib-metadata >= 0.22; python_version < "3.8"',
+  'importlib-metadata >= 4.6; python_version < "3.10"',  # Not required for 3.8+, but fixes a stdlib bug
   'tomli >= 1.1.0; python_version < "3.11"',
 ]
 
diff --git a/src/build/__main__.py b/src/build/__main__.py
index 7edbe4f..ed6934c 100644
--- a/src/build/__main__.py
+++ b/src/build/__main__.py
@@ -112,7 +112,7 @@ def _build_in_isolated_env(
         # first install the build dependencies
         env.install(builder.build_system_requires)
         # then get the extra required dependencies from the backend (which was installed in the call above :P)
-        env.install(builder.get_requires_for_build(distribution))
+        env.install(builder.get_requires_for_build(distribution, config_settings or {}))
         return builder.build(distribution, outdir, config_settings or {})
 
 
@@ -126,7 +126,7 @@ def _build_in_current_env(
     builder = _ProjectBuilder(srcdir)
 
     if not skip_dependency_check:
-        missing = builder.check_dependencies(distribution)
+        missing = builder.check_dependencies(distribution, config_settings or {})
         if missing:
             dependencies = ''.join('\n\t' + dep for deps in missing for dep in (deps[0], _format_dep_chain(deps[1:])) if dep)
             _cprint()
diff --git a/src/build/_importlib.py b/src/build/_importlib.py
new file mode 100644
index 0000000..f95b2a6
--- /dev/null
+++ b/src/build/_importlib.py
@@ -0,0 +1,14 @@
+import sys
+
+
+if sys.version_info < (3, 8):
+    import importlib_metadata as metadata
+elif sys.version_info < (3, 9, 10) or (3, 10, 0) <= sys.version_info < (3, 10, 2):
+    try:
+        import importlib_metadata as metadata
+    except ModuleNotFoundError:
+        from importlib import metadata
+else:
+    from importlib import metadata
+
+__all__ = ['metadata']
diff --git a/src/build/_util.py b/src/build/_util.py
index 234297f..0582a28 100644
--- a/src/build/_util.py
+++ b/src/build/_util.py
@@ -27,10 +27,7 @@ def check_dependency(
     """
     import packaging.requirements
 
-    if sys.version_info >= (3, 8):
-        import importlib.metadata as importlib_metadata
-    else:
-        import importlib_metadata
+    from ._importlib import metadata
 
     req = packaging.requirements.Requirement(req_string)
     normalised_req_string = str(req)
@@ -51,8 +48,8 @@ def check_dependency(
             return
 
     try:
-        dist = importlib_metadata.distribution(req.name)
-    except importlib_metadata.PackageNotFoundError:
+        dist = metadata.distribution(req.name)
+    except metadata.PackageNotFoundError:
         # dependency is not installed in the environment.
         yield (*ancestral_req_strings, normalised_req_string)
     else:
diff --git a/src/build/util.py b/src/build/util.py
index 9f0570e..9f204b8 100644
--- a/src/build/util.py
+++ b/src/build/util.py
@@ -3,25 +3,19 @@
 from __future__ import annotations
 
 import pathlib
-import sys
 import tempfile
 
 import pyproject_hooks
 
 from . import PathType, ProjectBuilder, RunnerType
+from ._importlib import metadata
 from .env import DefaultIsolatedEnv
 
 
-if sys.version_info >= (3, 8):
-    import importlib.metadata as importlib_metadata
-else:
-    import importlib_metadata
-
-
-def _project_wheel_metadata(builder: ProjectBuilder) -> importlib_metadata.PackageMetadata:
+def _project_wheel_metadata(builder: ProjectBuilder) -> metadata.PackageMetadata:
     with tempfile.TemporaryDirectory() as tmpdir:
         path = pathlib.Path(builder.metadata_path(tmpdir))
-        return importlib_metadata.PathDistribution(path).metadata
+        return metadata.PathDistribution(path).metadata
 
 
 def project_wheel_metadata(
@@ -29,7 +23,7 @@ def project_wheel_metadata(
     isolated: bool = True,
     *,
     runner: RunnerType = pyproject_hooks.quiet_subprocess_runner,
-) -> importlib_metadata.PackageMetadata:
+) -> metadata.PackageMetadata:
     """
     Return the wheel metadata for a project.
 


Test patch:
diff --git a/tests/constraints.txt b/tests/constraints.txt
index ee7ad5f..36446db 100644
--- a/tests/constraints.txt
+++ b/tests/constraints.txt
@@ -1,4 +1,4 @@
-importlib-metadata==0.22
+importlib-metadata==4.6
 packaging==19.0
 pyproject_hooks==1.0
 setuptools==42.0.0; python_version < "3.10"
diff --git a/tests/test_main.py b/tests/test_main.py
index 456ff74..515dc22 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -135,7 +135,7 @@ def test_build_isolated(mocker, package_test_flit):
 
     install.assert_any_call({'flit_core >=2,<3'})
 
-    required_cmd.assert_called_with('sdist')
+    required_cmd.assert_called_with('sdist', {})
     install.assert_any_call(['dep1', 'dep2'])
 
     build_cmd.assert_called_with('sdist', '.', {})
diff --git a/tests/test_projectbuilder.py b/tests/test_projectbuilder.py
index f03839d..98a96ee 100644
--- a/tests/test_projectbuilder.py
+++ b/tests/test_projectbuilder.py
@@ -4,6 +4,7 @@
 import copy
 import logging
 import os
+import pathlib
 import sys
 import textwrap
 
@@ -12,13 +13,7 @@ import pytest
 
 import build
 
-
-if sys.version_info >= (3, 8):  # pragma: no cover
-    from importlib import metadata as importlib_metadata
-else:  # pragma: no cover
-    import importlib_metadata
-
-import pathlib
+from build import _importlib
 
 
 build_open_owner = 'builtins'
@@ -30,7 +25,7 @@ DEFAULT_BACKEND = {
 }
 
 
-class MockDistribution(importlib_metadata.Distribution):
+class MockDistribution(_importlib.metadata.Distribution):
     def locate_file(self, path):  # pragma: no cover
         return ''
 
@@ -48,7 +43,7 @@ class MockDistribution(importlib_metadata.Distribution):
             return CircularMockDistribution()
         elif name == 'nested_circular_dep':
             return NestedCircularMockDistribution()
-        raise importlib_metadata.PackageNotFoundError
+        raise _importlib.metadata.PackageNotFoundError
 
 
 class ExtraMockDistribution(MockDistribution):
@@ -167,7 +162,7 @@ class NestedCircularMockDistribution(MockDistribution):
     ],
 )
 def test_check_dependency(monkeypatch, requirement_string, expected):
-    monkeypatch.setattr(importlib_metadata, 'Distribution', MockDistribution)
+    monkeypatch.setattr(_importlib.metadata, 'Distribution', MockDistribution)
     assert next(build.check_dependency(requirement_string), None) == expected
 
 
@@ -502,7 +497,7 @@ def test_runner_user_specified(tmp_dir, package_test_flit):
 def test_metadata_path_no_prepare(tmp_dir, package_test_no_prepare):
     builder = build.ProjectBuilder(package_test_no_prepare)
 
-    metadata = importlib_metadata.PathDistribution(
+    metadata = _importlib.metadata.PathDistribution(
         pathlib.Path(builder.metadata_path(tmp_dir)),
     ).metadata
 
@@ -513,7 +508,7 @@ def test_metadata_path_no_prepare(tmp_dir, package_test_no_prepare):
 def test_metadata_path_with_prepare(tmp_dir, package_test_setuptools):
     builder = build.ProjectBuilder(package_test_setuptools)
 
-    metadata = importlib_metadata.PathDistribution(
+    metadata = _importlib.metadata.PathDistribution(
         pathlib.Path(builder.metadata_path(tmp_dir)),
     ).metadata
 
@@ -524,7 +519,7 @@ def test_metadata_path_with_prepare(tmp_dir, package_test_setuptools):
 def test_metadata_path_legacy(tmp_dir, package_legacy):
     builder = build.ProjectBuilder(package_legacy)
 
-    metadata = importlib_metadata.PathDistribution(
+    metadata = _importlib.metadata.PathDistribution(
         pathlib.Path(builder.metadata_path(tmp_dir)),
     ).metadata
 
diff --git a/tests/test_self_packaging.py b/tests/test_self_packaging.py
index 00870fd..b134f53 100644
--- a/tests/test_self_packaging.py
+++ b/tests/test_self_packaging.py
@@ -21,6 +21,7 @@ sdist_files = {
     'src/build/__init__.py',
     'src/build/__main__.py',
     'src/build/_exceptions.py',
+    'src/build/_importlib.py',
     'src/build/_util.py',
     'src/build/env.py',
     'src/build/py.typed',
@@ -31,6 +32,7 @@ wheel_files = {
     'build/__init__.py',
     'build/__main__.py',
     'build/_exceptions.py',
+    'build/_importlib.py',
     'build/_util.py',
     'build/env.py',
     'build/py.typed',