Skip to content

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

Closed
wants to merge 1 commit into from

Conversation

nascheme
Copy link
Member

@nascheme nascheme commented Jul 12, 2018

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

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.
@nascheme
Copy link
Member Author

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");
Copy link
Member Author

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.

Copy link
Member Author

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)
Copy link
Member Author

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
Copy link
Member Author

@nascheme nascheme Jul 12, 2018

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.

@nascheme
Copy link
Member Author

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.

@nascheme
Copy link
Member Author

GH-8229 has been merged and does a similar thing in a bit simpler fashion.

@nascheme nascheme closed this Jul 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants