Repository navigation
Simplify Modules/Setup{,.dist,.local} #76611
Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2018-07-16.17:04:41.138>
created_at = <Date 2017-12-27.11:15:03.747>
labels = ['type-feature', '3.8', 'build']
title = 'Simplify Modules/Setup{,.dist,.local}'
updated_at = <Date 2019-05-06.15:37:43.569>
user = 'https://github.com/JulienPalard'bugs.python.org fields:
activity = <Date 2019-05-06.15:37:43.569>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-07-16.17:04:41.138>
closer = 'pitrou'
components = ['Build']
creation = <Date 2017-12-27.11:15:03.747>
creator = 'mdk'
dependencies = []
files = []
hgrepos = []
issue_num = 32430
keywords = ['patch']
message_count = 28.0
messages = ['309079', '309081', '309273', '309281', '309282', '309283', '309285', '309286', '309287', '309613', '309654', '309698', '310312', '310343', '310345', '310346', '310348', '310372', '310373', '311752', '321395', '321493', '321570', '321744', '321745', '321768', '341533', '341540']
nosy_count = 12.0
nosy_names = ['twouters', 'barry', 'nascheme', 'pitrou', 'vstinner', 'eric.smith', 'xdegaye', 'martin.panter', 'koobs', 'yan12125', 'mdk', 'anthony shaw']
pr_nums = ['5062', '8229', '8260', '5015']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue32430'
versions = ['Python 3.8']
Activity
JulienPalard commentedon Dec 27, 2017
Related to: https://bugs.python.org/issue32429
The bahavior of Modules/Setup{,.dist,.local} looks inherited from an old time:
From this point in time, Setup is newer than Setup.dist (as copied from it), the following can happen:
We don't know, so the Makefile is printing a warning telling he don't know and we have to fix the situation. First problem is: this warning is invisible due to the high load of Makefile prints, this is the issue https://bugs.python.org/issue32429.
Proposed solutions are:
Marc-Andre Lemburg noted that for those using a sources tarballs, in the single Setup file case, it's not nice for them to loose the original content of Setup when modifying it, and being able to do a
cp [Modules/Setup.dist](https://github.com/python/cpython/blob/main/Modules/Setup.dist) [Modules/Setup](https://github.com/python/cpython/blob/main/Modules/Setup)to restore it is nice.Personally I'll go for a single Setup file (No .dist, no .local, no copy), that one can modify and commit, delegating the pain of merging to git, as I prefer simple solutions.
I don't think the importance of keeping a ".dist" backup is worth the pain and we're not doing it for other files (If one using a source tarball is modifying a .c file, the original content is lost too, and we'll not provide .c.dist files).
pitrou commentedon Dec 27, 2017
+1 for this.
xdegaye commentedon Dec 31, 2017
The Setup files are configuration files, so their handling is different from the handling of Makefile.pre.in or the handling of the source files.
The only reference to Setup.local in the Python documentation is at https://docs.python.org/3/extending/extending.html#compilation-and-linkage, so most users that need to configure the build with a Setup file are probably using Modules/Setup instead of Modules/Setup.local and the solution that implements this enhancement cannot overwrite an existing Modules/Setup file. This solution must also be robust and transparent when switching to/from a branch where the solution is not implemented yet (a bpo branch created earlier).
PR 5062 adds the
--enable-use-setup-distconfigure option to use Modules/Setup.dist instead of Modules/Setup to build the Makefile. The build behavior is unchanged when this option is not used.pitrou commentedon Dec 31, 2017
I would hope to simplify the build process, not to complicate it. In other words, I think the PR is a step in the wrong direction, *except* if using Setup.dist becomes the default and the other way is clearly marked deprecated.
pitrou commentedon Dec 31, 2017
Note the use case we're trying to preserve here is probably extremely rare. I've never needed it myself, and I've never seen it done by anyone else. We're talking about a very small demographics who's probably skilled enough to handle a little complexity in their workflow.
xdegaye commentedon Dec 31, 2017
In msg294174 Thomas says he intends to use it for Python at Google in order to "avoid third-party libraries even when they are available on the build system".
I do not have a strong opinion about this. Implementing the reverse configure option '--enable-custom-setup' is straightforward (actually this is what I did upon the first shot at this PR), Setup.dist becomes the default then and Thomas can still use Setup.local to add the *disabled* modules. It is annoying that there is no documentation on the build process that can be modified to describe the deprecation in details. Do you think a What'sNew entry is sufficient ?
pitrou commentedon Dec 31, 2017
First, I would count Google in the extremely skilled category :-)
(cc'ing Thomas so that he can chime in)
Second, if someone are doing a custom build of Python, they are very likely modifying something else than Modules/Setup (*). So they already have some kind of custom branch or fork of CPython tracking their own customizations. Why Modules/Setup can't be handled the same way I'm not sure I understand.
() see for example the Anaconda build of Python, which modifies several files but *not Modules/Setup: https://github.com/AnacondaRecipes/python-feedstock/
pitrou commentedon Dec 31, 2017
Yes, I think so. Also my hope is to remove it in 3.8 :-)
xdegaye commentedon Dec 31, 2017
Yes 3.8 and this leaves plenty of time for writing some documentation on the build process :-)
JulienPalard commentedon Jan 7, 2018
I also think PR 5062 complicate the thing I'm trying to simplify, it's the wrong direction.
I also don't understand why having a single versioned
Setupfile is not enough, can someone give a clear explanation on this point?vadmium commentedon Jan 8, 2018
FWIW there was documentation in the README about the Setup files, removed in Subversion r57681 (= Git revision 1c896e3). It looks like it still survives in the 2.7 version.
Yhg1s commentedon Jan 9, 2018
We do use Setup/Setup.local at Google (and have for many years now), and I find it very useful. (FWIW, the 'we would use' comment in msg294174 was about the new '*disabled*' feature, not about Setup files in general.) Avoiding local changes is also important to us, because it makes it much easier to track upstream changes. If anyone was thinking of getting rid of Modules/Setup, please don't, at least not without something more sensible than setup.py to replace it :)
That said, I don't really care about the Setup.dist/Setup distinction. I vaguely recall the situation before we had Setup.dist, which I guess was slightly worse, but primarily for people who build from the source repo. Having a Setup.local file is convenient as long as the Setup file is necessary for the base build, but Setup/Setup.dist doesn't matter.
5 remaining items
xdegaye commentedon Jan 21, 2018
I fully concur with all points in Victor msg310345.
xdegaye commentedon Jan 21, 2018
A solution to this issue must deal with the following use case:
A developer has made changes to Modules/Setup and is tracking these changes in a VCS, for example with mercurial or with a git submodule through a symlink. By the time he is about to pull the commit that is fixing this issue from the Python repository, he is also experimenting with a new modification in Modules/Setup that he has not commited or staged yet in his own mercurial or git submodule repository or whatever.
IMO a solution to this issue that overwrites the current user changes when he is merging the corresponding commit is not acceptable.
JulienPalard commentedon Feb 6, 2018
Taking arguments into account, this solution may work:
be Setup.dist
cp Setup.dist SetupThis solution fixes the bpo-32429 as there is no longer a copy, it does not starts to track the Setup file so it should nor introduce any issue while switching to and from branches with this modification, and allow one to have multiple named configurations.
pitrou commentedon Jul 10, 2018
I've created a PR that renames Setup.dist to Setup and removes extraneous logic.
nascheme commentedon Jul 11, 2018
PR 8229 doesn't work if the build dir is separate from the src dir (mentioned in the PR comments). The attached patch (makesetup_fallback_dist.txt) changes makesetup to fallback to using $srcdir/Modules/Setup.dist if Modules/Setup doesn't exist. I removed the copying of Setup.dist to Setup. Also, I removed the dependencies in the makefile that look for an out-of-date Setup file and the ones that automatically run makesetup. There is now a "makesetup" target with no deps. You would have to run "make makesetup" if you want to regenerate config.c and the Makefile. Otherwise, running "configure" would also do the job.
One possible issue with the patch: I expect that some build systems expect that, after configure, Modules/Setup will exist in the build dir and can be modified. For that reason, I think a better fix is to make 'configure' unconditionally create Modules/Setup from $srcdir/Modules/Setup.dist.
I'm attaching as a patch as I'm not sure about creating a bunch of half-baked PRs to show ideas. It seems to me that a small patch as a text attachment is better. Maybe I'm a dinosaur. ;-P
nascheme commentedon Jul 12, 2018
I removed the "makesetup_fallback_dist.txt". PR-8260 is a more polished implementation of the same idea.
pitrou commentedon Jul 16, 2018
New changeset 961d54c by Antoine Pitrou in branch 'master':
bpo-32430: Rename Modules/Setup.dist to Modules/Setup (GH-8229)
961d54c
pitrou commentedon Jul 16, 2018
With PR bpo-8229 pushed the distinction between Setup.dist and Setup has disappeared: you have a Modules/Setup file in the source tree and that's where you do any changes, which you are responsible to track yourself (e.g. using a patch file or a git fork).
vstinner commentedon Jul 16, 2018
Thank you Antoine for fixing this issue! This issue annoyed me forever, but
I never tried to fix it. It was very annoying when using git bisect for
example.
tonybaloney commentedon May 6, 2019
PR #49312 is still open in GitHub, is this PR still required?
vstinner commentedon May 6, 2019
It's not. I closed it, thanks.