Avoid using private _config_vars

[Avasam]

Summary of changes

This PR attempts to avoid using a private global variables, to instead use the available public API. This resolves a noqa and upcomming type: ignore in https://github.com/pypa/setuptools/pull/3979/files#diff-820ceee125a584dd996725a342b2aaec47c5e96399faf426348dfeebe242ea4bR31
All 3 implementations of get_config_vars (in sysconfig, distutils.sysconfig, and setuptools._distutils.sysconfig) return the global dictionary directly (not a copy), so usage in build_ext should be equivalent.
get_config_vars also already ensures _config_vars is initialized

I'm not sure about the test change however.

Pull Request Checklist

• Changes have tests
• News fragment added in newsfragments/. (I don't think this counts as user-facing changes?)
  (See documentation for details)

[abravalheri]

Changes in setuptools/_distutils/tests/test_util.py are overwritten every time setuptools syncs up with pypa/distutils, so probably they have to be proposed in the pypa/distutils repository first and later merged here.

[Avasam]

Thanks! I can go make the changes upstream to the right place.

[abravalheri]

Hi @Avasam, thank you for the contribution and all the effort you are putting toward improving the typing in setuptools.

All 3 implementations of get_config_vars (in sysconfig, distutils.sysconfig, and setuptools._distutils.sysconfig) return the global dictionary directly (not a copy), so usage in build_ext should be equivalent.
get_config_vars also already ensures _config_vars is initialized

I don't understand this comment, could you please expland a little bit...

Isn't disutils.get_config_vars using copy() of sysconfig.get_config_vars() in https://github.com/pypa/distutils/blob/e651e536ef1fbafbe48856815602127df0371835/distutils/sysconfig.py#L539 ? Also this is an implementation detail that can change at any point, right?

Other than that, build_ext is clearly attempting to modify the global state in the _customize_compiler_for_shlib... As ugly as an attempt of using a private variable may look, my personal opinion is the following:

• The code still reads more clear and intentional to me the way it is now: I can clearly see that build_ext is doing something shady and that _customize_compiler_for_shlib only works because it patches the internals of distutils¹.
• If at some point distutils stops using _config_vars, the tests will fail immediately when we try to access an attribute that is no longer defined, and we will be forced to re-evaluate the patching technique. By using a public API, we loose this "free sanity check".

Footnotes

1. I.e, distutils.sysconfig.customize_compiler is only affected because distutils.sysconfig._config_vars is patched. ↩

[Avasam]

Isn't disutils.get_config_vars using copy() of sysconfig.get_config_vars() in pypa/distutils@e651e53/distutils/sysconfig.py#L539 ?

Only if it was None (ie: not already initialized), in which case setuptools would've failed anyway with an AttributeError

Also this is an implementation detail that can change at any point, right?

Possibly? But setuptools is also in control of that now that distutils isn't part of the stdlib. (and I doubt a security update to Python 3.8 - 3.11 would change this behaviour)

Other than that, build_ext is clearly attempting to modify the global state in the _customize_compiler_for_shlib... As ugly as an attempt of using a private variable may look, my personal opinion is the following:

• The code still reads more clear and intentional to me the way it is now: I can clearly see that build_ext is doing something shady and that _customize_compiler_for_shlib only works because it patches the internals of distutils1.
• If at some point distutils stops using _config_vars, the tests will fail immediately when we try to access an attribute that is no longer defined, and we will be forced to re-evaluate the patching technique. By using a public API, we loose this "free sanity check".

Whilst I'm challenging the use of a variable that is both private and not exposed in stubs, if you truly believe this is what setuptools should be doing, then I can expose _config_vars in the distutils stubs to help type checkers in #3979 & #4192 . This conversation will serve as the rationale for exposing it in typeshed.

[abravalheri]

Thank you very much @Avasam for having a deeper look on this.

Whilst I'm challenging the use of a variable that is both private and not exposed in stubs, if you truly believe this is what setuptools should be doing, then I can expose _config_vars in the distutils stubs to help type checkers.

My personal opinion is that setuptools is doing something shady and using the public API of distutils would only give a false sense of security.

The reality is that the patch is entangled to depend on that particular private variable... The moment the implementation changes everything falls apart. By importing the private attribute, at least we have a small "sanity check" giving hints about the global change in the state being necessary for the patching to have the desirable effect.

Unless Jason have requested this change, I think we can just be happy with our lives and add an type: ignore comment where it is needed. We probably don't have to change the distutils stubs (because frankly we are doing something shady anyway). Or if we really want to put more effort on this, add a cast to "reinterpret" the type somewhere inside of the file?

[Avasam]

A type-ignore with an explicit type declaration (or cast if it's really needed) with a documented resolution is fine by me in this case.

[Avasam]

Closed by 55eeabd