Update distutils.core.setup/run_setup to better match setuptools.build_meta.
#71
Conversation
Adds a unit test for that scenario
`setuptools.build_meta` uses `tokenize.open` instead of open (and replaces `\r\n` with `\n`) when exec-ing `setup.py` files. It seems that the advantage of using `tokenize.open` is that it supports automatic encoding detection.
This allows reusing the function for already made distribution objects.
distutils/core.py
Outdated
| try: | ||
| try: | ||
| sys.argv[0] = script_name | ||
| if script_args is not None: | ||
| sys.argv[1:] = script_args | ||
| with open(script_name, 'rb') as f: | ||
| exec(f.read(), g) | ||
| _open = getattr(tokenize, 'open', open) |
There was a problem hiding this comment.
If Python < 3.2 is not supported, this getattr could also be dropped.
I was not sure about which version distutils is targetting.
There was a problem hiding this comment.
It should be the same as setuptools, so this could be dropped.
|
Thank you very much @FFY00 for the review. I updated he code to use |
There was a problem hiding this comment.
This change is dangerously close to an enhancement. For context, I'd like to limit feature enhancements for distutils to only the most urgent (platform compatibility) until distutils is merged into Setuptools and maintained there. The reason is that the current state of affairs means that most users are using distutils from the stdlib and not from this repo. Until SETUPTOOLS_USE_DISTUTILS=local becomes the default, this code is largely un-validated in the wild. Limiting the divergence of this repo with stdlib distutils will limit the amount of divergence and thus surprise when this distutils becomes default.
That said, this change is primarily about aligning with Setuptools, whose technique is proven and verified in the wild, so this change will actually aid in distutils adoption and limit the amount of divergence.
For that reason, let's proceed.
|
Thank you very much @jaraco. I will keep that in mind, and I totally understand what you explained. |
Motivation
I noticed that
setuptools.build_meta"re-implements" therun_setupfunction (there are differences between the 2 functions, e.g.distutils.core.run_setupwraps the code in atry...except, but the main idea behind both look similar to me).There is one major difference:
setuptoolsusetokenize.openand replaces\r\nwith\n.I went through setuptools commit history but did not find any comments/commit messages explaining exactly why, but I assume that it improves encoding detection.
My intention with this PR is to update the
distutilscode using the same technique as used insetuptools, so eventually the same function could be used.Changes
tokenize.opentechnique as employed bysetuptools.build_metaindistutils.core.run_setup.setupfunction and split outrun_commands, this way, one can usedist = run_setup("setup.py", stop_after="config")to get the configuration and later runrun_commands(dist).Side note
In terms of implications for
setuptools, I imagine that eventually we could build the "new-static-config-style" distribution objects directly from thesetup.cfg/pyproject.tomland then run them withrun_commands, instead of always creating asetup.pystub.Similarly, when
setup.pyis found,dist = run_setup("setup.py", stop_after="config")could be used to obtain thedistobject and then revert to the same flow as asetup.cfg/pyproject.toml-only project...(Basically a change in the mental model, instead of making
setup.cfg/pyproject.tomluse the same flow assetup.pyone, we can make asetup.pyuse the same flow assetup.cfg).