-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
bpo-32430: During 'configure', don't copy Setup.dist to Setup. #8260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In the past, users building Python were expected to modify the Modules/Setup file. In that case, preserving their local modifications on re-execution of 'configure' was useful. Further, to support users modifying Setup and re-runnning make, there was dependency rules in the Makefile to re-run makesetup if needed, which would re-generate Makefile and Modules/config.c. For modern builds of Python, most users never touch Modules/Setup. In that case, the behavior of preserving Modules/Setup is counter-productive since updating Modules/Setup.dist will result in a stale copy of Modules/Setup being used. In that case 'make' outputs a warning. In the past, the warning was the last output from 'make'. Now we run setup.py and so that warning is buried under a pile of build tool output. So, the net effect is that the build can fail and the user doesn't know how to fix it. Running 'configure' again is not sufficient because the out-of-date Modules/Setup file is preserved. This change does the following: - In 'configure', don't create Modules/Setup from Modules/Setup.dist. If the user wants a customized Setup, they will have to copy it themselves. - In 'makesetup', if Modules/Setup does not exist, read the config from Modules/Setup.dist. Note this is a bit tricky since the source and build folders can be separate (i.e. $srcdir/Modules/xyx is not the same as Modules/xyz). The effect of this change is that if Modules/Setup exists, it will be used in preference to Modules/Setup.dist. - In the 'Makefile', stop checking for an out-of-date Modules/Setup file. If the user is creating Setup, we will have to trust them to keep it up-to-date. That should not be a problem for 3rd party distributions like Red Hat, Debian, etc. They would normally start a build from a clean source tree and so nothing should be out-of-date. - In the 'Makefile', remove the dependency rule to re-run 'makesetup'. Again, this should never be necessary in the normal case. Running 'configure' will result in 'makesetup' getting run. There is also a make target 'makesetup' if someone wants to modify Modules/Setup and wishes to re-generate Makefile and Modules/config.c. Run "make makesetup" will process Setup/Setup.dist. - Change getpath.c to look for Modules/config.c as the marker for a source folder. That file gets generated and so we can use it in the same way that Modules/Setup was previously used. This marker is important in the case that separate build and source folders are being used.
Renaming Setup.dist to Setup, as done in GH-8229 could be done too. However, I don't see any benefits to doing that. It was the behavior of 'configure' copying the file (and refusing to do so if Setup already existed) that is the real source of the trouble, IMHO. The name of the file is not so important. Keeping it as Setup.dist will allow some 3rd party builds to continue to work as they do things like use 'sed' to take Setup.dist and create their own customized 'Setup'. With this PR, that will still work. OTOH, if they try to find an existing Modules/Setup, they will fail. Not sure which would be more common but leaving it as Setup.dist seems to more conservative fix. |
@@ -322,7 +322,7 @@ search_for_prefix(const _PyCoreConfig *core_config, | |||
/* Check to see if argv[0] is in the build directory */ | |||
wcsncpy(prefix, calculate->argv0_path, MAXPATHLEN); | |||
prefix[MAXPATHLEN] = L'\0'; | |||
joinpath(prefix, L"Modules/Setup"); | |||
joinpath(prefix, L"Modules/config.c"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, perhaps this is not okay for the PC build? Since we don't copy Setup.dist to Setup, the check would fail without a change. We could look for Setup.local but looking for config.c makes more sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Talking to myself, ;-)]
It looks like this should be okay. The PC build has its own getpath.c file and so this change doesn't affect it. The purpose of this check (as I understand) is to find if we are running 'python' from within the build directory. So, we use a file that exists in the build directory (and if separate source and build directories are used, not in the source directory).
# the user does not create or modify Setup and so Setup.dist | ||
# from the source distribution is used as-is. | ||
case $setupfile in | ||
*/Setup) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern matching on */Setup is a bit ugly. Because makesetup can be called with different arguments (not just from 'configure'), we have to handle running on different Setup files. Eons ago, makesetup + Setup would be used instead of setup.py for 3rd party extension modules. Now, I suspect 'makesetup' might only be used from within the cpython build.
echo "creating Modules/Setup" >&6 | ||
if test ! -f Modules/Setup | ||
then | ||
cp $srcdir/Modules/Setup.dist Modules/Setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These few lines are the source of the 'evilness' with the current behavior, IMHO. Setup is created from Setup.dist but only on the first run of 'configure'. If you update your source tree using the VCS, Setup.dist could get updated but Setup becomes stale. Re-running 'configure' is not sufficient to fix it. You have to do a "make distclean" or something dramatic.
I noticed another possible impact of this change. 'makesetup' can be used for extensions outside the cpython build tree. Before Greg Ward made distutils, that was how 3rd party C extensions were built. They would include their own Setup file in their source distribution and makesetup was called from the Python install library folder. I suspect that no extension actually uses Setup/makesetup anymore and they just use distutils. Using Setup for 3rd party extensions should still work but I haven't tested that. Related is the fact that Tools/freeze also looks at Setup. I haven't tested that either but I think it may still work too. |
GH-8229 has been merged and does a similar thing in a bit simpler fashion. |
In the past, users building Python were expected to modify the
Modules/Setup file. In that case, preserving their local
modifications on re-execution of 'configure' was useful. Further,
to support users modifying Setup and re-runnning make, there was
dependency rules in the Makefile to re-run makesetup if needed,
which would re-generate Makefile and Modules/config.c.
For modern builds of Python, most users never touch Modules/Setup.
In that case, the behavior of preserving Modules/Setup is
counter-productive since updating Modules/Setup.dist will result in
a stale copy of Modules/Setup being used. In that case 'make'
outputs a warning. In the past, the warning was the last output
from 'make'. Now we run setup.py and so that warning is buried
under a pile of build tool output. So, the net effect is that the
build can fail and the user doesn't know how to fix it. Running
'configure' again is not sufficient because the out-of-date
Modules/Setup file is preserved.
This change does the following:
In 'configure', don't create Modules/Setup from
Modules/Setup.dist. If the user wants a customized Setup, they
will have to copy it themselves.
In 'makesetup', if Modules/Setup does not exist, read the config
from Modules/Setup.dist. Note this is a bit tricky since the
source and build folders can be separate (i.e. $srcdir/Modules/xyx
is not the same as Modules/xyz). The effect of this change is
that if Modules/Setup exists, it will be used in preference to
Modules/Setup.dist.
In the 'Makefile', stop checking for an out-of-date Modules/Setup
file. If the user is creating Setup, we will have to trust them
to keep it up-to-date. That should not be a problem for 3rd party
distributions like Red Hat, Debian, etc. They would normally
start a build from a clean source tree and so nothing should be
out-of-date.
In the 'Makefile', remove the dependency rule to re-run
'makesetup'. Again, this should never be necessary in the normal
case. Running 'configure' will result in 'makesetup' getting run.
There is also a make target 'makesetup' if someone wants to modify
Modules/Setup and wishes to re-generate Makefile and
Modules/config.c. Run "make makesetup" will process
Setup/Setup.dist.
Change getpath.c to look for Modules/config.c as the marker for a
source folder. That file gets generated and so we can use it in
the same way that Modules/Setup was previously used. This marker
is important in the case that separate build and source folders
are being used.
https://bugs.python.org/issue32430