Revert Only pass --build-option to bdist_wheel in build_meta

[jameshilliard]

Summary of changes

Reverts #4079 which broke the ability to pass build_ext arguments via config settings.

For example this command was broken by #4079 when building psycopg2.

python -m build -n -s -C--build-option=build_ext -C--build-option=--pg-config=/home/buildroot/buildroot/output/per-package/python-psycopg2/host/aarch64-buildroot-linux-gnu/sysroot/usr/bin/pg_config

Closes

Pull Request Checklist

• Changes have tests
• News fragment added in newsfragments/.
  (See documentation for details)

[abravalheri]

Hi @jameshilliard , thank you very much for the contribution.

There is a problem of lack of consensus here.
According to the previous discussion the solution of passing the arguments to all hooks also breakes the build process for some other users in a different way...

The best here would be working towards a more prevalent solution.

[jameshilliard]

There is a problem of lack of consensus here.
According to the previous discussion the solution of passing the arguments to all hooks also breakes the build process for some other users in a different way...

The impression I got from the various threads is that changing the behavior of --build-option like this would cause a regression and it seems nobody from that thread noticed #4079 to comment on it. It's unclear why the behavior for this flag which which was previously working correctly for a number of use cases was changed to do something very different without any usable replacement option for those use cases.

[abravalheri]

Hi @jameshilliard thank you very much for having a deeper look on this and working towards the solution.

The impression I got from the various threads is that changing the behavior of --build-option like this would cause a regression and it seems nobody from that thread noticed #4079 to comment on it.

I don't think this assessment is entirely accurate.

In the thread linked, there is a lot of people asking for a long term solution, which is the point being discussed in #4083.

When working on the error that prompted #4079, I proposed the patch (describing the approach I would use for it) in the same thread, so even if the participants did not had the time review #4079, they were informed about the proposed changes before they were applied.

Moreover, after the patch proposal, there was one comment "against", but my interpretation is that it was a comment in the nature "we need a proper/major fix or long-term solution" other than against the particular proposed patch (I did point that out in the thread).

In the same thread, I got feedback from one of pip's maintainers, that agreed the patch would be good enough for a quick-and-dirty solution that does not solve the overall problem, but does improve the error being discussed.

#4079 makes it clear that the change is not the final solution for the config_settings saga, but just a stopgap. Handling config_settings is a hard challenge because the way PEP 517 runs each build hook in a brand new process and because evaluating setup.py imposes a strong level of decoupling between setuptools.build_meta and the rest of setuptools. It requires a lot of work. #4083 aims at discussing this challenge and trying to find solutions for it.

---

Now, my problem with this PR is that it fixes problems for one group of users, but re-introduces problems for another group of users... We are between a rock and a hard place here, so I don't think adopting this PR exactly how it is right now would be beneficial, we would only be caught in the middle of a tug of war...

The best approach in my opinion is proceeding towards #4083, which would fix the problem for all groups of users.

---

Suggested Workaround

Please also note that the way it is right now config_settings is at best supported in a BETA/best-case basis, for all intents and purposes... It is not something we can really consider an stable interface on the setuptools side.

The only known "stable" mechanism so far for passing configuration parameters for individual commands is via setup.cfg.

If no other tool is relying on DIST_EXTRA_CONFIG¹, you can also use it² to avoid modifying an existing setup.cfg and instead add an extra one.

This is an arbitrary example that does not correspond to the reproducer mentioned in the PR original text, but gives a good idea of what is possible to do:

export DIST_EXTRA_CONFIG=/tmp/setuptools-build.cfg
echo -e '[build_ext]\nparallel = 8\n[bdist_wheel]\npy_limited_api = cp311' > $DIST_EXTRA_CONFIG
python -m build

Footnotes

1. I know that cibuildwheel takes over DIST_EXTRA_CONFIG for their needs to communicate with setuptools. ↩

2. Needs "latest" distutils, so it will not work with SETUPTOOLS_USE_DISTUTILS=stdlib. ↩

[jameshilliard]

When working on the error that prompted #4079, I proposed the patch (describing the approach I would use for it) in the same thread, so even if the participants did not had the time review #4079, they were informed about the proposed changes before they were applied.

I'm mostly referring to this comment which seems to quite clearly indicate that this would be a significant regression.

In that comment a stopgap approach for not breaking existing use cases is suggested as well and it's unclear to me why that approach was not taken instead of this one which breaks existing use cases without providing a usable replacement for those use cases.

See:

The easiest thing for setuptools to do in my mind is to introduce a set of “legacy” namespaced build options, one each for egg_info, sdist and wheel. Paper over the cracks while we hash out what is to become of setuptools’ config settings.

In the same thread, I got feedback from one of pip's maintainers, that agreed the patch would be good enough for a quick-and-dirty solution that does not solve the overall problem, but does improve the error being discussed.

When it comes to python packaging there seems to be an issue where non-pip based use cases are an afterthought.

Now, my problem with this PR is that it fixes problems for one group of users, but re-introduces problems for another group of users... We are between a rock and a hard place here, so I don't think adopting this PR exactly how it is right now would be beneficial, we would only be caught in the middle of a tug of war...

So why not just offer a new flag for the new behavior and preserve the previous behavior under the old flag as a stopgap?

Please also note that the way it is right now config_settings is at best supported in a BETA/best-case basis, for all intents and purposes... It is not something we can really consider an stable interface on the setuptools side.

