main: ensure config_settings are passed to get_requires_for_build

[jameshilliard]

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 #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 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 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](pypa/setuptools#3380) change the situation? — Reply to this email directly, [view it on GitHub](#627 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AHNRFWGCFNSRYNTI6VCMZX3XNIHZVANCNFSM6AAAAAAZTUGI2Y). You are receiving this because you were mentioned.Message ID: ***@***.***>

[FFY00]

I commented on #264 (comment), 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?

[build-system]
requires = ["setuptools"]
build-backend = "backend"
backend-path = ["_custom_build"]

# _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 at least 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.

[jameshilliard]

@FFY00 did you get a chance to review this?