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.

Sign up for GitHub

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
nascheme wants to merge 1 commit into from
Closed

bpo-32430: During 'configure', don't copy Setup.dist to Setup. #8260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Doc/extending/extending.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@ of an unpacked source distribution, add a line to the file
and rebuild the interpreter by running :program:`make` in the toplevel
directory. You can also run :program:`make` in the :file:`Modules/`
subdirectory, but then you must first rebuild :file:`Makefile` there by running
':program:`make` Makefile'. (This is necessary each time you change the
:file:`Setup` file.)
':program:`make` makesetup'. (This is necessary each time you change the
:file:`Setup.local` file.)

If your module requires additional libraries to link with, these can be listed
on the line in the configuration file as well, for instance:
Expand Down
24 changes: 7 additions & 17 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,11 +684,7 @@ $(DLLLIBRARY) libpython$(LDVERSION).dll.a: $(LIBRARY_OBJS)
oldsharedmods: $(SHAREDMODS)


Makefile Modules/config.c: Makefile.pre \
$(srcdir)/Modules/config.c.in \
$(MAKESETUP) \
Modules/Setup \
Modules/Setup.local
makesetup:
$(SHELL) $(MAKESETUP) -c $(srcdir)/Modules/config.c.in \
-s Modules \
Modules/Setup.local \
Expand All @@ -697,16 +693,6 @@ Makefile Modules/config.c: Makefile.pre \
@echo "The Makefile was updated, you may need to re-run make."


Modules/Setup: $(srcdir)/Modules/Setup.dist
@if test -f Modules/Setup; then \
echo "-----------------------------------------------"; \
echo "Modules/Setup.dist is newer than Modules/Setup;"; \
echo "check to make sure you have all the updates you"; \
echo "need in your Modules/Setup file."; \
echo "Usually, copying Modules/Setup.dist to Modules/Setup will work."; \
echo "-----------------------------------------------"; \
fi

Programs/_testembed: Programs/_testembed.o $(LIBRARY) $(LDLIBRARY) $(PY3LIBRARY)
$(LINKCC) $(PY_LDFLAGS) $(LINKFORSHARED) -o $@ Programs/_testembed.o $(BLDLIBRARY) $(LIBS) $(MODLIBS) $(SYSLIBS) $(LDLAST)

Expand Down Expand Up @@ -1478,7 +1464,11 @@ libainstall: @DEF_MAKE_RULE@ python-config
$(INSTALL_DATA) Programs/python.o $(DESTDIR)$(LIBPL)/python.o
$(INSTALL_DATA) $(srcdir)/Modules/config.c.in $(DESTDIR)$(LIBPL)/config.c.in
$(INSTALL_DATA) Makefile $(DESTDIR)$(LIBPL)/Makefile
$(INSTALL_DATA) Modules/Setup $(DESTDIR)$(LIBPL)/Setup
@if test -e Modules/Setup; then \
$(INSTALL_DATA) Modules/Setup $(DESTDIR)$(LIBPL)/Setup ; \
else \
$(INSTALL_DATA) $(srcdir)/Modules/Setup.dist $(DESTDIR)$(LIBPL)/Setup ; \
fi
$(INSTALL_DATA) Modules/Setup.local $(DESTDIR)$(LIBPL)/Setup.local
$(INSTALL_DATA) Misc/python.pc $(DESTDIR)$(LIBPC)/python-$(VERSION).pc
$(INSTALL_SCRIPT) $(srcdir)/Modules/makesetup $(DESTDIR)$(LIBPL)/makesetup
Expand Down Expand Up @@ -1767,7 +1757,7 @@ Python/thread.o: @THREADHEADERS@
.PHONY: frameworkinstallmaclib frameworkinstallapps frameworkinstallunixtools
.PHONY: frameworkaltinstallunixtools recheck clean clobber distclean
.PHONY: smelly funny patchcheck touch altmaninstall commoninstall
.PHONY: gdbhooks
.PHONY: gdbhooks makesetup

# IF YOU PUT ANYTHING HERE IT WILL GO AWAY
# Local Variables:
Expand Down
7 changes: 7 additions & 0 deletions Misc/NEWS.d/next/Build/2018-07-11-17-49-25.bpo-32430.c5qJcT.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
The 'configure' script no longer copies Modules/Setup.dist to Modules/Setup.
If Modules/Setup exists, it will be used in preference to Modules/Setup.dist
(this fallback logic is done by the Modules/makesetup script). Remove the
makefile rules to re-generate Makefile and Modules/config.c if Modules/Setup
or Modules/config.c.in changes. To re-generate, you must run "make
makesetup". Normally that is not required since 'configure' already runs
makesetup.
2 changes: 1 addition & 1 deletion Modules/getpath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy Markdown
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.

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
Copy Markdown
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.

[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).

if (isfile(prefix)) {
/* Check VPATH to see if argv0_path is in the build directory. */
vpath = Py_DecodeLocale(VPATH, NULL);
Expand Down
18 changes: 15 additions & 3 deletions Modules/makesetup
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,23 @@ CYGWIN*) if test $libdir = .
esac

# Main loop
for i in ${*-Setup}
for setupfile in ${*-Setup}
do
case $i in
# Check if Setup file does not exist. In that case, use
# Setup.dist from libdir (source) folder. In the normal case,
# 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
Copy Markdown
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.

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.

if ! test -f $setupfile; then
setupfile=$libdir/Setup.dist
fi
;;
esac

case $setupfile in
-n) echo '*noobjects*';;
*) echo '*doconfig*'; cat "$i";;
*) echo '*doconfig*'; cat "$setupfile";;
esac
done |
sed -e 's/[ ]*#.*//' -e '/^[ ]*$/d' |
Expand Down
6 changes: 0 additions & 6 deletions configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18355,12 +18355,6 @@ $as_echo "$as_me: WARNING: unrecognized options: $ac_unrecognized_opts" >&2;}
fi


echo "creating Modules/Setup" >&6
if test ! -f Modules/Setup
then
cp $srcdir/Modules/Setup.dist Modules/Setup
Copy link
Copy Markdown
Member Author

@nascheme nascheme Jul 12, 2018 •
edited
Loading

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.

fi

echo "creating Modules/Setup.local" >&6
if test ! -f Modules/Setup.local
then
Expand Down
6 changes: 0 additions & 6 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5565,12 +5565,6 @@ AC_CONFIG_FILES(Makefile.pre Misc/python.pc Misc/python-config.sh)
AC_CONFIG_FILES([Modules/ld_so_aix], [chmod +x Modules/ld_so_aix])
AC_OUTPUT

echo "creating Modules/Setup" >&AS_MESSAGE_FD
if test ! -f Modules/Setup
then
cp $srcdir/Modules/Setup.dist Modules/Setup
fi

echo "creating Modules/Setup.local" >&AS_MESSAGE_FD
if test ! -f Modules/Setup.local
then
Expand Down