So setuptools warns quite loudly that directly invoking setup.py is deprecated, which kinda implies that one should be able to transition to pep517. This approach of deprecating/breaking features before having working replacements are ready has made transitioning to pep517 quite difficult in general for distros, this was a major issue with even having a pep517 frontend working in the first place as it wasn't originally possible to bootstrap a pep517 toolchain properly as it was incorrectly assumed everyone could just use pip.

If no other tool is relying on DIST_EXTRA_CONFIG1, you can also use it2 to avoid modifying an existing setup.cfg and instead add an extra one.

Hmm, I'll give this a try and see if it's workable for us at all.

I'll reopen #4217 for now since it seems to at least partially reduce the breakage from this change.

[layday]

I'm mostly referring to this comment which seems to quite clearly indicate that this would be a significant regression.

I wasn't aware that $DIST_EXTRA_CONFIG existed when I made that comment. You might be able to generate a throw-away config file using process substitution for a more direct alternative to config settings - unsure if that's supported by the env var.

[abravalheri]

Hi @jameshilliard, thank you for taking the time to go through my comments and add your toughts.

I just want to highlight that I am definitely not against fixing this bug, and that when the patch you reported as problematic was introduced, to the best of my knowledge, it was not a given that it would end up causing exceptions (you were the first one to openly report crashes that to us, after a couple of months that the PR was merged). I honestly apologize for the inconvenience.

For the sake of increasing productivity and avoid further stress, the criteria that I am considering wheatear to accept PRs targeting the issue is:

• The PR should not introduce other problems (or re-introduce)
• The PR should not introduce significant changes in user interface that has not been previously discussed (e.g., adding a config_settings["--temporary-legacy-options"] would need to be first discussed).

... + the usual stuff (pass the tests, add relevant test cases, etc...)

---

I'm mostly referring to this comment which seems to quite clearly indicate that this would be a significant regression.

You probably already saw this, but these concerns and viability were discussed in:

#2491 (comment)
#2491 (comment)

In that comment a stopgap approach for not breaking existing use cases is suggested as well and it's unclear to me why that approach was not taken instead of this one which breaks existing use cases without providing a usable replacement for those use cases.

You mentioned a proposed stopgap approach that would not break existing use cases. Please note that the proposal was not accompanied by a PR and that adding more namespaces/flags/options/structure to the interface would require more discussion... My honest interpretation of this comment was: "this is not enough users have the need for passing config_settings to all build hooks"; to which my line of thought was: "look passing config_settings to all hooks is already broken right now, it does not work. Not doing anything does not help. Let's make --global-option and --build-option partially work as they were intended to be legacy backward compatible with pip, while we keep looking for a long-term solution".

When it comes to python packaging there seems to be an issue where non-pip based use cases are an afterthought.

This is a very fair comment, and I agree with that. Ideally we should involve people that have stakes of different use cases, so everyone has a say.

In Setuptools we are open to work with everyone and we try to find compromises that also would work for non-pip based use cases. There were changes in both pypa/distutils and pypa/setuptools code bases that were prompted and developed together with people from the fedora/debian/arch/nix/conda/... communities. We just ask everyone to please bear with us because setuptools/distutils is complex.

In this particular instance, it is clear that both config_settings["--build-option"] and config_settings["--global-option"] were added as a temporary backward-compatibility hack to interoperate with pip's legacy options. Not only they use the same name as the pip's options, but also they are hilariously convoluted to write in the CLI, which cannot be justified otherwise. So for better or worse, pip is the primary target. What happens is that people have other needs, so out of necessity they have to resort to using this interface as well.

But equally in other places in the code base, you can find compatibility specific hacks for non-pip use cases.

So why not just offer a new flag for the new behavior and preserve the previous behaviour under the old flag as a stopgap?

Compounding more fragile APIs on top of an already fragile API does not sound like a good idea to me.

I might to sound like a broken record but: it was not immediately a given that the change would break other workflows. It took around 3 months for us to be reported otherwise - it there was an immediate report before pip have changed their code base, I would probably have reverted the change on the spot.

The reason why we are having the discussion in the first place is because config_settings implementation when PEP 517 was first introduced in setuptools was seriously lacky, if not completely unusable. In hindsight, probably it would be best to add a warning every time that config_settings was passed as a non-empty dict, so people at least would not create false expectations about this feature being stable (if anyone is interested on that, please feel free to open a PR).

So setuptools warns quite loudly that directly invoking setup.py is deprecated, which kinda implies that one should be able to transition to pep517. This approach of deprecating/breaking features before having working replacements are ready has made transitioning to pep517 quite difficult in general for distros, this was a major issue with even having a pep517 frontend working in the first place as it wasn't originally possible to bootstrap a pep517 toolchain properly as it was incorrectly assumed everyone could just use pip.

Please note that the whole transition to PEP 517 was extremely difficult to setuptools itself as well. We still have problems nowadays with it, and we still have spend tons of energy on it. We are all kind of in the same boat.

Since deprecation warnings and documentation changes are the only means setuptools have to let their users know that something will change at some point in the future, our approach is to add them as soon as possible. However, we still keep deprecated features around for months in a row (and more often years) to avoid major breaks...

I'll reopen #4217 for now since it seems to at least partially reduce the breakage from this change.

Thank you very much. I think that is a good idea. If we can fix this problem without re-introducing other issues, that would be the best.

[abravalheri]

We probably can close this now, in favour of #4217.